Bug 1060 - sysdeps/generic/glob.c merge from gnulib (part 1 of 3)
Summary: sysdeps/generic/glob.c merge from gnulib (part 1 of 3)
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: libc (show other bugs)
Version: 2.3.5
: P2 normal
Target Milestone: ---
Assignee: Roland McGrath
URL:
Keywords:
Depends on:
Blocks: 1062
  Show dependency treegraph
 
Reported: 2005-07-12 07:02 UTC by Paul Eggert
Modified: 2006-01-11 05:31 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments
sysdeps/generic/glob.c merge from gnulib (part 1 of 3) (8.87 KB, patch)
2005-07-12 07:03 UTC, Paul Eggert
Details | Diff
Revised patch. (8.87 KB, patch)
2005-08-25 01:52 UTC, Derek Price
Details | Diff
Revised patch. (7.68 KB, patch)
2005-09-08 15:44 UTC, Derek Price
Details | Diff
improved patch to match current sources and use old-style defns (7.34 KB, patch)
2005-09-08 20:32 UTC, Paul Eggert
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Eggert 2005-07-12 07:02:51 UTC
As part of gnulib we've ported glibc sysdeps/generic/glob.c and posix/glob.h
to several other platforms, and fixed a couple of bugs.  We'd like to merge our
improvements and fixes into glibc.  The attached patch contains the first part
of our changes.  These are merely porting changes, useful when glob.c and glob.h
are employed outside of glibc; they should not affect glibc's behavior.
Comment 1 Paul Eggert 2005-07-12 07:03:53 UTC
Created attachment 552 [details]
sysdeps/generic/glob.c merge from gnulib (part 1 of 3)
Comment 2 Dmitry V. Levin 2005-08-24 23:31:25 UTC
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?
Comment 3 Derek Price 2005-08-25 01:52:32 UTC
Created attachment 617 [details]
Revised patch.

How is this?  I replaced occurrances of LOGIN_NAME_MAX with GET_LOGIN_NAME_MAX.
Comment 4 Paul Eggert 2005-09-01 22:37:04 UTC
Yes, renaming LOGIN_NAME_MAX is fine.  I've installed the revised
patch with GET_LOGIN_NAME_MAX into gnulib.  Thanks.
Comment 5 Roland McGrath 2005-09-08 07:47:47 UTC
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.
 
Comment 6 Derek Price 2005-09-08 15:44:26 UTC
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,
Comment 7 Paul Eggert 2005-09-08 20:32:44 UTC
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?
Comment 8 Roland McGrath 2005-09-08 23:32:19 UTC
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?
Comment 9 Derek Price 2005-09-09 01:42:30 UTC
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.
Comment 10 Ulrich Drepper 2005-09-17 18:07:48 UTC
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.
Comment 11 Sourceware Commits 2006-01-11 05:30:09 UTC
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

Comment 12 Roland McGrath 2006-01-11 05:31:09 UTC
I've put in the patch, removing the changes we don't want.