So today I'm a little bit lazy and I've got the day off work. As my previous plan suggested I wanted to spend at least some of the day tackling semi-random bugs. Earlier I picked a victim: less.
less rocks, and I use it daily. I even wrote an introduction to less once upon a time.
So lets take a look at two bugs from the long-neglected pile. These two issues are basically the same:
They seem like simple ones to fix, with the same root cause. Here's an example if you want to play along at home:
cp /dev/null testing gzip testing zless testing.gz
What do you see? I see this:
"testing.gz" may be a binary file. See it anyway?
When I select "y" I see the raw binary of the compressed file.
So, we can reproduce it. Now to see why it happens. /bin/zless comes from the gzip package and is a simple shell script:
#!/bin/sh # snipped a lot of text LESSOPEN="|gzip -cdfq -- %s"; export LESSOPEN exec less "$@"
So what happens if we run that?
$ LESSOPEN="|gzip -cdfq -- ~/testing.gz" /usr/bin/less ~/testing.gz "/home/skx/testing.gz" may be a binary file. See it anyway?
i.e. it fails in the same way. Interestingly this works just fine:
gzip -cdfq -- ~/testing.gz | less
So we've learnt something interesting and useful. We've learnt that when LESSOPEN is involved we get the bug. Which suggests we should "apt-get source less" and then "rgrep LESSOPEN ~/less-*/".
Doing so reveals the following function in filename.c:
public char *
open_altfile(filename, pf, pfd)
char *filename;
int *pf;
void **pfd;
{
/* code to test whether $LESSOPEN is set, and attempt to run the
command if it is */
/*
* Read one char to see if the pipe will produce any data.
* If it does, push the char back on the pipe.
*/
f = fileno(fd);
SET_BINARY(f);
if (read(f, &c, 1) != 1)
{
/*
* Pipe is empty. This means there is no alt file.
*/
pclose(fd);
return (NULL);
}
ch_ungetchar(c);
*pfd = (void *) fd;
*pf = f;
return (save("-"));
That might not be digestible, but basically less runs the command specified in $LESSOPEN. If it may read a single character of output from that command it replaces the file it was going to read with the output of the command instead!
(i.e. Here less failed to read a single character, because our gzipped file was zero-bytes long! So instead it reverted to showing the binary gzipped file.)
So we have a solution: If we want this to work we merely remove the "read a single character test". I can't think of circumstance in which that would do the wrong thing, so I've submitted a patch to do that.
Bug(s) fixed.
Incidentally if you like these kind of "debuggin by example" posts, or hate them, do let me know. So I'll know whether to take notes next time or not..
Tags: bugs, debian, less, public bugfixing 22 comments
