Re: cygwin build problem with m4 HEAD

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Ralf Wildenhues on 9/9/2005 1:31 AM:
>>ltdl.c, in lt_argz_insert, blindly calls argz_insert without checking whether 
>>the before argument is NULL.  But newlib (up until my patch posted there 
>>today 
>>is incorporated, http://sources.redhat.com/ml/newlib/2005/msg00516.html) 
>>treated this as EINVAL, thus breaking load_deplibs() and failing every single 
>>testcase of the m4 testsuite because of a failure to initialize.
> 
> Thank you so much for analyzing this and providing a fix, I owe you a
> beer for that one!
> 
> Your fix is not quite appropriate for inclusion, however, because our
> replacement argz.c does not provide argz_add.  Does newlib have a
> similar bug in argz_append?

No - the newlib bug in argz_insert was that it called argz_add and
correctly appended the string, but then a missing return statement made it
fall through to the next line which checked for before being out of range,
such that it returned EINVAL on success, and overwrote the proper ENOMEM
on failure.  Adding the return statement fixed newlib, although that means
there are systems in the wild (such as cygwin 1.5.18) with the bug based
on when they compiled newlib.  I checked newlib's argz_append, and it
works just fine.

> If not, then the patch below should work as
> well, I believe (untested with newlib), and is less work than providing
> argz_add as well.  Could you confirm this?  If ok, I'll apply this CVS
> HEAD and backport to branch-1-5.

Don't apply the patch as is - I've thought about it a bit more.  I think a
better patch, in the spirit of gnulib, is to update m4/argz.m4 to do a
configure-time run test to see if the system argz_insert is broken, in
which case libtool's argz_insert is used instead.  Leave libtool's ltdl.c
alone - no need to dirty it with workarounds that can be solved by fixing
argz_insert instead.  Besides, if any other project uses gnulib's argz
module, they should get correct behavior as well.  Unfortunately, I
imagine that such a fix to the argz.m4 file will have to be pessimistic
during cross-compiles, and is further complicated by the fact that argz.c
should probably provide only a replacement argz_insert that falls back on
the system argz_append if it was only the system argz_insert that is
broken.  However, my employer has not signed a copyright disclaimer,
despite my repeated requests (although I'm making progress, and hope to
get it one of these days), and the amount of m4 magic needed to add a
runtime check and improve argz.c is most likely beyond the trivial patch
limits.

> 
> By the way, how does the m4 testsuite fare on cygwin with this fixed?

It dropped from 76 failures (ie. every single test) down to 9 failures
(I'll post further info to the m4 lists).

> 
> Cheers,
> Ralf
> 
>       * libltdl/ltdl.c (lt_argz_insert): Work around newlib argz_insert bug.
>         * Makefile.am (VERSION_INFO): Bumped revision.
>         Reported by Eric Blake <address@hidden>.
> 
> Index: Makefile.am
> ===================================================================
> RCS file: /cvsroot/libtool/libtool/Makefile.am,v
> retrieving revision 1.161
> diff -u -r1.161 Makefile.am
> --- Makefile.am       5 Sep 2005 06:21:48 -0000       1.161
> +++ Makefile.am       9 Sep 2005 07:21:02 -0000
> @@ -230,7 +230,7 @@
>  AM_CPPFLAGS          = -I. -I$(srcdir) -Ilibltdl -I$(srcdir)/libltdl \
>                         -I$(srcdir)/libltdl/libltdl
>  AM_LDFLAGS           = -no-undefined
> -VERSION_INFO         = -version-info 6:0:0
> +VERSION_INFO         = -version-info 6:1:0
>  
>  noinst_LTLIBRARIES   = $(LT_DLLOADERS)
>  
> Index: libltdl/ltdl.c
> ===================================================================
> RCS file: /cvsroot/libtool/libtool/libltdl/ltdl.c,v
> retrieving revision 1.231
> diff -u -r1.231 ltdl.c
> --- libltdl/ltdl.c    28 Jul 2005 10:01:03 -0000      1.231
> +++ libltdl/ltdl.c    9 Sep 2005 07:21:02 -0000
> @@ -1,5 +1,5 @@
>  /* ltdl.c -- system independent dlopen wrapper
> -   Copyright (C) 1998, 1999, 2000, 2004 Free Software Foundation, Inc.
> +   Copyright (C) 1998, 1999, 2000, 2004, 2005 Free Software Foundation, Inc.
>     Originally by Thomas Tanner <address@hidden>
>  
>     NOTE: The canonical source of this file is maintained with the
> @@ -1445,7 +1445,12 @@
>  {
>    error_t error;
>  
> -  if ((error = argz_insert (pargz, pargz_len, before, entry)))
> +  if (before) 
> +    error = argz_insert (pargz, pargz_len, before, entry);
> +  else
> +    error = argz_append (pargz, pargz_len, entry, 1 + strlen (entry));
> +
> +  if (error)
>      {
>        switch (error)
>       {
> 

- --
Life is short - so eat dessert first!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFDIXKS84KuGfSFAYARAr/WAJ9+otyfidVTsHWCLtPOUOqEqq/qSACdF4w4
KIRsezZCrcvJ/3dD3kIv90k=
=Ejrm
-----END PGP SIGNATURE-----