* 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 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
* 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
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 public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).