Re: cygwin build problem with m4 HEAD
-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 [EMAIL PROTECTED].
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 - 1.161
+++ Makefile.am 9 Sep 2005 07:21:02 -
@@ -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.c28 Jul 2005 10:01:03 - 1.231
+++ libltdl/ltdl.c9 Sep 2005 07:21:02 -
@@ -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 [EMAIL PROTECTED]
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 [EMAIL PROTECTED]
-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-
Re: cygwin build problem with m4 HEAD
[ taking libtool-patches back out ] Hi Eric, * Eric Blake wrote on Fri, Sep 09, 2005 at 01:31:30PM CEST: According to Ralf Wildenhues on 9/9/2005 1:31 AM: 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. I hate runtime checks. This one will mean hopeless complication of things, and worse results for any kind of cross compilation. And all of this for a system where people are unlikely to run year-old installations anyway. Besides, the workaround costs like what? Two lines of code. Multiply by a hundred for the test + replacement. 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. I could attempt such a patch, but do not see its value outweighing its costs, see above. In matters like these, I would _by far_ prefer a documentation of such limitations, in the spirit of the Autoconf manual. 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). Quite a relief. Thanks for checking. Cheers, Ralf * libltdl/ltdl.c (lt_argz_insert): Work around newlib argz_insert bug. * Makefile.am (VERSION_INFO): Bumped revision. Reported by Eric Blake [EMAIL PROTECTED]. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: cygwin build problem with m4 HEAD
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Ralf Wildenhues on 9/9/2005 6:46 AM: I hate runtime checks. This one will mean hopeless complication of things, and worse results for any kind of cross compilation. And all of this for a system where people are unlikely to run year-old installations anyway. Besides, the workaround costs like what? Two lines of code. Multiply by a hundred for the test + replacement. True enough - to date, only newlib has been identified with the bug, the bug is now fixed, and will show up in the next mingw and cygwin release. Maybe you could be optimistic when cross-compiling and assume there is no bug (since cross-compiling is hard enough that they are likely to have an up-to-date version of newlib). But if you don't want to do the runtime test at all, it is somewhat your call as maintainer of the gnulib argz module. 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. I could attempt such a patch, but do not see its value outweighing its costs, see above. In matters like these, I would _by far_ prefer a documentation of such limitations, in the spirit of the Autoconf manual. Well, currently, neither gnulib/lib/argz_.h nor argz.c have documentation, so that may be the best approach - just documenting all of the argz_* functions, and stating that in the spirit of portability to older versions of newlib, use argz_append instead of argz_insert(,,NULL,). By the way, I was also able to configure m4 with ac_cv_func_argz_insert=no ac_cv_type_error_t=no to force the use of gnulib's argz module in preference to newlib's system argz_* without even patching libtool. * libltdl/ltdl.c (lt_argz_insert): Work around newlib argz_insert bug. * Makefile.am (VERSION_INFO): Bumped revision. Reported by Eric Blake [EMAIL PROTECTED]. So maybe this libtool patch can be applied as a bandaid to the current cygwin 1.5.18, then in another year or so be reverted when all known sane systems that provide argz_insert have correctly accepted NULL for a while. - -- Life is short - so eat dessert first! Eric Blake [EMAIL PROTECTED] -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 iD8DBQFDIY/284KuGfSFAYARAibbAKCM7m5HHHyEBEy1lUb5EBl4mPrd3ACg0PnP +YczwIyajB+6Dv/+xPRMmbg= =RBi/ -END PGP SIGNATURE- ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: glob_.h glibc
Derek Price [EMAIL PROTECTED] writes: It's late, I'm tired. Patches actually attached now. Thanks. A few comments. First, that SYS_CDEFS_H thing is really confusing. Also, I worry that the GLOB_PREFIX thing doesn't respect the POSIX name space rules. How about if we use a new symbol __GLOB_GNULIB to address both issues? That will make the glibc glob.h file clearer. Second, I prefer the name glob-libc.h to glob-glibc.h. glibc and libc are both common names, but libc is one less character. Third, we can lessen the number of changes to libc glob.h by #defining dirent to direct as needed in glob_.h. Fourth, we can AC_REQUIRE AC_GNU_SOURCE, so that we needn't worry about __USE_GNU. In summary, how about the following patch instead? (Please note that I haven't tested it.) 2005-09-09 Derek Price [EMAIL PROTECTED] Paul Eggert [EMAIL PROTECTED] * modules/glob (Files): Add glob-libc.h. * lib/glob-libc.h: Renamed from glob_.h. The new version is taken from libc's glob.h, except with '#ifdef __GLOB_GNULIB' protecting things that should be done only in gnulib contexts. * lib/glob_.h: New file, containing only the glob things needed for gnulib. (GLOB_PREFIX): Remove. All uses changed to look for __GLOB_GNULIB. (__GLOB_CONCAT, __GLOB_XCONCAT, __GLOB_ID): Remove; no longer needed. (glob, globfree, glob_pattern_p): Now defined simply in terms of rpl_glob, rpl_globfree, rpl_glob_pattern_p. (__GLOB_GNULIB): New macro, to keep the glob.h changes clearer and to respect the namespace rules better. * m4/glob.m4 (gl_GLOB_SUBSTITUTE): AC_LIBSOURCES for glob.c, glob_.h, glob-libc.h. (gl_PREREQ_GLOB): Remove _SYS_CDEFS_H hack; no longer needed. --- lib/glob_.h 7 Jun 2005 14:58:46 - 1.3 +++ lib/glob_.h 9 Sep 2005 17:54:51 - @@ -1,33 +1,32 @@ -/* Copyright (C) 1991,92,95-98,2000,2001,2004 Free Software Foundation, Inc. - This file is part of the GNU C Library. +/* glob_.h -- Find a path matching a pattern. - The GNU C Library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 2.1 of the License, or (at your option) any later version. + Copyright (C) 2005 Free Software Foundation, Inc. - The GNU C Library is distributed in the hope that it will be useful, + Written by Derek Price [EMAIL PROTECTED] Paul Eggert [EMAIL PROTECTED] + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2, or (at your option) + any later version. + + This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software Foundation, + Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ - You should have received a copy of the GNU Lesser General Public - License along with the GNU C Library; if not, write to the Free - Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA - 02111-1307 USA. */ - -#ifndef_GLOB_H -#define_GLOB_H 1 - -/* Note the reversal of the common HAVE_SYS_CDEFS_H idiom below. In this - way, #ifndef _SYS_CDEFS_H may be used to include sys/cdefs.h both when - it has been checked for via the GNULIB configure test and found and when - it has not been checked for, which we can presume means that the glob.h - GNULIB shares with GLIBC is being included as a system header and not as - part of GNULIB, in which case sys/cdefs.h may be assumed. */ -#ifndef _SYS_CDEFS_H +#ifndef GLOB_H +#define GLOB_H 1 + +#ifdef HAVE_SYS_CDEFS_H # include sys/cdefs.h #endif + +#include stddef.h + #ifndef __BEGIN_DECLS # define __BEGIN_DECLS # define __END_DECLS @@ -36,209 +35,27 @@ # define __THROW #endif -__BEGIN_DECLS - -/* We need `size_t' for the following definitions. */ -#ifndef GLOB_PREFIX -# ifndef __size_t -# if defined __GNUC__ __GNUC__ = 2 -typedef __SIZE_TYPE__ __size_t; -# ifdef __USE_XOPEN -typedef __SIZE_TYPE__ size_t; -# endif -# else -# include stddef.h -# ifndef __size_t -#define __size_t size_t -# endif -# endif -# else -/* The GNU CC stddef.h version defines __size_t as empty. We need a real - definition. */ -# undef __size_t -# define __size_t size_t -# endif -#else /* GLOB_PREFIX */ -# include stddef.h -# undef __size_t +#ifndef __size_t # define __size_t
Re: glob_.h glibc
Derek Price [EMAIL PROTECTED] writes: Fourth, we can AC_REQUIRE AC_GNU_SOURCE, so that we needn't worry about __USE_GNU. We ran into this problem the first time we went through this: http://lists.gnu.org/archive/html/bug-gnulib/2005-05/msg00144.html. Ouch. Sorry I forgot that. I assume you fixed this by removing the AC_REQUIRE([AC_GNU_SOURCE]) from glob.m4 and restoring the condtional #define __USE_GNU 1 to glob_.h? If it's more complicated than that, please let me know. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
