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

[Jim Meyering]

David Lutterkort <address@hidden> wrote:
> On Wed, 2008-06-04 at 00:57 +0200, Jim Meyering wrote:
>> David Lutterkort <address@hidden> wrote:
>> >
>> > I've given your patch a quick spin, and ran into the following issues:
>> >
>> >       * __attribute_pure__ in argz.h is not defined
>> >       * all the prototypes in argz.h are duplicated (you need to remove
>> >         the __ prototypes, not rewrite them to ones without __)
>> >       * dependencies of the argz module on mempcpy, stpcpy, strndup, and
>> >         strnlen are missing
>> >       * m4/argz.m4 doesn't check for the new functions added from libc;
>> >         it only checks and sets HAVE_ macros for the old ones.
>> >
>> > Apart from these, it works nicely, and I managed to build my test
>> > program on FreeBSD and have it pass all its tests.
>>
>> Thanks for the feedback!
>> I've fixed the first three things.
>
> And with this patch, things work perfectly in my tests.
>
>> I did notice the tests for the 7 existing symbols,
>> but was reluctant to add the new ones since all functions go
>> back so far.  I checked and see that argz_replace was
>> added about 11 years ago, before glibc-2.0.95, so I doubt
>> any useful system will have all of the existing 7 but lack
>> any of the remaining functions.  So, for now at least,
>> I'm inclined to leave that test as is.
>
> Yeah, it seems silly to test for all of them; wouldn't it make sense to
> check for just one of them (the last one added) ?

argz_replace is the one with the most recent first log entry:

  $ for i in $(echo $f); do \
  printf '%10s: ' $i; git log string/argz-$i.c|grep '^Date'|tail -1 ;done
      append: Date:   Mon Apr 29 05:14:22 1996 +0000
      addsep: Date:   Mon Dec 2 04:00:15 1996 +0000
       ctsep: Date:   Fri May 3 17:43:42 1996 +0000
      insert: Date:   Mon Apr 29 05:14:22 1996 +0000
        next: Date:   Fri May 3 17:43:48 1996 +0000
   stringify: Date:   Mon Apr 29 05:14:22 1996 +0000
       count: Date:   Mon Apr 29 05:14:22 1996 +0000
     extract: Date:   Mon Apr 29 05:14:22 1996 +0000
      create: Date:   Mon Apr 29 05:14:22 1996 +0000
      delete: Date:   Mon Apr 29 05:14:22 1996 +0000
     replace: Date:   Thu Jun 19 18:38:51 1997 +0000

So, yes, it'd be nice to change it to check only that one function,
but there's the risk that some package depends on the side effect of
those HAVE_ symbols being defined.

Using google code search for

  #if.*HAVE_ARGZ_(APPEND|ADD|CREATE_SEP|INSERT|NEXT|STRINGIFY|COUNT)

finds fewer than 200 matches, and most are in ltdl-related and
config.h.in (i.e., auto-generated) code, as expected, so it's
probably safe enough, since they're not using the gnulib module.

If no one objects, I'll include the following change in my patch,
via this incremental addition:

>From d7c0e56e86cc1d8032e098e1af75a042916bab65 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Wed, 4 Jun 2008 12:23:41 +0200
Subject: [PATCH] argz.m4: check for only one function, argz_replace

* m4/argz.m4: Check only for the existence of one function,
argz_replace, since it seems to have been added most recently.
Also, remove the side effect of defining HAVE_ARGZ_* symbols.
---
 m4/argz.m4 |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/m4/argz.m4 b/m4/argz.m4
index 3992ab6..bed025a 100644
--- a/m4/argz.m4
+++ b/m4/argz.m4
@@ -27,8 +27,7 @@ AC_CHECK_TYPES([error_t],
 #endif])

 ARGZ_H=
-AC_CHECK_FUNCS([argz_add argz_append argz_count argz_create_sep argz_insert \
-       argz_next argz_stringify], [], [ARGZ_H=argz.h; AC_LIBOBJ([argz])])
+AC_CHECK_FUNC([argz_replace], [], [ARGZ_H=argz.h; AC_LIBOBJ([argz])])

 dnl if have system argz functions, allow forced use of
 dnl libltdl-supplied implementation (and default to do so
--
1.5.6.rc1.2.g5648b