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

[Ralf Wildenhues]

Hi David,

* David Lutterkort wrote on Thu, May 29, 2008 at 01:06:50AM CEST:
> * 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
> * argz.m4: check for newly added functions
> * modules/argz: new dependencies on mempcpy, stpcpy, and strndup

> --- a/lib/argz.c
> +++ b/lib/argz.c

> +error_t
> +argz_create (char *const argv[], char **argz, size_t *len)
> +{
> +  int argc;
> +  size_t tlen = 0;
> +  char *const *ap;
> +  char *p;
> +
> +  for (argc = 0; argv[argc] != NULL; ++argc)
> +    tlen += strlen (argv[argc]) + 1;
> +
> +  if (tlen == 0)
> +    *argz = NULL;
> +  else
> +    {
> +      *argz = malloc (tlen);
> +      if (*argz == NULL)
> +     return ENOMEM;
> +
> +      for (p = *argz, ap = argv; *ap; ++ap, ++p)
> +     p = __stpcpy (p, *ap);

Any reason this wasn't changed to stpcpy?

> +    }
> +  *len = tlen;
> +
> +  return 0;
> +}
[...]
> +error_t
> +argz_replace (char **argz, size_t *argz_len, const char *str, const char 
> *with,
> +             unsigned int *replace_count)
> +{
[...]
> +               if (delayed_copy)
> +                 /* We avoided copying SRC to DST until we found a match;
> +                       now that we've done so, copy everything from the start
> +                       of SRC.  */
> +                 {
> +                   if (arg > src)
> +                     err = __argz_append (&dst, &dst_len, src, (arg - src));

Shouldn't this be argz_append?  How come you don't get a link error
with this?

> +                   delayed_copy = 0;
> +                 }
> +               if (! err)
> +                 err = __argz_add (&dst, &dst_len, to);

Likewise, argz_add.

> +               free (to);
> +             }
> +           else
> +             err = ENOMEM;
> +
> +           if (replace_count)
> +             (*replace_count)++;
> +         }
> +       else if (! delayed_copy)
> +         err = __argz_add (&dst, &dst_len, arg);

Likewise, argz_add.

> +     }
[...]
> --- a/m4/argz.m4
> +++ b/m4/argz.m4
> @@ -25,7 +25,8 @@ AC_CHECK_TYPES([error_t],
>  #endif])
>  
>  ARGZ_H=
> -AC_CHECK_FUNCS([argz_add argz_append argz_count argz_create_sep argz_insert \
> +AC_CHECK_FUNCS([argz_add argz_add_sep argz_append argz_count \
> +        argz_create_sep argz_insert argz_replace argz_delete \
>       argz_next argz_stringify], [], [ARGZ_H=argz.h; AC_LIBOBJ([argz])])

Sigh, this test gets slower and slower... :-/

I'm leaning towards accepting this patch once the above issues and Jim's
comments are addressed.  Still thinking about splitting argz.c into one
file per function, though; and I guess testsuite exposure would be a
nice thing to have, too (not that these are your tasks).

Cheers, and thanks,
Ralf