Red Hat Bugzilla – Bug 805735
Foption patch depends on uninitialised variable when reading from stdin and also implies -X option
Last modified: 2015-03-04 18:57:55 EST
The patch in the F17 and rawhide less packages to fix up Foption is broken in a couple different ways.
I don't personally use Fedora, but I noticed this while reviewing your patches for inclusion in Mageia.
Here is a description I previously sent to Vojtech Vitek. He requested I open a bug to track this.
I was just debugging some annoying behaviour in "less" last night in
Mageia. Years ago (c. 2005) we, ahem, borrowed the Foption patch from
As I periodically look for improvements etc, I checked the current
Fedora version of this patch.
In my use case however, I could not make it work.
I wanted to use -FR as my default options, but no matter what I tried,
the -F option always seemed to also imply the -X option too.
The -X option sadly has the side effect of breaking mouse scroll wheel
integration while using less under e.g. gnome-terminal.
So I dug into the code a little bit and found two bugs with the above patch.
The first is that the line count function always returned 0. It seems
that position(TOP) call always returned the NULL_POSITION thus never
counted any lines.
Secondly the value for line_count was only initialised when working with
files, not when working with stdin, so the behaviour in that case would
have been non-deterministic based on whatever random memory value was in
The patch should fix it up and seems to work for me:
Any further review would be appreciated.
thank you for your patch. I've reviewed it briefly and tested as well.
I've merged the changes into the original Foption patch and created the Foption.v2.patch, see http://pkgs.fedoraproject.org/gitweb/?p=less.git;a=blob;f=less-444-Foption.v2.patch;h=eb9140da8e4714e2dc7e423f56416992fd315c5f;hb=47da4db53ff5f00cfa893e05c9995959b8491242
Let's see how the Fedora community accepts the -F behavior now while testing Rawhide.
Annoyingly my colleague pointed out a small problem with my patch...
It relates to low-flow data...
(i=1; while [ $i -lt 50 ]; do echo "This is line $i"; i=$((i + 1)); sleep 0.5; done) | less
(i=1; while [ $i -lt 50 ]; do echo "This is line $i"; i=$((i + 1)); sleep 0.5; done) | less -F
In the latter case it waits for 0.5*21 = 11s before displaying any of the data.
Due to the kind of data I work with and the places where -F is used, I personally don't have a problem with this, but there should really be more code added to cope with this kind of situation :s
(PS I was rounding up before you point out my lack of math skills :p)
(In reply to comment #2)
> (i=1; while [ $i -lt 50 ]; do echo "This is line $i"; i=$((i + 1)); sleep 0.5;
> done) | less -F
> In the latter case it waits for 0.5*21 = 11s before displaying any of the data.
I can't reproduce this. Could you please test the update packages, eg. less-444-7.fc18? Thanks!
Hmm, I don't have an handy Fedora install sadly, but I tested the patch itself and I can still reproduce it with that applied rather than the previous two patches I had. We do however have a newer version of less than you guys (445 vs 444) so that may explain the difference in behaviour.
That said, I can quite easily see in the patched code how this happens: The get_line_count() function will always count up to as many lines as the screen is high. If the input is coming from stdin, and there has not yet been an EOF, it will wait until that many lines exist, thus delaying display of ANY output until the screen is full.
In an ideal world it would start displaying (almost) immediately and only switch to the scrolly output when the screen is full.
Anyway, I'm using this as a "less" bug tracker rather than a fedora one so that's likely not overly helpful to you. I can bow out of the discussion if you like (especially if you cannot reproduce)?