Re: [bug-gnulib] New GNULIB glob module?

[Karl Berry]

Subject: [bug-gnulib] New GNULIB glob module?

Would it be possible to simply use the libc code as-is?  I guess I mean,
with whatever changes are needed sent back to libc.

So much stuff in gnulib is 95% the same as libc.  It doesn't seem good.

k


___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Karl Berry wrote:

> Subject: [bug-gnulib] New GNULIB glob module?
>
>Would it be possible to simply use the libc code as-is? I guess I mean,
>with whatever changes are needed sent back to libc.
>
>So much stuff in gnulib is 95% the same as libc. It doesn't seem good.


Well, like I said, a bunch of the cruft I cut out could be removed
because it was much simpler to depend on other GNULIB modules.  The
alternative would have been to add a bunch of tests to detect the things
that were being switched on, and I expect would have been much more
complicated since those problems have already been solved in the GNULIB
modules the file now references and I would have had to test the new
tests...

To have libc use the same code, it would be best if they imported the
GNULIB modules my new glob depends on and then reimported my glob
files.  We might have to find a slightly better compromise on the header
file since I'm not sure that all the assumptions I made there that are
valid with GNULIB would be valid at run time for other programs
expecting only glibc.

(Glancing back at the header file now, it looks like I did a few things
that should be undone/fixed anyhow.  I reviewed my changes to glob.c
more thoroughly, but I disabled a few things in glob.h like the 64 bit
file offsets, intending to revisit the issue later.  I don't know all
the issues involved there...  what's "__REDIRECT_NTH" and why does it
break in my compiler?  I was hoping someone else might be able to deal
with that more quickly than I, but most of the rest of my changes should
be valid and useful as/in the port to GNULIB...)

Regards,

Derek
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.0 (Cygwin)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFCgoiULD1OTBfyMaQRApY+AJ9VsOAvoa3endmyHCxT1ai/+Nch4ACfWAye
yXShaWGtcwczl045kRTY7G4=
=Wf8s
-END PGP SIGNATURE-




___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Paul Eggert]

Derek Price <[EMAIL PROTECTED]> writes:

> I needed a portable version of glob for CVS, so I imported the glob
> module from glibc 2.3.5 into a GNULIB format.

Wow!  Nice project.

> Mostly I kept the glibc version intact, though I managed to simplify
> some portability cruft immensely by replacing huge chunks of code
> with #includes of GNULIB modules.

Ideally we should write code that can be folded back into glibc,
identical to the gnulib version.  That should be doable here, but some
more hacking is needed.

First, what is "__ptr_t"?  Shouldn't it be replaced by "void *"
uniformly?

> -/* AIX requires this to be the first thing in the file.  */
> -#if defined _AIX && !defined __GNUC__
> - #pragma alloca
> -#endif

This change is good, since glibc doesn't need that code and gnulib
doesn't either (any more).

> -#if defined HAVE_LIMITS_H || defined __GNU_LIBRARY__
> -# include 
> -#endif
> +#include 

This sort of change is good, since gnulib assumes C89 or better now.

> -#define GLOB_INTERFACE_VERSION 1
> -#if !defined _LIBC && defined __GNU_LIBRARY__ && __GNU_LIBRARY__ > 1
> -# include 
> -# if _GNU_GLOB_INTERFACE_VERSION == GLOB_INTERFACE_VERSION
> -#  define ELIDE_CODE
> -# endif
> -#endif

This removal looks reasonable, except shouldn't glob.m4 check that
_GNU_GLOB_INTERFACE_VERSION is 1?  It can do that as a compile-time
check.

> -#if defined HAVE_UNISTD_H || defined _LIBC
> +#if HAVE_HEADER_UNISTD_H || _LIBC

This sort of change doesn't look right.  Why not leave the code alone?
Nothing ever defines HAVE_HEADER_UNISTD_H.  There are several other
instances of this sort of thing, e.g., HAVE_HEADER_SYS_NDIR_H,
HAVE_DIRENT64.

> -#if _LIBC
> -# define HAVE_DIRENT64   1
> +#if defined _DIRENT_HAVE_D_TYPE && !defined HAVE_DIRENT_D_TYPE
> +# define HAVE_DIRENT_D_TYPE  1
>  #endif

This should use HAVE_STRUCT_DIRENT_D_TYPE, from d-type.m4.  Also,
don't see why you need to change the spelling of HAVE_D_TYPE.  Please
try to leave the identifiers the same, so that the glibc maintainers
have an easier time of following the patch.

> -#ifdef _LIBC
> -# include 
> -# undef strdup
> -# define strdup(str) __strdup (str)
> -# define sysconf(id) __sysconf (id)
> -# define closedir(dir) __closedir (dir)
> -# define opendir(name) __opendir (name)
> -# define readdir(str) __readdir64 (str)
> -# define getpwnam_r(name, bufp, buf, len, res) \
> -   __getpwnam_r (name, bufp, buf, len, res)
> -# ifndef __stat64
> -#  define __stat64(fname, buf) __xstat64 (_STAT_VER, fname, buf)
> -# endif
> -# define HAVE_STAT64 1
> -#endif

Please leave this sort of code in, so that the source code can be
identical in libc.

> +#include "mempcpy.h"
> +#include "stat-macros.h"
> +#include "strdup.h"

These includes need to be protected by #ifndef _LIBC.

> +int glob_pattern_p (const char *pattern, int quote);

I don't see why this declaration is needed in glob.c.
Isn't it in glob_.h?

> -   if (__glob_pattern_p (drive_spec, !(flags & GLOB_NOESCAPE)))
> +   if (glob_pattern_p (drive_spec, !(flags & GLOB_NOESCAPE)))

I'd leave the code alone here, to minimize the libc changes.
You can put a "#define __glob_pattern_p glob_pattern_p" inside
Similarly for __stat64, etc.

> -  if (flags & GLOB_MARK)
> -{
> -  /* Append slashes to directory names.  */
> -  size_t i;

Why remove this feature?  Is there any harm to leaving it alone?

>  int
> -__glob_pattern_p (pattern, quote)
> - const char *pattern;
> - int quote;
> +glob_pattern_p (const char *pattern, int quote)

This sort of change is good.

> -# ifdef _LIBC
> -weak_alias (__glob_pattern_p, glob_pattern_p)
> -# endif

We can leave the code alone here; it doesn't hurt.

>struct stat st;
> +# ifdef HAVE_FUNCTION_STAT64
>struct stat64 st64;
> +# endif

This sort of thing is awkward.  How about if you remove the
"# define st64 st" line?  Then you can omit this sort of change.

> -  return (((flags & GLOB_ALTDIRFUNC)
> -? (*pglob->gl_stat) (fullname, &st)
> -: __stat64 (fullname, &st64)) == 0);
> +  return flags & GLOB_ALTDIRFUNC
> +  ? (*pglob->gl_stat) (fullname, &st) == 0 && S_ISDIR (st.st_mode)
> +  : stat64 (fullname, &st64) == 0 && S_ISDIR (st64.st_mode);

Please stick with the original indenting style and names.

Why did you insert the && S_ISDIR here?  Is this a bug fix?  It's not
clear from the patch.  (Also, you need a ChangeLog entry suitable for
sending this patch to the glibc folks.)

> +#ifdef HAVE_DIRENT_D_TYPE
> +# define MAY_BE_LINK(d) (d->d_type == DT_UNKNOWN || d->d_type == DT_LNK)
> +# define MAY_BE_DIR(d)   (d->d_type == DT_DIR || MAY_BE_LINK (d))

Parenthesize (d) and move these definitions to an early part of the
program, and put in an else-part, e.g.:

#if HAVE_D_TYPE
# define MAY_BE_LINK(d) ((d)->d_type == DT_UNKNOWN || (d)->d_type == DT_LNK)
# define MAY_BE_DIR(d)  ((d)->d_type == DT_DIR || MAY_BE_LINK (d))
# define MUST_BE(d, t)  ((d)->d_type == (t))
#else
# define MAY_BE_LINK(d) true
# define MAY_BE_D

Re: [bug-gnulib] New GNULIB glob module?

[Derek Price]

Most of this looks good, but one quick question as I get to this:  I
assume from the following that I can expect _LIBC to be defined iff the
file is being compiled as part of glibc?  I wasn't sure...

Cheers,

Derek


Paul Eggert wrote:

>>-#ifdef _LIBC
>>-# include 
>>-# undef strdup
>>-# define strdup(str) __strdup (str)
>>-# define sysconf(id) __sysconf (id)
>>-# define closedir(dir) __closedir (dir)
>>-# define opendir(name) __opendir (name)
>>-# define readdir(str) __readdir64 (str)
>>-# define getpwnam_r(name, bufp, buf, len, res) \
>>-   __getpwnam_r (name, bufp, buf, len, res)
>>-# ifndef __stat64
>>-#  define __stat64(fname, buf) __xstat64 (_STAT_VER, fname, buf)
>>-# endif
>>-# define HAVE_STAT64 1
>>-#endif
>>
>>
>
>Please leave this sort of code in, so that the source code can be
>identical in libc.
>
>  
>
>>+#include "mempcpy.h"
>>+#include "stat-macros.h"
>>+#include "strdup.h"
>>
>>
>
>These includes need to be protected by #ifndef _LIBC.
>  
>



___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Paul Eggert]

Derek Price <[EMAIL PROTECTED]> writes:

>>First, what is "__ptr_t"?  Shouldn't it be replaced by "void *"
>>uniformly?
>
> This is one of the items I didn't touch since it didn't raise any
> compiler warnings here.  It looks like it is mostly being used as a void
> *, though I'm not sure why since there are a few direct void * decls. 

My guess is that __ptr_t is a glibc thing, and is present to allow
ports to pre-C89 hosts that lacked support for void *.  We no longer
worry about such hosts, so we can simply use void *.

> It looks like there are a few unnecessary typecasts

Possibly they're needed for people who use C++ compilers to
compile C code?

> It's okay to assume C89 or better for glibc?  I knew it was correct for
> GNULIB but I was unsure about glibc.

Yes, it's OK.

> Otherwise, my correction is as you specified, but if we can assume C89
> in glob.c, why can't we assume C89 here and just #include ?

glob.h is not supposed to infringe on the user namespace when it's
part of glibc.  However, it's OK (and to some extent unavoidable) for
glob.h to do so when it's part of user code.

> -#if defined HAVE_UNISTD_H || defined _LIBC
> +#if HAVE_UNISTD_H || _LIBC

I'd omit this change; it's not necessary and it just complicates
the job of evaluating the patch.

>  # ifdef HAVE_VMSDIR_H
> +   /* Should this be imported from glibc?  */
>  #  include "vmsdir.h"

glibc doesn't have a vmsdir.h.  This area should be fixed up but this
is a separate matter; I'd omit this change for now.

> +# define CONVERT_D_TYPE(d64, d32)

This macro isn't used anywhere and can be removed.

> +# define MUST_BE(d, t)   ((d)->d_type == (t))
> +# define MAY_BE_LINK(d)  (MUST_BE((d), DT_UNKNOWN) || MUST_BE((d), 
> DT_LNK))
> +# define MAY_BE_DIR(d)   (MUST_BE((d), DT_DIR) || MAY_BE_LINK (d))

This is a bit hard to follow, I'm afraid, and the names are
contributing to the confusion.  We should use "MIGHT" instead of
"MAY", to avoid the classic "may" ambiguity.  How about if we rename
MUST_BE to DIRENT_MUST_BE, MAY_BE_LINK to DIRENT_MIGHT_BE_LINK, and
MAY_BE_DIR to DIRENT_MIGHT_BE_DIR?  We also refrain from having the
MAY_BE macros call MUST_BE.  Something like this:

/* True if the directory entry D must be of type T.  */  
# define DIRENT_MUST_BE(d, t) ((d)->d_type == (t))

/* True if the directory entry D might be a symbolic link.  */
# define DIRENT_MIGHT_BE_SYMLINK(d) \
((d)->d_type == DT_LNK || (d)->d_type == DT_UNKNOWN)

/* True if the directory entry D might be a directory.  */
# define DIRENT_MIGHT_BE_DIR(d) \
((d)->d_type == DT_DIR || DIRENT_MIGHT_BE_SYMLINK (d))

> -#if _LIBC
> +#if _LIBC || HAVE_FUNCTION_DIRENT64
>  # define HAVE_DIRENT64   1

I don't see why this change is needed.  The .m4 file sets
HAVE_DIRENT64.

> +/* GNULIB includes that shouldn't be used when copiling LIBC.  */

I'd remove this comment; it's obvious.

> +/* Correct versions of these headers all come from GNULIB when missing or
> + * broken.
> + */

Likewise.  Also, please use GNU style for comments (no leading-*).

  #ifdef HAVE_FUNCTION_STAT64
  /* Assume the struct stat64 type.  */
  # define HAVE_STAT64  1
  #endif

The m4 file defines HAVE_STAT64, so this entire code block should be
removed.

>  #ifndef HAVE_STAT64
> +  /* Use stat() instead.  */
>  # define __stat64(fname, buf) __stat (fname, buf)

I'd omit this change; the comment is obvious.

> +# define stat64 stat

I don't see why this #define is needed; doesn't the code use __stat64
uniformly?

> +#elif !_LIBC

Generally the code uses "defined _LIBC" rather than "_LIBC".  It's not
universally true, but I'd stick to the strong-majority style when adding
new code.

> +  /* GNULIB needs the usual versions.  */
> +# define __stat64 stat64

First, lose the comment (it's obvious); second, all the #define's of
__stat64 should be kept near each other.

> +  /* GNULIB needs the usual versions.  */

Again, the comment isn't needed.

> +# define __stat stat
> +# define __alloca alloca
> +# define __readdir readdir
> +# define __readdir64 readdir64

Perhaps these should be put next to the __stat64 define?

> -#include 

The Gnulib tradition is to include the interface file (here, )
first, immediately after including .  Could you please do
that here, too?

> -#ifdef HAVE_GETLOGIN_R
> +#ifdef HAVE_FUNCTION_GETLOGIN_R

This change shouldn't be needed.

> -
> +
> +
> +
>  static const char *next_brace_sub (const char *begin, int flags) __THROW;

This change shouldn't be needed.

> -#   if defined HAVE_GETLOGIN_R || defined _LIBC
> +#   if defined HAVE_FUNCTION_GETLOGIN_R || defined _LIBC

This change shouldn't be needed.

> -#   if defined HAVE_GETPWNAM_R || defined _LIBC
> +#   if defined HAVE_FUNCTION_GETPWNAM_R || defined _LIBC

Nor this.

> -#  if defined HAVE_GETPWNAM_R || defined _LIBC
> +#  if defined HAVE_FUNCTION_GETPWNAM_R || defined _LIBC

Nor this.

> -#endif
> +#endif /* !defined _LIBC || !defined NO_GLOB_PATTERN_P */

Nor this.

> -static int
> -link_exists_p (cons

Re: [bug-gnulib] New GNULIB glob module?

[Paul Eggert]

Derek Price <[EMAIL PROTECTED]> writes:

> We've been removing those sort of constructs from CVS as part of the
> move away from K&R support.  IIRC, I was told that typecasts to and from
> (void *) were necessary on some really old systems, but that I could be
> confident that we wouldn't encounter any C89 systems that required it. 

That's correct.

> Perhaps this is simply legacy code related to the __ptr_t stuff and
> could be removed as well?

Yes, quite possibly.  How about if we simply assume C89-or-later for
now.  If someone runs into a problem with a bogus masquerading
compiler we can fix it later.

> why does that preclude a #include  in glob.h?

Because in glibc #include  isn't supposed to define irrelevant
identifiers like wchar_t (which  does define).

It's OK for #include  to define these identifiers in gnulib,
though.  Partly because we can't avoid it, and partly because we're
not as persnickety anyway.

I'll review the rest of your message later (sorry, too many meetings today!).


___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Paul Eggert]

Derek Price <[EMAIL PROTECTED]> writes:

>  /* Enable GNU extensions in glob.h.  */
> -#ifndef _GNU_SOURCE
> +#if defined _LIBC && !defined _GNU_SOURCE
>  # define _GNU_SOURCE 1
>  #endif

I just checked the glibc source file include/libc-symbols.h, and it
defines both _LIBC and _GNU_SOURCE.  So this stuff isn't needed at
all; please remove these lines from glob.c instead.

> +   HAVE_STRUCT_DIRENT_D_TYPE is defined by GNULIB's glob.m4 when the same
> +   member is found.  */

A bit wordy; please rephrase to "HAVE_STRUCT_DIRENT_D_TYPE plays the
same rule in gnulib."

> -  qsort ((__ptr_t) &pglob->gl_pathv[oldcount],
> +  qsort ((void *) &pglob->gl_pathv[oldcount],

You can remove the cast entirely, right?

> -   free ((__ptr_t) pglob->gl_pathv[pglob->gl_offs + i]);
> -  free ((__ptr_t) pglob->gl_pathv);
> +   free ((void *) pglob->gl_pathv[pglob->gl_offs + i]);
> +  free ((void *) pglob->gl_pathv);

Similarly, these casts aren't needed.

-   free ((__ptr_t) array[--i]);
+   free ((void *) array[--i]);

Nor these.

> - free ((__ptr_t) array[--i]);
> + free ((void *) array[--i]);

Nor this one.

> +   p = mempcpy ((void *) new->name, name, len);

Nor this one.

> - free ((__ptr_t) names->name);
> + free ((void *) names->name);

Nor this.


> -  return (((flags & GLOB_ALTDIRFUNC)
> -? (*pglob->gl_stat) (fullname, &st)
> -: __stat64 (fullname, &st64)) == 0);
> +  return ((flags & GLOB_ALTDIRFUNC)
> +   ? (*pglob->gl_stat) (fullname, &st) == 0 && S_ISDIR (st.st_mode)
> +   : __stat64 (fullname, &st64) == 0 && S_ISDIR (st64.st_mode));

As long as you're reparenthesizing you might as well get rid of
the unnecessary ones around (flags & GLOB_ALTDIRFUNC).

> +   new->name = malloc (len + 1
> +   + ((flags & GLOB_MARK) && isdir));

This line is 80 columns; should probably shrink it to 78, e.g., via.

  new->name =
malloc (...);

glob.c is looking pretty good now.  I gotta run now, but I'll look at
glob.h later today I hope.


___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Paul Eggert]

Now for commentary on glob.h.

Derek Price <[EMAIL PROTECTED]> writes:

> --- ../glibc-2.3.5/posix/glob.h   2004-09-16 20:55:15.0 -0400
> +++ lib/glob_.h   2005-05-13 12:21:39.0 -0400
> @@ -19,29 +19,48 @@
>  #ifndef  _GLOB_H
>  #define  _GLOB_H 1
>  
> -#include 
> +#ifdef _LIBC
> +# include 
> +#else
> +# include 
> +# include 
> +# undef __size_t
> +# define __size_t size_t
> +#endif

Why do we need to include  here?  All we need is size_t,
right?  And stddef.h gives us that.

> +/* Some system libraries erroneously define these.  */
> +#undef   GLOB_ERR
> ...

Which system libraries are these?  Perhaps a comment?

> +# define getopt __GLOB_ID (getopt)

Surely this line is spurious and can be removed.

> -#if !defined __USE_FILE_OFFSET64 || __GNUC__ < 2
> +#if !defined _LIBC || !defined __USE_FILE_OFFSET64 || __GNUC__ < 2

Can't we remove at least the "|| __GNUC__ < 2" here?  glibc assumes
recent versions of GCC.

>  extern void __REDIRECT_NTH (globfree, (glob_t *__pglob), globfree64);
>  #endif
> -
>  #ifdef __USE_LARGEFILE64
>  extern int glob64 (__const char *__restrict __pattern, int __flags,
>  int (*__errfunc) (__const char *, int),

Let's omit this change; it's a different matter.


___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Paul Eggert]

One other remark about glob.c.  You should remove this comment, as
it's obsolete now:

/* Comment out all this code if we are using the GNU C Library, and are not
   actually compiling the library itself.  This code is part of the GNU C
   Library, but also included in many other GNU distributions.  Compiling
   and linking in this code is a waste when using the GNU C library
   (especially if it is a shared library).  Rather than having every GNU
   program understand `configure --with-gnu-libc' and omit the object files,
   it is simpler to just do this in the source for each such file.  */


Now for comments on m4/glob.m4 and modules/glob.  This finishes up
this round of reviewing.

gl_GLOB_SUBSTITUTE and gl_GLOB both invoke gl_PREREQ_GLOB.  This
is overkill, surely.  I'd say only the former needs to invoke it.

> +  AC_CHECK_HEADERS([glob.h gnu-versions.h], [], [GLOB_H=glob.h])
> +
> +  if test -z "$GLOB_H"; then
> +AC_EGREP_CPP([gl_gnu_glob_version = 1$],
> +[#include 
> +gl_gnu_glob_version = _GNU_GLOB_INTERFACE_VERSION],
> +  [], [GLOB_H=glob.h])
> +  fi

This seems a bit brittle.  Why not simply try to compile this program?

  #include 
  char a[_GNU_GLOB_INTERFACE_VERSION == 1 ? 1 : -1];

> +  AC_CHECK_HEADERS_ONCE([dirent.h sys/ndir.h sys/dir.h ndir.h])dnl

Substitute AC_REQUIRE([AC_HEADER_DIRENT]) for this line.

> +  AC_CHECK_FUNCS_ONCE([dirent64 getlogin_r getpwnam_r stat64])dnl

The other three functions are fine, but: dirent64 is a function?  I
thought it was a structure tag.

Surely you mean to use AC_CHECK_TYPE([struct dirent64]), and for the
code to to refer to HAVE_STRUCT_DIRENT64 rather than HAVE_DIRENT64?

Come to think of it, how does the code currently work here?  In the
normal case (where the maintainer has not used configure's
--disable-largefile option, and where the system supports a 64-bit
file interface) we want glob to use the 64-bit file interface.  Is
that what is happening here?  I can't quite follow it.


> +++ modules/glob  13 May 2005 16:23:46 -
> @@ -0,0 +1,43 @@
> +Description:
> +Search for files and directories with paths matching a pattern.
> +
> +Depends-on:
> +alloca
> +extensions
> +fnmatch
> +mempcpy
> +realloc

I don't see why this module depends on the realloc module.  It doesn't
ever realloc anything to size zero.

> +License:
> +??? LGPL ???

Yes, it's LGPL.


___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Paul Eggert]

Derek Price <[EMAIL PROTECTED]> writes:

> Are you sure?  You asked me to restore similar parens around bit-ands
> back at several other locations despite other work that changed the
> lines, in an earlier email.  Not that I disagree now.  I actually prefer
> the version without the unnecessary parens around the  bit-and.  I just
> think we should be consistent.

It's a minor point, but expressions like (a & b && c) are assumed to
be confusing, as they depend on obscure precedence rules, whereas
expressions like (a & b ? c : d) are not confusing in the same way:
there's only one way to parse them, even if you have forgotten the
precedence.


___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Paul Eggert]

Derek Price <[EMAIL PROTECTED]> writes:

>>Why do we need to include  here?  All we need is size_t,
>>right?  And stddef.h gives us that.
>
> If I don't, I get the following error:
>
> In file included from glob.c:23:
> glob.h:107: error: syntax error before "struct"
> In file included from /usr/include/errno.h:36,
>  from glob.c:25:
> /usr/include/bits/errno.h:38: error: syntax error before "extern"

Sorry, I can't debug that, since I don't have the exact version you do.

In the latest version you sent, we have code that looks like this:


   #ifdef _LIBC
   # include 
   #else
   # include 
   # include 
   # undef __size_t
   # define __size_t size_t
   #endif

   __BEGIN_DECLS

First, already we have something bogus: that __BEGIN_DECLS.  It must
be protected by "#ifdef _LIBC", since random C environments don't have
it.  Similarly for __END_DECLS.

Second, that  ought to be unnecessary.  We need only
size_t, and  provides that.  If we need  for
some other reason on your platform, let's figure out why that is, and
attack its root source rather than the symptoms.  (The rest of this
message does that, I think.)

> Hrm.  Tracing this a little farther, it is only , or even
> the ,

OK, here's what's happening, I think.  Every glibc header includes
 (and thus ) first thing, and this defines a
whole bunch of stuff.  If you attempt to declare libc-related stuff
without including  first, bad things happen.

The simplest fix would be to do something like this:

#if defined _LIBC || HAVE_SYS_CDEFS_H
# include 
#endif

with the appropriate change to glob.m4.

But let's step back a second.  Why are we worried about building
gnulib glob.c under glibc?  It will never happen, right?  So perhaps
we needn't worry about this problem at all.


> I copied and pasted this comment and the #undef block
> from the glob.c file.  It was previously placed just prior to the
> #include of .  This looked the better place for it since I
> assumed that we wouldn't want applications using the GNULIB glob module
> to issue lots of redefinition warnings.

OK, but in general this problem cannot be fixed in glob_.h.  If the
application includes our glob.h and then the buggy system files, it
will still lose big-time.

I tracked down these #undefs to this patch:

http://sourceware.org/cgi-bin/cvsweb.cgi/libc/posix/Attic/glob.c.diff?r1=1.20&r2=1.21&cvsroot=glibc

which has this ChangeLog entry in glibc:

Thu Jan 21 20:12:25 1993  Roland McGrath  ([EMAIL PROTECTED])

* posix/glob.c: Put #includes of  and  after
all system includes, in case one of them has conflicting defns of
FNM_* or GLOB_*, so we will redefine.  #undef FNM_* and GLOB_*
before including our headers.

I have heard of this problem occurring with  -- indeed,
 documents that it occurs with HP-UX A.08.07 -- but it's
not clear to me that the problem every actually occurred with
.  Perhaps Roland made the change for  simply as a
precaution, because it had happened with .

Also, the problem, if it occurred with glob.h, was a long time ago,
and perhaps we need not worry about it any more.  HP-UX 8.07 was
released in 1992, and HP hasn't supported it for many years.  I doubt
whether people are building new GNU software for it any more.

With that in mind, how about if we simply remove those #undef's?  If
the problem recurs we can put them back in again.


___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Thanks for the latest round.  I'm going to be out of the office today,
but I should get to it by tomorrow.

Regards,

Derek
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.0 (Cygwin)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFCiICvLD1OTBfyMaQRAsMbAJ96UgJcf/G+zBfOlXBJM+nBJzff/ACglQte
CYnnsSKWV74qlZJbIcVNNWk=
=0b5Z
-END PGP SIGNATURE-




___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Derek Price]

Paul Eggert wrote:

>Derek Price <[EMAIL PROTECTED]> writes:
>
>  
>
>> /* Enable GNU extensions in glob.h.  */
>>-#ifndef _GNU_SOURCE
>>+#if defined _LIBC && !defined _GNU_SOURCE
>> # define _GNU_SOURCE 1
>> #endif
>>
>>
>
>I just checked the glibc source file include/libc-symbols.h, and it
>defines both _LIBC and _GNU_SOURCE.  So this stuff isn't needed at
>all; please remove these lines from glob.c instead.
>  
>

Done.

>>+   HAVE_STRUCT_DIRENT_D_TYPE is defined by GNULIB's glob.m4 when the same
>>+   member is found.  */
>>
>>
>
>A bit wordy; please rephrase to "HAVE_STRUCT_DIRENT_D_TYPE plays the
>same rule in gnulib."
>  
>

Rephrased.

>>-  qsort ((__ptr_t) &pglob->gl_pathv[oldcount],
>>+  qsort ((void *) &pglob->gl_pathv[oldcount],
>>
>>
>
>You can remove the cast entirely, right?
>  
>

So you agreed in an earlier message.  I've now removed all typecasts to
and from void * which are declared unnecessary by C89.

>>-   free ((__ptr_t) pglob->gl_pathv[pglob->gl_offs + i]);
>>-  free ((__ptr_t) pglob->gl_pathv);
>>+   free ((void *) pglob->gl_pathv[pglob->gl_offs + i]);
>>+  free ((void *) pglob->gl_pathv);
>>
>>
>
>Similarly, these casts aren't needed.
>
>-  free ((__ptr_t) array[--i]);
>+  free ((void *) array[--i]);
>
>Nor these.
>
>  
>
>>- free ((__ptr_t) array[--i]);
>>+ free ((void *) array[--i]);
>>
>>
>
>Nor this one.
>
>  
>
>>+   p = mempcpy ((void *) new->name, name, len);
>>
>>
>
>Nor this one.
>
>  
>
>>- free ((__ptr_t) names->name);
>>+ free ((void *) names->name);
>>
>>
>
>Nor this.
>  
>

Removed, removed, removed, removed, and removed, per the above.

>>-  return (((flags & GLOB_ALTDIRFUNC)
>>-? (*pglob->gl_stat) (fullname, &st)
>>-: __stat64 (fullname, &st64)) == 0);
>>+  return ((flags & GLOB_ALTDIRFUNC)
>>+   ? (*pglob->gl_stat) (fullname, &st) == 0 && S_ISDIR (st.st_mode)
>>+   : __stat64 (fullname, &st64) == 0 && S_ISDIR (st64.st_mode));
>>
>>
>
>As long as you're reparenthesizing you might as well get rid of
>the unnecessary ones around (flags & GLOB_ALTDIRFUNC).
>  
>

Are you sure?  You asked me to restore similar parens around bit-ands
back at several other locations despite other work that changed the
lines, in an earlier email.  Not that I disagree now.  I actually prefer
the version without the unnecessary parens around the  bit-and.  I just
think we should be consistent.

>>+   new->name = malloc (len + 1
>>+   + ((flags & GLOB_MARK) && isdir));
>>
>>
>
>This line is 80 columns; should probably shrink it to 78, e.g., via.
>
>  new->name =
>malloc (...);
>  
>

Done.

>glob.c is looking pretty good now.  I gotta run now, but I'll look at
>glob.h later today I hope.
>  
>

Thanks.  I'm not attaching a patch - I'll include a complete one
shortly, after I process your next two emails.

Cheers,

Derek



___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Paul Eggert]

Derek Price <[EMAIL PROTECTED]> writes:

>>This seems a bit brittle.  Why not simply try to compile this program?
>>
>>  #include 
>>  char a[_GNU_GLOB_INTERFACE_VERSION == 1 ? 1 : -1];
>
> Because I like to avoid runtime tests if I can avoid it, since they
> cannot be used when cross-compiling.

The code that I proposed does not have any runtime tests, since the
array is allocated statically.  Tests like this are often used during
cross-compiling.

> it should be pretty safe to grep for "gnu_glob_version = 1", unless you
> are worried about some future CPP padding with spaces before the EOL?

Yes, that sort of thing.  cpp can even insert newlines if it wants to.
It's best to avoid cpp tricks if there is an easy substitute, which
there is here.

> There is one change I know I made that might have an effect.  I added
> the "!defined _LIBC" to the following block because I was unsure what
> __REDIRECT_NTH was supposed to be doing

I think that one's OK.

> Fixed.  It's still listed in srclist.txt as "gpl" however, since the
> other glibc modules were.  Why is that?

That tells the import-from-glibc module to use the GPL in the CVS
repository.  It's fine.

More messages to follow, since I need to look at the code again.


___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Derek Price]

Paul Eggert wrote:

>Now for commentary on glob.h.
>
>Derek Price <[EMAIL PROTECTED]> writes:
>
>  
>
>>--- ../glibc-2.3.5/posix/glob.h   2004-09-16 20:55:15.0 -0400
>>+++ lib/glob_.h   2005-05-13 12:21:39.0 -0400
>>@@ -19,29 +19,48 @@
>> #ifndef  _GLOB_H
>> #define  _GLOB_H 1
>> 
>>-#include 
>>+#ifdef _LIBC
>>+# include 
>>+#else
>>+# include 
>>+# include 
>>+# undef __size_t
>>+# define __size_t size_t
>>+#endif
>>
>>
>
>Why do we need to include  here?  All we need is size_t,
>right?  And stddef.h gives us that.
>  
>

If I don't, I get the following error:

In file included from glob.c:23:
glob.h:107: error: syntax error before "struct"
In file included from /usr/include/errno.h:36,
 from glob.c:25:
/usr/include/bits/errno.h:38: error: syntax error before "extern"

Hrm.  Tracing this a little farther, it is only , or even
the , that glob_.h was originally including, and not
, which is needed to avoid the above error.  
appears to define __USE_GNU (which the glob_.h file uses) when
_GNU_SOURCE is defined, but a #define __USE_GNU prior to the include of
 isn't sufficient to do the trick.  Have you seen anything
like this before?  I can't really tell the difference between
 and  with a simple test since they each appear
to include the other.  I'm personally inclined to leave the #include
 since it is the standard header and it seems to fix
things, but perhaps you can give me more information?

>>+/* Some system libraries erroneously define these.  */
>>+#undef   GLOB_ERR
>>...
>>
>>
>
>Which system libraries are these?  Perhaps a comment?
>  
>

I don't know.  I copied and pasted this comment and the #undef block
from the glob.c file.  It was previously placed just prior to the
#include of .  This looked the better place for it since I
assumed that we wouldn't want applications using the GNULIB glob module
to issue lots of redefinition warnings.

>>+# define getopt __GLOB_ID (getopt)
>>
>>
>
>Surely this line is spurious and can be removed.
>  
>

Done.

>>-#if !defined __USE_FILE_OFFSET64 || __GNUC__ < 2
>>+#if !defined _LIBC || !defined __USE_FILE_OFFSET64 || __GNUC__ < 2
>>
>>
>
>Can't we remove at least the "|| __GNUC__ < 2" here?  glibc assumes
>recent versions of GCC.
>  
>

I'll take your word for it.  I've also followed through to remove the
following block:

#if __USE_FILE_OFFSET64 && __GNUC__ < 2
# define glob glob64
# define globfree globfree64
#endif

And replaced this block:

# ifndef __size_t
#  if defined __GNUC__ && __GNUC__ >= 2
typedef __SIZE_TYPE__ __size_t;
#   ifdef __USE_XOPEN
typedef __SIZE_TYPE__ size_t;
#   endif
#  else
#   include 
#   ifndef __size_t
#define __size_t size_t
#   endif
#  endif
# else

with this block:

# ifndef __size_t
typedef __SIZE_TYPE__ __size_t;
#  ifdef __USE_XOPEN
typedef __SIZE_TYPE__ size_t;
#  endif
# else

>> extern void __REDIRECT_NTH (globfree, (glob_t *__pglob), globfree64);
>> #endif
>>-
>> #ifdef __USE_LARGEFILE64
>> extern int glob64 (__const char *__restrict __pattern, int __flags,
>> int (*__errfunc) (__const char *, int),
>>
>>
>
>Let's omit this change; it's a different matter.
>  
>

Done.

Still withholding my patch.  I have one more email to process.

Regards,

Derek



___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Paul Eggert wrote:

>Derek Price <[EMAIL PROTECTED]> writes:
>
>>Are you sure? You asked me to restore similar parens around bit-ands
>>back at several other locations despite other work that changed the
>>lines, in an earlier email. Not that I disagree now. I actually prefer
>>the version without the unnecessary parens around the bit-and. I just
>>think we should be consistent.
>
>
>It's a minor point, but expressions like (a & b && c) are assumed to
>be confusing, as they depend on obscure precedence rules, whereas
>expressions like (a & b ? c : d) are not confusing in the same way:
>there's only one way to parse them, even if you have forgotten the
>precedence.


Okay, parens removed.  Delaying new patch until I've processed the rest
of your emails.

Cheers,

Derek
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.0 (Cygwin)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFCii0TLD1OTBfyMaQRAgE0AKCMyzvr5P31kyXB2aDTDdUSOTp6HQCg2+dm
CjcxyJKKkqFEyexvpoMajtE=
=h/Cn
-END PGP SIGNATURE-




___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Paul Eggert wrote:

>First, already we have something bogus: that __BEGIN_DECLS. It must
>be protected by "#ifdef _LIBC", since random C environments don't have
>it. Similarly for __END_DECLS.


Done.

>The simplest fix would be to do something like this:
>
>#if defined _LIBC || HAVE_SYS_CDEFS_H
># include 
>#endif
>
>with the appropriate change to glob.m4.


Actually, this change doesn't appear to be necessary once I protect the
__BEGIN_DECLS and __END_DECLS stuff.

I've removed the #include of .

>But let's step back a second. Why are we worried about building
>gnulib glob.c under glibc? It will never happen, right? So perhaps
>we needn't worry about this problem at all.


Won't it?  I thought the idea was that when you and I settled on
something that looked good I would attempt to send it back to the libc
folks so that the files wouldn't need to be maintained separately.  So
far, our work contains at the least some cleanup, a bug fix, and an
efficiency improvement that should be useful in libc.

>With that in mind, how about if we simply remove those #undef's? If
>the problem recurs we can put them back in again.


Done.

Patch still delayed until I'm done with my email.

Regards,

Derek
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.0 (Cygwin)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFCijnMLD1OTBfyMaQRAklUAKDo25YXS7Hk5kqlYe1AzXICeBBLfwCfahXg
oD3soIvk5Shqw50Mfm4dgRY=
=/2W5
-END PGP SIGNATURE-




___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Derek Price]

Paul Eggert wrote:

>Derek Price <[EMAIL PROTECTED]> writes:
>
>  
>
>>>This seems a bit brittle.  Why not simply try to compile this program?
>>>
>>> #include 
>>> char a[_GNU_GLOB_INTERFACE_VERSION == 1 ? 1 : -1];
>>>  
>>>
>>Because I like to avoid runtime tests if I can avoid it, since they
>>cannot be used when cross-compiling.
>>
>>
>
>The code that I proposed does not have any runtime tests, since the
>array is allocated statically.  Tests like this are often used during
>cross-compiling.
>  
>

Ah, I didn't realize the optimizer could be relied on for code like
this.  Fixed.

Patch delayed until completion of today's email processing.

Regards,

Derek



___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Paul Eggert]

Derek Price <[EMAIL PROTECTED]> writes:

>>But let's step back a second. Why are we worried about building
>>gnulib glob.c under glibc? It will never happen, right? So perhaps
>>we needn't worry about this problem at all.
>
> Won't it?  I thought the idea was that when you and I settled on
> something that looked good I would attempt to send it back to the libc
> folks so that the files wouldn't need to be maintained separately.

Yes, that's the idea, but that's a different scenario.  In that
scenario we are building glob.c as part of glibc, so _LIBC is 1.  I am
talking about the scenario where glob.c is part of a user program
(i.e., glob.c is imported from gnulib), and where the C library is
glibc, but we are compiling glob.c anyway.  Will that ever happen?

Answering my own question: yes it will, in general, because perhaps
the gnulib version fixes a bug that's in the glibc version, and the
user wants the gnulib version.  So we should worry about this.
And (as far as I can see) the simplest fix is to do this:

#if defined _LIBC || HAVE_SYS_CDEFS_H
# include 
#endif

with the appropriate change to glob.m4 to define HAVE_SYS_CDEFS_H.


___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Paul Eggert wrote:

>Answering my own question: yes it will, in general, because perhaps
>the gnulib version fixes a bug that's in the glibc version, and the
>user wants the gnulib version. So we should worry about this.
>And (as far as I can see) the simplest fix is to do this:
>
>#if defined _LIBC || HAVE_SYS_CDEFS_H
># include 
>#endif
>
>with the appropriate change to glob.m4 to define HAVE_SYS_CDEFS_H.


Done.  Patch delayed.

Regards,

Derek
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.0 (Cygwin)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFCi2ApLD1OTBfyMaQRAtapAJ47AVMRrJV5FEhKkybHBWaYRrcrXgCgspg9
PuWjxQRih2G3aP9omCCowAU=
=s6g2
-END PGP SIGNATURE-




___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Derek Price]

Paul Eggert wrote:

>>There is one change I know I made that might have an effect.  I added
>>the "!defined _LIBC" to the following block because I was unsure what
>>__REDIRECT_NTH was supposed to be doing
>>
>>
>
>I think that one's OK.
>  
>

Larry Jones on the CVS team just made a comment that makes me wonder if
it is, at least if we still plan on sharing this header with glibc... 
isn't the conversion to 64 bit file sizes and offsets supposed to be
transparent and handled by compile-time magic?

So, all the HAVE_.*64 stuff in glob.c shouldn't be necessary when !_LIBC.

Also, this particular bit in glob_.h would need to be used when !_LIBC
as well, since user-space programs compiling against the glob.h system
header from glibc would want glob64 used as compile-time magic, correct?

Please excuse me if this is way off, I don't have much experience at the
64-bit file offset stuff, myself.  One thing that leads me to believe I
may be wrong is that it looks like the glob64 from glibc 2.3.5 is only a
stub function.

Regards,

Derek



___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Paul Eggert]

Derek Price <[EMAIL PROTECTED]> writes:

> all the HAVE_.*64 stuff in glob.c shouldn't be necessary when !_LIBC.

Yes, that's correct; it should just invoke stat, opendir, etc., without
worrying about their 64-bit variants.  I thought it already did that?
If not, where does it not do it?

> Also, this particular bit in glob_.h would need to be used when !_LIBC
> as well, since user-space programs compiling against the glob.h system
> header from glibc would want glob64 used as compile-time magic, correct?

Sorry, you've lost me here (I don't know what "this particular bit"
is).  If !_LIBC, then glob_.h should simply declare glob and globfree,
using the standard (i.e., non-64-bit) type names.  These functions
will use 64-bit types if they are available, but the user code will
too and the user code won't need to worry about this.  I had thought
that this is what the code did already.


> Please excuse me if this is way off, I don't have much experience at the
> 64-bit file offset stuff, myself.

The basic idea is that GNU applications never need access to names
like stat64, open64, etc.  They just invoke stat and open.

The only reason for stat64 and open64 is for support of mixed
applications, where some of the code uses 32-bit interfaces and other
code uses 64-bit.  This is useful when you don't have access to the
source of legacy code that knows only about 32-bit interfaces.  But
GNU programmers have access to the source code, so we can simply
recompile it to use 64-bit interfaces uniformly, which we can then
simply call via their usual names stat, open, etc.


> One thing that leads me to believe I may be wrong is that it looks
> like the glob64 from glibc 2.3.5 is only a stub function.

I don't remember the details, but I vaguely recall that glob64 was a
mistake.  Let's not worry about it for gnulib.


___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Paul Eggert]

Derek Price <[EMAIL PROTECTED]> writes:

> +#else
> +/* Is there really a case where the getlogin or getlogin_r proto can come 
> from
> +   somewhere other than ?  */
> +# ifdef HAVE_GETLOGIN_R
> +extern int getlogin_r (char *, size_t);
> +# else
> +extern char *getlogin (void);
> +# endif
> +#endif

This comment is somewhat odd, and suggests that you're as troubled by the
code as I am.  How about if you replace the above by the following:

   #if ! HAVE_DECL_GETLOGIN_R
   # if ! HAVE_DECL_GETLOGIN
   char *getlogin (void);
   # endif
   static int
   getlogin_r (char *name, size_t size)
   {
 char *n = getlogin ();
 if (n)
   {
 size_t nlen = strlen (n);
 if (nlen < size)
   {
 memcpy (name, n, nlen + 1);
 return 0;
   }
 errno = ERANGE;
   }
 return -1;
   }
   #endif

and modify the rest of the code to assume that getlogin_r works, and
modify glob.m4 to check for whether getlogin and getlogin_r are
declared, rather than whether they exist.  Or, perhaps better, you can
create a getlogin_r module with the above code, and have the glob
module depend on that.

> +# ifdef HAVE___POSIX_GETPWNAM_R
> +   /* Solaris.  */
> +#  define getpwnam_r(name, bufp, buf, len, res) \
> +__posix_getpwnam_r (name, bufp, buf, len, res)
> +# endif

I don't see why this is needed.  The Solaris include files already
contain the magic necessary to convert getpwnam_r to
__posix_getpwnam_r, right?  Or is it because that depends on
_POSIX_PTHREAD_SEMANTICS?  If so, why not define that, as follows?  I
don't see why any gnulib-using application would want the nonstandard
Solaris versions of the *_r functions.

--- extensions.m4.~1.6.~2005-02-23 05:49:36 -0800
+++ extensions.m4   2005-05-24 12:35:48 -0700
@@ -21,6 +21,10 @@ AC_DEFUN([gl_USE_SYSTEM_EXTENSIONS], [
 [/* Enable extensions on Solaris.  */
 #ifndef __EXTENSIONS__
 # undef __EXTENSIONS__
+#endif
+#ifndef _POSIX_PTHREAD_SEMANTICS
+# undef _POSIX_PTHREAD_SEMANTICS
 #endif])
   AC_DEFINE([__EXTENSIONS__])
+  AC_DEFINE([_POSIX_PTHREAD_SEMANTICS])
 ])


> +#if defined _DIRENT_HAVE_D_TYPE || defined HAVE_STRUCT_DIRENT_D_TYPE
>  # define HAVE_D_TYPE 1
> -#endif

At this point it would be simpler and clearer if we removed these
lines, and removed all uses of HAVE_D_TYPE.  We can simply substitute
"#ifdef _DIRENT_HAVE_D_TYPE" for "#ifdef HAVE_D_TYPE" later.
(HAVE_STRUCT_DIRENT_D_TYPE isn't relevant for that ifdef.)


Now turning to glob_.h:

> +#if defined _LIBC || HAVE_SYS_CDEFS_H
> +# include 
> +#endif

Ouch.  I just noticed that this isn't right for the case where glob.h
is installed into /usr/include/glob.h as part of a typical C library
installation.  First, _LIBC won't be defined there.  Second, glibc
can't infringe on the user's name space by depending on HAVE_SYS_CDEFS_H.

One way to work around this is to rewrite it as follows:

#ifndef _SYS_CDEFS_H
# include 
#endif

where glob.m4 defines _SYS_CDEFS_H if sys/cdefs.h is missing.
This differs from the usual gnulib convention, but it might
be simplest.

Another possibility is to do "mkdir sys; echo '' >sys/cdefs.h" if
 is missing, so that "#include " always
works.  This is a bit yucky on the build side, though it simplifies
the source code.


> +/* We need `size_t' for the following definitions.  */
> +#ifdef _LIBC
> +__BEGIN_DECLS

The comment seems a bit misplaced now.

How about if we build on my previous suggestion above, as follows?

#ifndef _SYS_CDEFS_H
# include 
#endif
#ifndef __BEGIN_DECLS
# define __BEGIN_DECLS
# define __END_DECLS
# define __THROW
#endif

and remove the existing "define __THROW".  This makes it clearer that
these three macros are defined by sys/cdefs.h.  Then we can keep the
__BEGIN_DECLS, __THROW, etc. stuff just the way that glibc has it.


> +#if defined _LIBC && defined __USE_LARGEFILE64
> +# ifdef __USE_GNU
> +struct stat64;
> +# endif

This isn't right, since glob.h can't depend on _LIBC (as mentioned above).
Please replace "defined _LIBC" with "!defined GLOB_PREFIX", everywhere
that you see _LIBC used (other than the changes already mentioned above).

> -# if defined __GNUC__ && __GNUC__ >= 2

This doesn't look right, since when installed as a part of glibc, the
header must be useable by GCC 1, or by non-GCC compilers.  Shouldn't
we leave this stuff in?  You might want to change it to

  # if defined __GNUC__ && __GNUC__ >= 2 && !defined GLOB_PREFIX

> -#if __USE_FILE_OFFSET64 && __GNUC__ < 2
> -# define glob glob64
> -# define globfree globfree64

Similarly, this code should be left in, with the first line changed to

  #if __USE_FILE_OFFSET64 && __GNUC__ < 2 && !defined GLOB_PREFIX

> -#if !defined __USE_FILE_OFFSET64 || __GNUC__ < 2
> +#if !defined _LIBC || !defined __USE_FILE_OFFSET64

Similarly, this should be:

  #if !defined __USE_FILE_OFFSET64 || __GNUC__ < 2 || defined GLOB_PREFIX


___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.o

Re: [bug-gnulib] New GNULIB glob module?

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Paul Eggert wrote:

>Derek Price <[EMAIL PROTECTED]> writes:
>
>>+# ifdef HAVE___POSIX_GETPWNAM_R
>>+ /* Solaris. */
>>+# define getpwnam_r(name, bufp, buf, len, res) \
>>+ __posix_getpwnam_r (name, bufp, buf, len, res)
>>+# endif
>
>
>I don't see why this is needed. The Solaris include files already
>contain the magic necessary to convert getpwnam_r to
>__posix_getpwnam_r, right? Or is it because that depends on
>_POSIX_PTHREAD_SEMANTICS? If so, why not define that, as follows? I
>don't see why any gnulib-using application would want the nonstandard
>Solaris versions of the *_r functions.
>
>--- extensions.m4.~1.6.~ 2005-02-23 05:49:36 -0800
>+++ extensions.m4 2005-05-24 12:35:48 -0700
>@@ -21,6 +21,10 @@ AC_DEFUN([gl_USE_SYSTEM_EXTENSIONS], [
> [/* Enable extensions on Solaris. */
> #ifndef __EXTENSIONS__
> # undef __EXTENSIONS__
>+#endif
>+#ifndef _POSIX_PTHREAD_SEMANTICS
>+# undef _POSIX_PTHREAD_SEMANTICS
> #endif])
> AC_DEFINE([__EXTENSIONS__])
>+ AC_DEFINE([_POSIX_PTHREAD_SEMANTICS])
> ])


I really have no idea.  Larry, can you tell us if defining
_POSIX_PTHREAD_SEMANTICS would work to get the POSIX version of
getpwnam_r on Solaris?

Regards,

Derek
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.0 (Cygwin)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFClKxWLD1OTBfyMaQRAot/AKDvKB48aoCE0kys6bB3zxx0KT06sACgijeV
z/hTtSVzW3MRjWbCzAZy7pg=
=4CER
-END PGP SIGNATURE-




___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Larry Jones]

Derek Price writes:
> 
> Larry, can you tell us if defining
> _POSIX_PTHREAD_SEMANTICS would work to get the POSIX version of
> getpwnam_r on Solaris?

It looks like it.

-Larry Jones

I never get to do anything fun. -- Calvin


___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Derek Price]

Larry Jones wrote:

>Derek Price writes:
>  
>
>>Larry, can you tell us if defining
>>_POSIX_PTHREAD_SEMANTICS would work to get the POSIX version of
>>getpwnam_r on Solaris?
>>
>>
>
>It looks like it.
>  
>

I've committed Paul's patch to the CVS CVS tree, as well as removing the
associated glob.c changes, and I will install it in GNULIB if it passes
CVS nightly testing on Solaris.

Regards,

Derek



___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Paul Eggert]

Derek Price <[EMAIL PROTECTED]> writes:

> I chose the _SYS_CDEFS_H route since it seemed simplest to me, though I
> chose to name the macro `MISSING_SYS_CDEFS_H'.

Sorry, that's not right, since it fails in the following scenario:

  #define MISSING_SYS_CDEFS_H 27
  #include 

in an ordinary program that is not using gnulib, but is using glibc.
MISSING_SYS_CDEFS_H is part of the user's namespace, and should not affect
how  works.

Let's just use _SYS_CDEFS_H.  We can then remove all mention of
MISSING_SYS_CDEFS_H from glob_.h and undo the most recent change to
glob.m4.  This should be OK: if we run into some non-glibc that has
this problem it will probably use some other include file anyway.

> You told me it was safe to make this assumption originally.

Sorry, I hadn't thought through the implications of merging this back
into glibc.


___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Derek Price]

Paul Eggert wrote:

>Derek Price <[EMAIL PROTECTED]> writes:
>
>  
>
>>I chose the _SYS_CDEFS_H route since it seemed simplest to me, though I
>>chose to name the macro `MISSING_SYS_CDEFS_H'.
>>
>>
>
>Sorry, that's not right, since it fails in the following scenario:
>
>  #define MISSING_SYS_CDEFS_H 27
>  #include 
>
>in an ordinary program that is not using gnulib, but is using glibc.
>MISSING_SYS_CDEFS_H is part of the user's namespace, and should not affect
>how  works.
>
>Let's just use _SYS_CDEFS_H.  We can then remove all mention of
>MISSING_SYS_CDEFS_H from glob_.h and undo the most recent change to
>glob.m4.  This should be OK: if we run into some non-glibc that has
>this problem it will probably use some other include file anyway.
>  
>

Fair enough, but why undo the change to glob.m4?  Shouldn't I just
change the target of the AC_DEFINE from MISSING_SYS_CDEFS_H to _SYS_CDEFS_H?

Regards,

Derek



___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Paul Eggert]

Derek Price <[EMAIL PROTECTED]> writes:

> Fair enough, but why undo the change to glob.m4?  Shouldn't I just
> change the target of the AC_DEFINE from MISSING_SYS_CDEFS_H to _SYS_CDEFS_H?

Yes, you're right.

Sorry, I'd forgotten the trick that I had suggested.  (This suggests
that it deserves a nice fat comment.  :-)


___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Derek Price]

Paul Eggert wrote:

>Derek Price <[EMAIL PROTECTED]> writes:
>
>  
>
>>Fair enough, but why undo the change to glob.m4?  Shouldn't I just
>>change the target of the AC_DEFINE from MISSING_SYS_CDEFS_H to _SYS_CDEFS_H?
>>
>>
>
>Yes, you're right.
>
>Sorry, I'd forgotten the trick that I had suggested.  (This suggests
>that it deserves a nice fat comment.  :-)
>  
>

Done.  I added this comment to both glob_.h & glob.m4, with different
comment leaders, of course:

/* Note the reversal of the common HAVE_SYS_CDEFS_H idiom below.  In this
   way, #ifndef _SYS_CDEFS_H may be used to include  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 
   GNULIB shares with GLIBC is being included as a system header and not as
   part of GNULIB, in which case  may be assumed.  */

I'm going to commit what I have as the new glob module soon if noone
says otherwise.  I and the CVS team made a few more minor changes to
make it work on Solaris, BSD, and Windows, but it is now compiling and
running on some 7 diverse platforms.

After that I will write up a ChangeLog entry for the glob-glibc2gnulib
diff and submit our changes back to the glibc team, unless someone here
who is used to working with them would like to take a go at the actual
submission part.  Perhaps it would be smoother if someone already known
to the glibc team introduced me and this patch?  Is the "definitive"
version of shared files usually in glibc, gnulib, or wherever someone
last remembered to merge when this happens?

Cheers,

Derek



___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

RE: [bug-gnulib] New GNULIB glob module?

[Conrad T. Pino]

Hi Derek,

> From: Derek Price
> 
> Done.  I added this comment to both glob_.h & glob.m4, with different
> comment leaders, of course:
> 
> /* Note the reversal of the common HAVE_SYS_CDEFS_H idiom below.  In this
>way, #ifndef _SYS_CDEFS_H may be used to include  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 
>GNULIB shares with GLIBC is being included as a system header and not as
>part of GNULIB, in which case  may be assumed.  */
> 
> I'm going to commit what I have as the new glob module soon if noone
> says otherwise.  I and the CVS team made a few more minor changes to
> make it work on Solaris, BSD, and Windows, but it is now compiling and
> running on some 7 diverse platforms.
> 
> After that I will write up a ChangeLog entry for the glob-glibc2gnulib
> diff and submit our changes back to the glibc team, unless someone here
> who is used to working with them would like to take a go at the actual
> submission part.  Perhaps it would be smoother if someone already known
> to the glibc team introduced me and this patch?  Is the "definitive"
> version of shared files usually in glibc, gnulib, or wherever someone
> last remembered to merge when this happens?

The recent commit:

User: dprice  
Date: 05/05/28 10:39:04

Modified:
 /ccvs/
  config.h.in, configure
 /ccvs/windows-NT/
  config.h, config.h.in, stamp-chi

Log:
 Regenerated.

breaks the Windows build since Microsoft does NOT provide "sys/cdefs.h"
implementation.

CVS Project compiles primarily with Visual Studio 6.0 on Windows 2000.

> Cheers,

Ditto,

> Derek

Conrad

Configuration: libcvs - Win32 Debug
Compiling...
glob.c
h:\conrad\projects\cvs-1.12\lib\glob.h(29) : fatal error C1083: Cannot open 
include file: 'sys/cdefs.h': No such file or directory
Error executing cl.exe.

cvs.exe - 1 error(s), 0 warning(s)


___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Conrad T. Pino wrote:

>breaks the Windows build since Microsoft does NOT provide "sys/cdefs.h"
>implementation.


Yes.  Since we don't run configure on Windows, _SYS_CDEFS_H needed to be
defined to 1 in the windows-NT/config.h.in.in.  I've done so and
installed it.

Regards,

Derek
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.0 (Cygwin)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFCmPCxLD1OTBfyMaQRAqfrAKDcNY5MOH8PnOYdID9dZgqNxj8b6wCgy3E6
ELXlzCQfccQvTI7rfH3GpY0=
=EgB+
-END PGP SIGNATURE-




___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Paul Eggert]

Derek Price <[EMAIL PROTECTED]> writes:

> After that I will write up a ChangeLog entry for the glob-glibc2gnulib
> diff and submit our changes back to the glibc team, unless someone here
> who is used to working with them would like to take a go at the actual
> submission part.  Perhaps it would be smoother if someone already known
> to the glibc team introduced me and this patch?

Yes, probably.  I'm willing to have a go at it.

I suggest submitting two patches.

(1) the part that makes it work with gnulib.
(2) the bug fix.

It's OK for (2) to assume that (1) has already been applied.

Each patch should have its own Changelog entry.

> Is the "definitive" version of shared files usually in glibc,
> gnulib, or wherever someone last remembered to merge when this
> happens?

It's supposed to be in glibc, but we're not as good at keeping glibc
up to date as we should be.


___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Derek Price]

Paul Eggert wrote:

>Derek Price <[EMAIL PROTECTED]> writes:
>  
>
>>submission part.  Perhaps it would be smoother if someone already known
>>to the glibc team introduced me and this patch?
>>
>>
>
>Yes, probably.  I'm willing to have a go at it.
>
>I suggest submitting two patches.
>
>(1) the part that makes it work with gnulib.
>  
>

glob-glibc2gnulib11.diff attached:

2005-05-31  Derek Price  <[EMAIL PROTECTED]>
Paul Eggert  <[EMAIL PROTECTED]>

* 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.  Move definitions of GLOB_* macros to glob_.h.
(DIRENT_MUST_BE, DIRENT_MIGHT_BE_SYMLINK, DIRENT_MIGHT_BE_DIR): New
macros to abstract dirent->d_type access.
(GETPW_R_SIZE_MAX, LOGIN_NAME_MAX): New macros to abstract sysconf
access.
* glob.h: Protect/define contents as necessary to coexist with
GNULIB.


>(2) the bug fix.
>
>It's OK for (2) to assume that (1) has already been applied.
>  
>

glibc-glob-list-links.diff attached.  Despite its name, it actually does
three things:

   1. Corrects an incorrect check for a successful return from
  getlogin_r to assume only 0 means success, per the POSIX2 spec:
  .
   2. Moves the check for GLOB_MARK directory status (and the append of
  `/') into glob_in_dir, where it is more efficient than performing
  a second pass and sometimes calling stat a second time on each
  file or directory.  All calls to stat are avoided when
  dirent->d_type is available.  No call to realloc of the directory
  name is ever necessary since room for the slash can be allocated
  in the first pass.
   3. Ignores broken links only when GLOB_ONLYDIR is set.  With glibc
  versions 2.3.3 through 2.3.5, the following in an empty directory
  would return nothing:

  ln -s doesnt-exist linkname
  glob ("*", ...)

  This fix syncs with the comments in the file, syncs with the
  POSIX2 spec, restores the pre-glibc-2.3.3 behavior, and simply
  makes more sense - why should `ls *' fail to list broken links?


2005-05-31  Derek R. Price  <[EMAIL PROTECTED]>

* glob.c: #include 
(glob): Only 0 return from getlogin_r means success, according
to POSIX
2.  Move GLOB_MARK rescan into...
(glob_in_dir): ...here - it improves efficiency.  Don't fail to
return
broken links when GLOB_ONLYDIR is not set.
(link_exists_p): Rename to...
(is_dir_p): ...this and update functionality accordingly.



Regards,

Derek
--- ../glibc-2.3.5/sysdeps/generic/glob.c   2004-10-27 14:21:02.0 
-0400
+++ lib/glob.c  2005-05-31 18:12:01.0 -0400
@@ -1,4 +1,4 @@
-/* Copyright (C) 1991-2002, 2003, 2004 Free Software Foundation, Inc.
+/* Copyright (C) 1991-2002, 2003, 2004, 2005 Free Software Foundation, Inc.
This file is part of the GNU C Library.
 
The GNU C Library is free software; you can redistribute it and/or
@@ -16,23 +16,16 @@
Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
02111-1307 USA.  */
 
-/* AIX requires this to be the first thing in the file.  */
-#if defined _AIX && !defined __GNUC__
- #pragma alloca
-#endif
-
 #ifdef HAVE_CONFIG_H
 # include 
 #endif
 
-/* Enable GNU extensions in glob.h.  */
-#ifndef _GNU_SOURCE
-# define _GNU_SOURCE   1
-#endif
+#include 
 
 #include 
 #include 
 #include 
+#include 
 
 /* Outcomment the following line for production quality code.  */
 /* #define NDEBUG 1 */
@@ -40,30 +33,7 @@
 
 #include  /* Needed on stupid SunOS for assert.  */
 
-
-/* Comment out all this code if we are using the GNU C Library, and are not
-   actually compiling the library itself.  This code is part of the GNU C
-   Library, but also included in many other GNU distributions.  Compiling
-   and linking in this code is a waste when using the GNU C library
-   (especially if it is a shared library).  Rather than having every GNU
-   program understand `configure --with-gnu-libc' and omit the object files,
-   it is simpler to just do this in the source for each such file.  */
-
-#define GLOB_INTERFACE_VERSION 1
-#if !defined _LIBC && defined __GNU_LIBRARY__ && __GNU_LIBRARY__ > 1
-# include 
-# if _GNU_GLOB_INTERFACE_VERSION == GLOB_INTERFACE_VERSION
-#  define ELIDE_CODE
-# endif
-#endif
-
-#ifndef ELIDE_CODE
 #if !defined _LIBC || !defined GLOB_ONLY_P
-
-#if defined STDC_HEADERS || defined __GNU_LIBRARY__
-# include 
-#endif
-
 #if defined HAVE_UNISTD_H || defined _LIBC
 # include 
 # ifndef POSIX
@@ -73,22 +43,13 @@
 # endif
 #endif
 
-#if !defined _AMIGA && !defined VMS && !defined WINDOWS32
-# include 
-#endif
+#include 
 
-#if !defined __GNU_LIBRARY__ && !defined STDC_HEADERS
-extern int errno;
-#endif
+#inclu

Re: [bug-gnulib] New GNULIB glob module?

[Paul Eggert]

Derek Price <[EMAIL PROTECTED]> writes:

>1. Corrects an incorrect check for a successful return from
>   getlogin_r to assume only 0 means success, per the POSIX2 spec:
>   
> .
>2. Moves the check for GLOB_MARK directory status (and the append of
>   `/') into glob_in_dir, where it is more efficient than performing
>   a second pass and sometimes calling stat a second time on each
>   file or directory.  All calls to stat are avoided when
>   dirent->d_type is available.  No call to realloc of the directory
>   name is ever necessary since room for the slash can be allocated
>   in the first pass.

These changes sound reasonable, though we should submit them as
separate patches.

Is (2) independent of (3)?  (Please see below for why this is important.)


>3. Ignores broken links only when GLOB_ONLYDIR is set.  With glibc
>   versions 2.3.3 through 2.3.5, the following in an empty directory
>   would return nothing:
>
>   ln -s doesnt-exist linkname
>   glob ("*", ...)
>
>   This fix syncs with the comments in the file, syncs with the
>   POSIX2 spec, restores the pre-glibc-2.3.3 behavior, and simply
>   makes more sense - why should `ls *' fail to list broken links?

This change sounds controversial to me.  glibc 2.3.5 behaves similarly
to Solaris 8 and to Solaris 10 -- I just checked, with the following
program and with the working directory containing only a dangling
symlink:

  #include 
  #include 

  int
  main (void)
  {
glob_t g;
int r = glob ("*", 0, NULL, &g);
int i;

if (r != 0)
  {
fprintf (stderr, "glob failed (%s)\n",
 r == GLOB_NOMATCH ? "GLOB_NOMATCH"
 : r == GLOB_NOSPACE ? "GLOB_NOSPACE"
 : "other glob failure");
return 1;
  }

for (i = 0; i < g.gl_pathc; i++)
  puts (g.gl_pathv[i]);
return 0;
  }

Solaris 8 and 10 both report "glob failed (GLOB_NOMATCH)".

Let's separate (3) into a separate patch and think about it more
carefully before submitting it.

Have you investigated with FreeBSD glob does?  It seems to use
gl_lstat, which our glob doesn't.  That's odd.  What's the point of
having a gl_lstat if it's never used?


___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Derek Price]

Paul Eggert wrote:

>Derek Price <[EMAIL PROTECTED]> writes:
>
>  
>
>>   1. Corrects an incorrect check for a successful return from
>>  getlogin_r to assume only 0 means success, per the POSIX2 spec:
>>  
>> .
>>   2. Moves the check for GLOB_MARK directory status (and the append of
>>  `/') into glob_in_dir, where it is more efficient than performing
>>  a second pass and sometimes calling stat a second time on each
>>  file or directory.  All calls to stat are avoided when
>>  dirent->d_type is available.  No call to realloc of the directory
>>  name is ever necessary since room for the slash can be allocated
>>  in the first pass.
>>
>>
>
>These changes sound reasonable, though we should submit them as
>separate patches.
>  
>

The attached  glibc-understand-getlogin_r.diff may come off as a little
fuzzy, but it should work for (1).  I've also reattached the patch for 2
& 3 minus the above chunk, as glibc-glob-list-links2.diff.

>Is (2) independent of (3)?  (Please see below for why this is important.)
>  
>

(2) could be separated with a little more work, but lets finish the
discussion below before I provide a second patch.

>>   3. Ignores broken links only when GLOB_ONLYDIR is set.  With glibc
>>  versions 2.3.3 through 2.3.5, the following in an empty directory
>>  would return nothing:
>>
>>  ln -s doesnt-exist linkname
>>  glob ("*", ...)
>>
>>  This fix syncs with the comments in the file, syncs with the
>>  POSIX2 spec, restores the pre-glibc-2.3.3 behavior, and simply
>>  makes more sense - why should `ls *' fail to list broken links?
>>
>>
>
>This change sounds controversial to me.  glibc 2.3.5 behaves similarly
>  
>

It may be.  It looks like the change was intentional
(http://sources.redhat.com/cgi-bin/cvsweb.cgi/libc/sysdeps/generic/glob.c?rev=1.52&content-type=text/x-cvsweb-markup&cvsroot=glibc),
but I still disagree.

>to Solaris 8 and to Solaris 10 -- I just checked, with the following
>program and with the working directory containing only a dangling
>symlink:
>
>  #include 
>  #include 
>
>  int
>  main (void)
>  {
>glob_t g;
>int r = glob ("*", 0, NULL, &g);
>int i;
>
>if (r != 0)
>  {
>fprintf (stderr, "glob failed (%s)\n",
> r == GLOB_NOMATCH ? "GLOB_NOMATCH"
> : r == GLOB_NOSPACE ? "GLOB_NOSPACE"
> : "other glob failure");
>return 1;
>  }
>
>for (i = 0; i < g.gl_pathc; i++)
>  puts (g.gl_pathv[i]);
>return 0;
>  }
>
>Solaris 8 and 10 both report "glob failed (GLOB_NOMATCH)".
>
>Let's separate (3) into a separate patch and think about it more
>carefully before submitting it.
>  
>

This would involve creating at least one new function for (2) which
would just need to be removed again for (3), and the POSIX2 glob spec
states that glob returns GLOB_NOMATCH only when, "the pattern does not
match any existing pathname, and GLOB_NOCHECK was not set in flags." 
(http://www.opengroup.org/onlinepubs/009695399/functions/glob.html)

Nowhere does the POSIX2 glob spec specify that a broken symlink should
not be considered an "existing pathname".  After all, the link exists,
only its target does not.  Interpreted in this way, glob cannot be used
by a program which, for instance, wished to verify that all links
matching a pattern had valid targets since the broken links would not be
returned by glob.

If glob is only going to consider a link as if it is its target, then
what about the case where a matching link points to a file in the same
directory that also matches the pattern?  Should glob only return one or
the other?

Perhaps a GNU extension similar to GLOB_ONLYDIR is in order
(GLOB_VERIFY_SYMLINKS?), but I do not think glob should be making these
value judgements when the user did not request it.  It certainly does
not appear to be implied by the POSIX spec as I read it.

>Have you investigated with FreeBSD glob does?  
>

No, but I ran your test program on NetBSD 1.6.1 and it does return the
broken symlink.

I copied and pasted most of my above commentary on why this should be
considered a bug into
, where I
found the discussion that apparently caused the original "bug fix" that
broke this.

Cheers,

Derek

--- ../glob-glibc2gnulib/lib/glob.c 2005-05-31 18:12:01.0 -0400
+++ lib/glob.c  2005-05-27 13:52:28.0 -0400
@@ -538,7 +539,7 @@ glob (const char *pattern, int flags,
buflen = 20;
  name = __alloca (buflen);
 
- success = getlogin_r (name, buflen) >= 0;
+ success = getlogin_r (name, buflen) == 0;
  if (success)
{
  struct passwd *p;
--- ../glob-glibc2gnulib/lib/glob.c 2005-05-31 18:12:01.0 -0400
+++ lib/glob.c  2005-05-27 13:52:28.0 -0400
@@ -175,

Re: [bug-gnulib] New GNULIB glob module?

[Paul Eggert]

Derek Price <[EMAIL PROTECTED]> writes:

> It may be.  It looks like the change was intentional
> (http://sources.redhat.com/cgi-bin/cvsweb.cgi/libc/sysdeps/generic/glob.c?rev=1.52&content-type=text/x-cvsweb-markup&cvsroot=glibc),
> but I still disagree.

I agree with you.  Historically, the "*" pattern in the shell has
always matched dangling links.  Otherwise "rm *" wouldn't work as
desired.

> I ran your test program on NetBSD 1.6.1 and it does return the
> broken symlink.

OK, let's argue that BSD is right and Solaris is wrong, and glibc
should be fixed.  If necessary we can take this to the POSIX committee.
In the mean time we can submit the three patches.  I'll take a look
at doing that.


___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

extensions.m4 patch (was Re: [bug-gnulib] New GNULIB glob module?)

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Paul Eggert wrote:

>--- extensions.m4.~1.6.~ 2005-02-23 05:49:36 -0800
>+++ extensions.m4 2005-05-24 12:35:48 -0700
>@@ -21,6 +21,10 @@ AC_DEFUN([gl_USE_SYSTEM_EXTENSIONS], [
> [/* Enable extensions on Solaris. */
> #ifndef __EXTENSIONS__
> # undef __EXTENSIONS__
>+#endif
>+#ifndef _POSIX_PTHREAD_SEMANTICS
>+# undef _POSIX_PTHREAD_SEMANTICS
> #endif])
> AC_DEFINE([__EXTENSIONS__])
>+ AC_DEFINE([_POSIX_PTHREAD_SEMANTICS])
> ])


I've installed this since it worked as advertised.

Cheers,

Derek
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.0 (Cygwin)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFClgh2LD1OTBfyMaQRAgWtAJ9OPSu9ONkQPFzzdXu8u+DSZadXaQCgsTWK
kIxtjhOf4KIbO/mVbj+jpEc=
=091c
-END PGP SIGNATURE-




___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs