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

From:

Jim Meyering

Subject:

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

Date:

Wed, 04 Jun 2008 18:51:45 +0200

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