Re: [PATCH 2/2] Add missing argz_* functions from glibc

[Jim Meyering]

David Lutterkort <address@hidden> wrote:
> * argz.c (argz_add_sep, argz_create, argz_create_sep, argz_replace,
>   argz_delete): import almost verbatim from glibc-2.7; only changes are
>   additional asserts and renaming of __ functions to public interface

Hi David,

Adding assertions is nice if you're sure you'll always use
the function in a context where an abort is acceptable,
but these argz functions may be called e.g., from libraries.
Of course, you could add the additional requirement to compile
with -DNDEBUG, but that's pretty onerous, and would make this
version of argz incompatible with the glibc version.

Only after writing the above did I go look at the unmodified
code in gnulib's argz.c, and there, I saw all of the similar,
existing uses of assert.  Now I see why you've done this.

Adding assertions like that is at odds with gnulib's policy of trying
to avoid gratuitous differences between code we import and the upstream
source.

For example, calling argz_replace with str=NULL and e.g., argz=NULL
succeeds using libc's version, yet evokes a failed assertion
with the one below.  One might argue that such a nonsensical call
deserves the abort, but...

It'd be nice to sync this code from glibc automatically.

> +argz_add_sep (char **argz, size_t *argz_len, const char *string, int delim)
> +{
> +  size_t nlen;
> +
> +  assert(argz);
> +  assert(argz_len);
> +  assert(string);
...