Snapshots for CURRENT (buffer_set issue?)

[Julien ÉLIE]

Hi,

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");
>     fdreserve(4);
> -    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)").

In tests/innd/chan-t.c:

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

But since:

    #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?)

-- 
Julien ÉLIE

« Vina bibant homines, animantia cetera fontes. »