Snapshots for CURRENT (buffer_set issue?)
julien at trigofacile.com
Tue Aug 21 13:33:56 UTC 2007
I believe I have found the problem here.
> Index: artparse-t.c
> --- artparse-t.c (révision 7651)
> +++ artparse-t.c (copie de travail)
> @@ -94,7 +94,8 @@
> if (Log == NULL)
> sysdie("Cannot open /dev/null");
> - buffer_set(&Path, "", 1);
> + buffer_set(&Path, "news.example.com!", 17);
18 in fact, in order to also have '\0' copied.
> /* Get our Path name, kill trailing !. */
> ARTpathme = xstrdup(Path.data);
> ARTpathme[Path.used - 1] = '\0';
> Perhaps we ought to make sure that Path.used is not 0
> with an ASSERT here in art.c?
It is in fact not necessary since Path.used is prepared with:
Path.used = strlen(innconf->pathhost) + 1;
so it should not be 0 when it is currently used (well, except for the
test suite which is the second location where it is used).
> Now, what strikes me most is that there is a new error in valgrind
> when I run the test:
> ==29891== Conditional jump or move depends on uninitialised value(s)
> ==29891== at 0x401E215: strlen (mc_replace_strmem.c:246)
> ==29891== by 0x8094506: x_strdup (xmalloc.c:127)
> ==29891== by 0x8056EB9: ARTsetup (art.c:286)
> ==29891== by 0x8051741: main (artparse-t.c:99)
Path.data now finishes with a '\0' so strlen can end up silently.
> And it looks the same as what there is in chan.t (the test which currently
> makes snapshots fail, with "strcmp (mc_replace_strmem.c:341)").
- WCHANset(cp, "some output", strlen("some output"));
+ WCHANset(cp, "some output", strlen("some output")+1);
permits to suppress valgrind error. (But some tests then fail because
the length is wrong. So it is not a decent solution.)
#define WCHANset(cp, p, l) buffer_set(&(cp)->Out, (p), (l))
I believe this is a deeper issue with buffer_set.
Shouldn't it check whether it NULL-terminates strings?
(And perhaps also other buffer_* functions?)
« Vina bibant homines, animantia cetera fontes. »
More information about the inn-workers