* scroll-conservatively overflow
@ 2004-04-16 10:00 Juanma Barranquero
2004-04-16 13:17 ` Kim F. Storm
0 siblings, 1 reply; 13+ messages in thread
From: Juanma Barranquero @ 2004-04-16 10:00 UTC (permalink / raw)
I like scrolling one line at a time. I have
(setq scroll-step 0
scroll-conservatively most-positive-fixnum)
on my .emacs because scroll-step's documentation recommends setting
scroll-conservatively to "a large value" and most-positive-fixnum seems
less arbitrary than 10000 or 65535 or whatever.
That used to work; now it doesn't (although I'm not sure it is because
of a recent change or I just took a time to notice), because
scroll-conservatively gets multiplied by FRAME_LINE_HEIGHT (f) in a
couple of places, and that causes an overflow.
I have two options: either cutting scroll-conservatively down to a
manageable size before any use of it, or patching the docstring for
scroll-step to say that a large, but *reasonable*, value should be used.
I favor patching try_scrolling, because I don't even want to think how
to describe "a reasonable value" on the docstring :) and because we should
protect against unintended overflows like this one (the users can have
set scroll-conservatively to a big value on their .emacs and would get
wrong behaviour when switching to 21.X, X > 3).
So, if no one opposes, I'll install the attached patch.
Juanma
--- xdisp.c.orig 2004-04-14 22:33:44.000000000 +0200
+++ xdisp.c 2004-04-16 11:39:38.000000000 +0200
@@ -10881,2 +10881,8 @@
+ /* Force scroll_conservatively to have a reasonable value so it doesn't cause
+ an overflow while computing how much to scroll. */
+ if (scroll_conservatively)
+ scroll_conservatively = min (scroll_conservatively,
+ MOST_POSITIVE_FIXNUM / FRAME_LINE_HEIGHT (f));
+
/* Compute how much we should try to scroll maximally to bring point
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: scroll-conservatively overflow
2004-04-16 10:00 scroll-conservatively overflow Juanma Barranquero
@ 2004-04-16 13:17 ` Kim F. Storm
2004-04-16 14:03 ` Romain Francoise
2004-04-17 19:42 ` Richard Stallman
0 siblings, 2 replies; 13+ messages in thread
From: Kim F. Storm @ 2004-04-16 13:17 UTC (permalink / raw)
Cc: emacs-devel
Juanma Barranquero <jmbarranquero@wke.es> writes:
> I like scrolling one line at a time. I have
>
> (setq scroll-step 0
> scroll-conservatively most-positive-fixnum)
>
> on my .emacs because scroll-step's documentation recommends setting
> scroll-conservatively to "a large value" and most-positive-fixnum seems
> less arbitrary than 10000 or 65535 or whatever.
>
> That used to work; now it doesn't (although I'm not sure it is because
> of a recent change or I just took a time to notice), because
> scroll-conservatively gets multiplied by FRAME_LINE_HEIGHT (f) in a
> couple of places, and that causes an overflow.
That change was made almost a year ago...
I changed some things related to scroll a few days ago which may have
made this more visible -- I don't know.
> So, if no one opposes, I'll install the attached patch.
Please install it.
--
Kim F. Storm <storm@cua.dk> http://www.cua.dk
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: scroll-conservatively overflow
2004-04-16 13:17 ` Kim F. Storm
@ 2004-04-16 14:03 ` Romain Francoise
2004-04-30 11:35 ` Romain Francoise
2004-04-17 19:42 ` Richard Stallman
1 sibling, 1 reply; 13+ messages in thread
From: Romain Francoise @ 2004-04-16 14:03 UTC (permalink / raw)
no-spam@cua.dk (Kim F. Storm) writes:
> I changed some things related to scroll a few days ago which may have
> made this more visible -- I don't know.
About scrolling: I've been bitten by a bug for some weeks now, I
originally thought it was specific to the multi-tty code but I
discovered this morning that it's not. (A fresh checkout from CVS HEAD
this morning exhibited the same issues.)
I see it in two different situations:
1. In Gnus, when I scroll a long article buffer (several pages) by
repeatedly hitting RET to scroll one line at a time, scrolling stops
on truncated lines. For example, when I read an article that
contains one very long line (e.g. a URL), it scrolls okay until the
long line is the first line in the Article buffer. I hit RET again
and the first part of the line scrolls away. I still see in the
buffer the continuation part of the long (truncated) line, and if I
hit RET again, nothing happens, it doesn't scroll more. I have
traced the call back in the Lisp code and it shows that the
`scroll-up' builtin function returns nil as usual but doesn't have
any effect on the buffer. I can keep hitting RET to no avail.
Scrolling works fine until the "End of buffer" message as usual when
the article does not contain long lines; it also works fine if I
resize the window so that lines don't get truncated. It also works
fine if I scroll past long lines in the article with SPC, then use
RET to scroll one line at a time.
This bug is reproducible in all instances of Emacs, at once.
2. In Dired, I get "End of buffer" errors when moving over truncated
lines, from the top to the bottom of a buffer (with `n'). For
example, if my Dired buffer contains, in the middle of a long
listing:
-rw-r--r-- 1 romain romain 2047 Feb 26 2003 bpf_dump.c
lrwxr-xr-x 1 romain romain 22 Nov 11 12:26 bpf_filter.c -> ./bpf/net/bpf_filter.c
-rw-r--r-- 1 romain romain 4966 Feb 26 2003 bpf_image.c
(The middle line is truncated after "-> ./bp", I see:
bpf_filter.c -> ./bp\
f/net/bpf_filter.c
which might not be the case for you depending on your window width.)
I move the cursor down to the `b' in "bpf_dump.c". I then use `n'
which moves the point to the `b' in "bpf_filter.c". If I then use
`n' to move down to the next line over the symlink line, the bell is
rung, and I get a "End of buffer" message in the minibuffer, which
is obviously wrong since the buffer continues.. After the error,
the point is on the last `c' in "bpf_image.c", instead of being on
the `b', but I think this is because of some Dired magic which gets
confused by the error.
The fun part is that it does not only happen in Dired buffers: in
this very Message buffer where I'm composing this post, I can move up
to the top with C-p, but when I move down to the bottom with C-n, I
get an "End of buffer" error over the pasted Dired line.
This bug does not appear in a fresh Emacs session just after I
launch it, it appears only after I've used Emacs a bit.
This only happens in console frames, in X frames I never managed to
reproduce these bugs. They are _very_ annoying since it makes the
display flash the bell every once in a while even though no error
occurred...
I can provide more information if needed.
--
Romain Francoise <romain@orebokech.com> | You know that old saying,
it's a miracle -- http://orebokech.com/ | that you always hurt the ones
| you love? Well it works both
| ways.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: scroll-conservatively overflow
2004-04-16 13:17 ` Kim F. Storm
2004-04-16 14:03 ` Romain Francoise
@ 2004-04-17 19:42 ` Richard Stallman
2004-04-17 20:52 ` Juanma Barranquero
1 sibling, 1 reply; 13+ messages in thread
From: Richard Stallman @ 2004-04-17 19:42 UTC (permalink / raw)
Cc: jmbarranquero, emacs-devel
> That used to work; now it doesn't (although I'm not sure it is because
> of a recent change or I just took a time to notice), because
> scroll-conservatively gets multiplied by FRAME_LINE_HEIGHT (f) in a
> couple of places, and that causes an overflow.
That change was made almost a year ago...
Multiplying by FRAME_LINE_HEIGHT is right in principle, but the
overflow is a bug. Perhaps we should use long long for that
multiplication, and if the result is bigger than the largest positive
integer, reduce it to the largest positive integer.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: scroll-conservatively overflow
2004-04-17 19:42 ` Richard Stallman
@ 2004-04-17 20:52 ` Juanma Barranquero
2004-04-18 21:31 ` Kim F. Storm
2004-04-19 11:37 ` Richard Stallman
0 siblings, 2 replies; 13+ messages in thread
From: Juanma Barranquero @ 2004-04-17 20:52 UTC (permalink / raw)
On Sat, 17 Apr 2004 15:42:07 -0400, Richard Stallman <rms@gnu.org> wrote:
> Perhaps we should use long long for that
> multiplication, and if the result is bigger than the largest positive
> integer, reduce it to the largest positive integer.
Well, that's almost what my patch does, with the difference that for
large values of scroll-conservatively it won't produce the largest
positive integer, but
int(most-positive-fixnum / FRAME_LINE_HEIGHT (f)) * FRAME_LINE_HEIGHT (f)
but the two are equivalent for all practical purposes.
/L/e/k/t/u
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: scroll-conservatively overflow
2004-04-17 20:52 ` Juanma Barranquero
@ 2004-04-18 21:31 ` Kim F. Storm
2004-04-19 11:37 ` Richard Stallman
1 sibling, 0 replies; 13+ messages in thread
From: Kim F. Storm @ 2004-04-18 21:31 UTC (permalink / raw)
Cc: emacs-devel
Juanma Barranquero <lektu@mi.madritel.es> writes:
> On Sat, 17 Apr 2004 15:42:07 -0400, Richard Stallman <rms@gnu.org> wrote:
>
> > Perhaps we should use long long for that
> > multiplication, and if the result is bigger than the largest positive
> > integer, reduce it to the largest positive integer.
>
> Well, that's almost what my patch does, with the difference that for
> large values of scroll-conservatively it won't produce the largest
> positive integer, but
>
> int(most-positive-fixnum / FRAME_LINE_HEIGHT (f)) * FRAME_LINE_HEIGHT (f)
>
> but the two are equivalent for all practical purposes.
Yes, IMO your patch is good enough in practice.
--
Kim F. Storm <storm@cua.dk> http://www.cua.dk
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: scroll-conservatively overflow
2004-04-17 20:52 ` Juanma Barranquero
2004-04-18 21:31 ` Kim F. Storm
@ 2004-04-19 11:37 ` Richard Stallman
1 sibling, 0 replies; 13+ messages in thread
From: Richard Stallman @ 2004-04-19 11:37 UTC (permalink / raw)
Cc: emacs-devel
> Perhaps we should use long long for that
> multiplication, and if the result is bigger than the largest positive
> integer, reduce it to the largest positive integer.
Well, that's almost what my patch does,
Ok, thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: scroll-conservatively overflow
2004-04-16 14:03 ` Romain Francoise
@ 2004-04-30 11:35 ` Romain Francoise
2004-04-30 16:38 ` Kim F. Storm
2004-05-01 17:50 ` Richard Stallman
0 siblings, 2 replies; 13+ messages in thread
From: Romain Francoise @ 2004-04-30 11:35 UTC (permalink / raw)
[-- Attachment #1: Type: text/plain, Size: 1050 bytes --]
Romain Francoise <romain@orebokech.com> writes:
> About scrolling: I've been bitten by a bug for some weeks now, I
> originally thought it was specific to the multi-tty code but I
> discovered this morning that it's not. (A fresh checkout from CVS HEAD
> this morning exhibited the same issues.)
I have finally found enough time to track it down. The change
responsible for all my scrolling problems is Richard's change to
indent.c on March 2nd:
2004-03-02 Richard M. Stallman <rms@gnu.org>
* indent.c (compute_motion): Save vpos in prev_vpos, like hpos etc.
Reverting this change (see attached patch) solves all issues in Dired,
Gnus, etc.
I have tried to come up with a fix but failed. I don't really
understand why this change was necessary, it adds complexity and does
not fix anything as far as I can see. If it was just a code cleanup,
maybe the best would be to revert it for the time being.
--
Romain Francoise <romain@orebokech.com> | This is a man's man's man's
it's a miracle -- http://orebokech.com/ | world. --James Brown
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: emacs-indent-fix.diff --]
[-- Type: text/x-patch, Size: 2421 bytes --]
Index: src/indent.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/indent.c,v
retrieving revision 1.160
diff -c -r1.160 indent.c
*** src/indent.c 5 Apr 2004 21:41:08 -0000 1.160
--- src/indent.c 30 Apr 2004 11:27:23 -0000
***************
*** 1197,1202 ****
--- 1197,1203 ----
= (INTEGERP (current_buffer->selective_display)
? XINT (current_buffer->selective_display)
: !NILP (current_buffer->selective_display) ? -1 : 0);
+ int prev_hpos = 0;
int selective_rlen
= (selective && dp && VECTORP (DISP_INVIS_VECTOR (dp))
? XVECTOR (DISP_INVIS_VECTOR (dp))->size : 0);
***************
*** 1224,1231 ****
int wide_column_end_hpos = 0;
int prev_pos; /* Previous buffer position. */
int prev_pos_byte; /* Previous buffer position. */
- int prev_hpos = 0;
- int prev_vpos = 0;
int contin_hpos; /* HPOS of last column of continued line. */
int prev_tab_offset; /* Previous tab offset. */
--- 1225,1230 ----
***************
*** 1274,1280 ****
pos = prev_pos;
pos_byte = prev_pos_byte;
hpos = prev_hpos;
- vpos = prev_vpos;
tab_offset = prev_tab_offset;
}
break;
--- 1273,1278 ----
***************
*** 1384,1390 ****
if (pos >= next_boundary)
next_boundary = pos + 1;
prev_hpos = width;
- prev_vpos = vpos;
prev_tab_offset = tab_offset;
}
}
--- 1382,1387 ----
***************
*** 1417,1423 ****
pos = prev_pos;
pos_byte = prev_pos_byte;
hpos = prev_hpos;
- vpos = prev_vpos;
tab_offset = prev_tab_offset;
/* NOTE on contin_hpos, hpos, and prev_hpos.
--- 1414,1419 ----
***************
*** 1438,1443 ****
--- 1434,1443 ----
hpos = contin_hpos;
vpos = vpos - 1;
}
+ else if (c == '\n')
+ /* If previous character is NEWLINE,
+ set VPOS back to previous line */
+ vpos = vpos - 1;
break;
}
***************
*** 1455,1461 ****
pos = prev_pos;
pos_byte = prev_pos_byte;
hpos = prev_hpos;
- vpos = prev_vpos;
tab_offset = prev_tab_offset;
}
break;
--- 1455,1460 ----
***************
*** 1464,1470 ****
break;
prev_hpos = hpos;
- prev_vpos = vpos;
prev_pos = pos;
prev_pos_byte = pos_byte;
wide_column_end_hpos = 0;
--- 1463,1468 ----
[-- Attachment #3: Type: text/plain, Size: 141 bytes --]
_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: scroll-conservatively overflow
2004-04-30 11:35 ` Romain Francoise
@ 2004-04-30 16:38 ` Kim F. Storm
2004-05-01 17:50 ` Richard Stallman
1 sibling, 0 replies; 13+ messages in thread
From: Kim F. Storm @ 2004-04-30 16:38 UTC (permalink / raw)
Romain Francoise <romain@orebokech.com> writes:
> Romain Francoise <romain@orebokech.com> writes:
>
> > About scrolling: I've been bitten by a bug for some weeks now, I
> > originally thought it was specific to the multi-tty code but I
> > discovered this morning that it's not. (A fresh checkout from CVS HEAD
> > this morning exhibited the same issues.)
>
> I have finally found enough time to track it down. The change
> responsible for all my scrolling problems is Richard's change to
> indent.c on March 2nd:
>
> 2004-03-02 Richard M. Stallman <rms@gnu.org>
>
> * indent.c (compute_motion): Save vpos in prev_vpos, like hpos etc.
>
> Reverting this change (see attached patch) solves all issues in Dired,
> Gnus, etc.
>
> I have tried to come up with a fix but failed. I don't really
> understand why this change was necessary, it adds complexity and does
> not fix anything as far as I can see. If it was just a code cleanup,
> maybe the best would be to revert it for the time being.
The change was made in response to the following bug report:
The fix was obviously not correct.
Please describe exactly what actions triggered the bug
and the precise symptoms of the bug:
Place the following in a file named `/tmp/x'
======cut here=====
Barf
Whee!
Yadda
======cut here=====
(the above contains three lines, no tabs, and no trailing spaces, for
a total of 27 characters)
Now do the following:
* start `emacs -q --no-site-file'
* find the file: C-x C-f /tmp/x RET
* turn on selective-display: C-u C-x $
* attempt to move point forward: C-n C-n
Note that the cursor did not appear to move after the second C-n.
That's the bug; it should have appeared on the third visible line
(right before the `Y').
* sanity check: C-x =
You should see point 6 of 27.
* again attempt to move point forward: C-n
* again sanity check: C-x =
It's still at 6 of 27.
If it matters, you can "un-stick" point by typing C-f, *then* typing
C-n; in that case, point indeed winds up on the third visible line,
where I'd expect it to.
--
Kim F. Storm <storm@cua.dk> http://www.cua.dk
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: scroll-conservatively overflow
2004-04-30 11:35 ` Romain Francoise
2004-04-30 16:38 ` Kim F. Storm
@ 2004-05-01 17:50 ` Richard Stallman
2004-05-02 12:37 ` Romain Francoise
1 sibling, 1 reply; 13+ messages in thread
From: Richard Stallman @ 2004-05-01 17:50 UTC (permalink / raw)
Cc: emacs-devel
2004-03-02 Richard M. Stallman <rms@gnu.org>
* indent.c (compute_motion): Save vpos in prev_vpos, like hpos etc.
Reverting this change (see attached patch) solves all issues in Dired,
Gnus, etc.
I have tried to come up with a fix but failed. I don't really
understand why this change was necessary, it adds complexity and does
not fix anything as far as I can see.
I made that change to fix a specific bug. That function was returning
incorrect results in a certain case. I don't remember the test case,
unfortunately.
This change also makes the function cleaner because it handles vpos
like the various other values.
Apparently the new code has another bug. That often happens. Could
you figure out precisely what the bug is? Then we could find the
right fix.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: scroll-conservatively overflow
2004-05-01 17:50 ` Richard Stallman
@ 2004-05-02 12:37 ` Romain Francoise
2004-05-02 16:58 ` Stefan Monnier
2004-05-03 14:03 ` Richard Stallman
0 siblings, 2 replies; 13+ messages in thread
From: Romain Francoise @ 2004-05-02 12:37 UTC (permalink / raw)
Cc: no-spam, emacs-devel
Richard Stallman <rms@gnu.org> writes:
> I made that change to fix a specific bug. That function was returning
> incorrect results in a certain case. I don't remember the test case,
> unfortunately.
Kim F. Storm kindly provided me with the original bug report so I've
been able to reproduce it.
> This change also makes the function cleaner because it handles vpos
> like the various other values.
Yes; I wasn't convinced at first that doing this was necessary, but now
that I have looked into it in more detail I agree that it is cleaner
this way.
> Apparently the new code has another bug. That often happens. Could
> you figure out precisely what the bug is? Then we could find the
> right fix.
I have spent some time reading the code and found the problem: you had
overlooked a case where vpos needs to be saved in prev_vpos when dealing
with continuation lines. Your change did not save the value even though
it had been incremented to take into account the fact that the line is
continued on another line, thus making the next iteration restore vpos
from prev_vpos with a wrong value, which caused all my scrolling
problems in Gnus and Dired.
I suggest the following patch against CVS which merely saves vpos in
prev_vpos in that case, and that fixes the various problems I was
having. I have also checked that the original bug is still fixed with
that change. Could you please install it?
Index: src/indent.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/indent.c,v
retrieving revision 1.160
diff -c -r1.160 indent.c
*** src/indent.c 5 Apr 2004 21:41:08 -0000 1.160
--- src/indent.c 2 May 2004 11:54:42 -0000
***************
*** 1407,1412 ****
--- 1407,1413 ----
vpos++;
contin_hpos = prev_hpos;
prev_hpos = 0;
+ prev_vpos = vpos;
}
}
Suggested changelog entry:
2004-05-02 Romain Francoise <romain@orebokech.com> (tiny change)
* indent.c (compute_motion): Save vpos in prev_vpos when dealing
with continuation lines, too.
Thanks,
--
Romain Francoise <romain@orebokech.com> | This is a man's man's man's
it's a miracle -- http://orebokech.com/ | world. --James Brown
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: scroll-conservatively overflow
2004-05-02 12:37 ` Romain Francoise
@ 2004-05-02 16:58 ` Stefan Monnier
2004-05-03 14:03 ` Richard Stallman
1 sibling, 0 replies; 13+ messages in thread
From: Stefan Monnier @ 2004-05-02 16:58 UTC (permalink / raw)
Cc: emacs-devel, no-spam
> 2004-05-02 Romain Francoise <romain@orebokech.com> (tiny change)
> * indent.c (compute_motion): Save vpos in prev_vpos when dealing
> with continuation lines, too.
Thank you,
Installed,
Stefan
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: scroll-conservatively overflow
2004-05-02 12:37 ` Romain Francoise
2004-05-02 16:58 ` Stefan Monnier
@ 2004-05-03 14:03 ` Richard Stallman
1 sibling, 0 replies; 13+ messages in thread
From: Richard Stallman @ 2004-05-03 14:03 UTC (permalink / raw)
Cc: no-spam, emacs-devel
Thanks for finding the new bug.
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2004-05-03 14:03 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-04-16 10:00 scroll-conservatively overflow Juanma Barranquero
2004-04-16 13:17 ` Kim F. Storm
2004-04-16 14:03 ` Romain Francoise
2004-04-30 11:35 ` Romain Francoise
2004-04-30 16:38 ` Kim F. Storm
2004-05-01 17:50 ` Richard Stallman
2004-05-02 12:37 ` Romain Francoise
2004-05-02 16:58 ` Stefan Monnier
2004-05-03 14:03 ` Richard Stallman
2004-04-17 19:42 ` Richard Stallman
2004-04-17 20:52 ` Juanma Barranquero
2004-04-18 21:31 ` Kim F. Storm
2004-04-19 11:37 ` Richard Stallman
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.