Bug 1060

Created attachment 552 [details]
sysdeps/generic/glob.c merge from gnulib (part 1 of 3)

The LOGIN_NAME_MAX macro introduced by this patch is already defined with
another value by bits/local_lim.h

Could you please consider renaming LOGIN_NAME_MAX?

Created attachment 617 [details]
Revised patch.

How is this?  I replaced occurrances of LOGIN_NAME_MAX with GET_LOGIN_NAME_MAX.

Yes, renaming LOGIN_NAME_MAX is fine.  I've installed the revised
patch with GET_LOGIN_NAME_MAX into gnulib.  Thanks.

Do prototypize the defns for external functions such as glob itself.
Using K&R defns ensures maximal warnings for missing decls.

The removal of old portability cruft in glob.c and replacement with gnulib cruft
is fine.  Send a patch just doing that.

The glob.h changes are way too much cruft in the installed header.
Users of gnulib glob can include whatever gnulib headers they need before glob.h.
 

Created attachment 655 [details]
Revised patch.

It looks like you are right, and most of our changes to glob.h could be moved
into a pre-included header.  If it turns out anything needs to stay in there,
I'll open another issue.

I've attached a revised patch containing only the changes to glob.c.

Thanks again,

Created attachment 657 [details]
improved patch to match current sources and use old-style defns

Roland asked that we use old-style function defns for external
functions, and p3.diff didn't do that.	Also, p3.diff didn't apply to
the current sources due to more-recent changes.  I fixed both problems
and attach a fixed patch.

Roland, can you please explain why it's nice to use old-style function
definitions here?  Doesn't gcc's -Wmissing-prototypes option suffice
to detect missing declarations?

The log entry in that attachment still says "move GLOB_* definitions to glob.h",
though glob.h is unchanged.  Is having #include <glob.h> early without the
#undef GLOB_* ok for portability, given glob.h won't change to do any #undef's?

I think it will be fine.  Now that <glob.h> is included first, it's definitions
for the GLOB_* macros should take precedence, unless the headers that
"erroneously define" them do something extremely bad, like #undeffing them first
or failing to protect their definitions with #ifndef GLOB_WHATEVER, but I say
that bridge would best be crossed when we get to it.

The ChangeLog message was probably slightly inaccurate.  How about this
corercted one (I only changed the one sentence)?

2005-07-11  Derek Price  <derek@ximbiot.com>
        and Paul Eggert  <eggert@cs.ucla.edu>

    * sysdeps/generic/glob.c:  Update copyright.  Assume freestanding
    C89 compiler.  Simplify cruft that may be replaced with GNULIB
    modules.  Make no attempt to find 64-bit versions of file access
    functions directly when !_LIBC.  Avoid undef'ing GLOB_* macros by
    #including <glob.h> before potentially offending headers.
    (DIRENT_MUST_BE, DIRENT_MIGHT_BE_SYMLINK, DIRENT_MIGHT_BE_DIR): New
    macros to abstract dirent->d_type access.
    (GETPW_R_SIZE_MAX, GET_LOGIN_NAME_MAX): New macros to abstract sysconf
    access.

Where is the equivalent of the _GNU_GLOB_INTERFACE_VERSION tests?  How will code
using this gnulib nonsense determine whether the system has an adequate glob
implementation which is usable?

And don't remove casts from malloc et.al calls.

Subject: Bug 1060

CVSROOT:	/cvs/glibc
Module name:	libc
Changes by:	roland@sources.redhat.com	2006-01-11 05:30:03

Modified files:
	posix          : glob.c 

Log message:
	2006-01-10  Derek Price  <derek@ximbiot.com> Paul Eggert  <eggert@cs.ucla.edu>
	
	[BZ #1060]
	* posix/glob.c: Assume freestanding C89 compiler.  Simplify cruft that
	may be replaced with GNULIB modules.  Make no attempt to find 64-bit
	versions of file access functions directly when [!_LIBC].
	Don't define GLOB_* macros here.
	(DIRENT_MUST_BE, DIRENT_MIGHT_BE_SYMLINK, DIRENT_MIGHT_BE_DIR): New
	macros to abstract dirent->d_type access.
	(GETPW_R_SIZE_MAX, GET_LOGIN_NAME_MAX): New macros to abstract sysconf
	access.

Patches:
http://sources.redhat.com/cgi-bin/cvsweb.cgi/libc/posix/glob.c.diff?cvsroot=glibc&r1=1.68&r2=1.69

I've put in the patch, removing the changes we don't want.