[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Paul Eggert
Subject: Re: [bug-gnulib] New GNULIB glob module?
Date: Sat, 14 May 2005 20:18:31 -0700
User-agent: Gnus/5.1006 (Gnus v5.10.6) Emacs/21.4 (gnu/linux)

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 <gnu-versions.h>
> +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 <glob.h>
  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 -0000
> @@ -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.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]