Re: bug in variable.c: fontpath_handler(ACTION_GET)

[Hans-Bernhard B. <br...@ph...>]

On Mon, 24 Nov 2003, Petr Mikulik wrote:

> ==30824== Invalid read of size 1
> ==30824==    at 0x4016371B: strlen (in /usr/lib/valgrind/valgrind.so)
> ==30824==    by 0x80E88BC: fontpath_handler (variable.c:519)

A similar bug seems to be in loadpath_handler, too, where it's easier
to trigger: just give the commands

	test pal
	test pal

in a fresh gnuplot session that does have a loadpath from the environment
($GNUPLOT_LIB is set), and the second one will cause a SIGSEGV in
loadpath_handler, in a line that's in exactly the same context as line
variable.c:519 which valgrind complained about:

(gdb) cont
Continuing.

Program received signal SIGSEGV, Segmentation fault.
0x080dae74 in loadpath_handler (action=16, path=0x0)
    at ../../src/variable.c:175
175                 p += strlen(p);
(gdb) l
170                 if (p < limit)
171                     return p;
172                 else
173                     return NULL;
174             } else {
175                 p += strlen(p);
176                 /* skip over '\0' */
177                 p++;
178                 if (p < limit)
179                     return p;

So the real problem obviously is that 'beenhere' wasn't reset to zero,
even though the state of 'p' suggests it has.

And the reason for that is pretty clear, too: fontpath_handler() and
loadpath_handler() both expect the caller to *always* do ACTION_GET until
it gets a NULL return.  The code in 'test palette' doesn't do that, and
thus it crashes.  The right fix would thus be to set 'beenhere' to zero
whenever p is NULLed, i.e. on each re-initialization of fontpath_handler()
or loadpath_handler, respectively.

Actually 'beenhere' serves no useful purpose anyway, as far as I can see.
Tests for it can be replaced by the condition (p != NULL), I think.

I'm checking in a fix along these lines.

-- 
Hans-Bernhard Broeker (br...@ph...)
Even if all the snow were burnt, ashes would remain.