Listen to me when I'm telling you

Thursday, 14 February 2008

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..

| 22 comments.

 

Comments On This Entry

[gravitar] Ondrej

Submitted at 13:40:21 on 14 february 2008

Yes, please continue posting debugging posts, it's very useful.
[gravitar] plagal

Submitted at 13:50:45 on 14 february 2008

Well, I like them. :-)
[gravitar] Michael

Submitted at 14:18:58 on 14 february 2008

Very intresting, thanks :)
[gravitar] Jonathan Ballet

Submitted at 14:23:19 on 14 february 2008

I Just wanted to say that I like these kind of posts :)
And thanks for the fix(shs) !
[gravitar] Adam

Submitted at 14:32:25 on 14 february 2008

I like these types of posts. They're very informative. Thanks for walking us through the way you diagnose, debug and fix a problem. Cheers from the U.S.!
[gravitar] Philip Stubbs

Submitted at 14:51:12 on 14 february 2008

I like these posts. I am not a coder, and it is very interesting to see how you are able to locate the exact location in the source code, and identify what it does.
The little bit of programming that I have done has had to be small enough that I can picture the whole thing in my head. Looking at other peoples code I find really hard.
One day, I would like to squash a bug :-)
[gravitar] Amaya

Submitted at 14:58:00 on 14 february 2008

I loved this "debuggin by example" post, keep them coming!
[gravitar] Jaime

Submitted at 16:09:44 on 14 february 2008

Yup, I agree too: these debugging posts are excellent. PS why "cp /dev/null testing" rather than "touch testing"? Is it just personal preference? :-)
[author] Steve

Submitted at 16:19:39 on 14 february 2008

Thanks for the (positive) feedback!

As for why the "cp /dev/null" command, rather than touch there was a similar line in the bug report. So I just adapted that.

Generally I'd use touch - actually I'm suprised I didn't change it, I guess I was too busy thinking about how to reproduce the problem with the least effort!

[gravitar] Marius Gedminas

Submitted at 16:40:40 on 14 february 2008

Removing that check breaks a documented feature of less. Have you looked at the "INPUT PREPROCESSOR" section of the less(1) manual page? Have you tested that less regularfile.txt still works when you do eval `lesspipe`?
[author] Steve

Submitted at 16:50:40 on 14 february 2008

I didn't test the documentation to see if this was expected behaviour - but I did test that otherwise things work as expected.

Certainly "eval $(lessfile)" and "eval $(lesspipe)" both continue to allow normal and .gz files to be read.

[gravitar] Jeff

Submitted at 18:55:11 on 14 february 2008

I like the debugging by example posts as well. For someone like me who would like to be involved, but isn't. They give a great sense of what being involved looks like.
[gravitar] Josh

Submitted at 19:02:28 on 14 february 2008

Great post. Please keep up the "debugging by example posts". I really want to become more involved in the community and posts like this make it evident how to get started.
Thanks, Josh
[gravitar] Saint Aardvark

Submitted at 19:27:54 on 14 february 2008

I like this post a lot. More Debian debugging, please!
[gravitar] Alex

Submitted at 23:26:29 on 14 february 2008

Call me a sadist, but I just love watching you crush those little critters. :)
[gravitar] xipmix

Submitted at 00:54:42 on 15 february 2008

This was indeed useful.
An idea I have been pondering for a while is how to make life easier for people who want to debug a more complex program. The standard response is "build a debug version" but not many end users know how to do that in a sane environment (that would allow the package maintainer to see if the problem is in the package or the environment it's being run in).
One way to address this is for debian to make -dbg packages available on request; if they're built on the buildds, they should be an exact match for the problematic package.
For extra points, one could build such packages so they provide reports automagically, along the lines of this project - http://www.cs.wisc.edu/cbi/. I have no idea how feasible this is. For one thing it has potential to vastly inflate the debian archive if not done selectively.
[gravitar] eric

Submitted at 08:34:45 on 15 february 2008

Yes, interesting post. I'm not a fan of C code but the example you provide is clear.
[gravitar] Kai

Submitted at 16:50:40 on 15 february 2008

As everyone can learn from others I'd appreciate if you would continue this sort of posts. Thanks in advance. Kind regards, Kai
[gravitar] George

Submitted at 11:09:11 on 16 february 2008

Just wanted to say that I find this interesting, so please do keep posting them. Thanks.
[gravitar] lters

Submitted at 16:33:58 on 18 february 2008

Please keep the debug posts coming!
[gravitar] Daf

Submitted at 16:39:09 on 19 february 2008

More of this sort of thing.
[gravitar] Torbjorn

Submitted at 13:23:16 on 18 february 2008

I find your debugging by example posts very interesting, and very easy to learn from .. please keep them coming.

 

Comments are closed on posts which are more than ten days old.

Recent Posts

Recent Tags