gethrxtime: fall back on gettime?
Hi Paul, What do you think of making gethrxtime fall back on gettime? Currently, if it can't find a monotonic timer, it tries gettimeofday, then resorts to using time. Those are also the last resorts of gettime. The difference is that if gethrxtime used gettime, it'd benefit by using nanotime or clock_gettime (CLOCK_REALTIME, if present, before using the fallback functions. Presuming you want to keep the current behavior where gethrxtime may fall back on using a probably-non-monotonic clock, the comment in gethrxtime.h /* Get the current time, as a count of the number of nanoseconds since an arbitrary epoch (e.g., the system boot time). This clock can't be set, is always increasing, and is nearly linear. */ should be changed to read more like the one in gethrxtime.c -- or removed altogether. While we're on the subject, how about removing gettime's use of time? If there is a system lacking all of the preceding functions, then we should find/use a function it does provide that has some sub-second precision rather than using `time (NULL)', which has none. Then we could say that the function is guaranteed to provide at least nominal sub-second precision. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: gethrxtime: fall back on gettime?
Paul Eggert <[EMAIL PROTECTED]> wrote: > Jim Meyering <[EMAIL PROTECTED]> writes: > >> What do you think of making gethrxtime fall back on gettime? > > Yes, that makes sense to me. I installed the patch below. This > also fixes the comments to match the code. Looks fine. Thanks! >> While we're on the subject, how about removing gettime's use >> of time? If there is a system lacking all of the preceding >> functions, then we should find/use a function it does provide >> that has some sub-second precision rather than using `time (NULL)', >> which has none. Then we could say that the function is guaranteed >> to provide at least nominal sub-second precision. > > Hmm, my kneejerk reaction is that there are no guarantees with clocks. The key word is `nominal' :-) When we resort to using `time (NULL)', we know that in some cases there is *no* chance of sub-second precision. > Even if we successfully invoke clock_gettime or gettimeofday, there's > still no guarantee that the clock has subsecond precision, as it could > be a gettimeofday emulator running atop a clock with 1-second > resolution. Sure, when it comes to standards and time-related functions there's never a mandated precision. But in practice, why worry? Isn't it better to avoid imposing such a limit ourselves? > Also, if we insist on not calling time(), that means we'd have to > delve into Microsoft's _ftime in order to port to mingw, right? I'd > rather avoid _ftime if I could. Assuming someone cares about the affected systems, I'd be happy to let them do it. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
why we need openat et al in the kernel
[ I have to preface this by saying I'm not interested in the attribute-related semantics of openat, but rather in the fd-relative-open--related semantics. ] Why do we need openat and related functions in the kernel? Without openat-like functions[1], it is impossible to process an arbitrarily deep hierarchy (efficiently) or even a single arbitrarily-distant file without changing the working directory. Even on systems with no PATH_MAX limitation, it is prohibitively expensive (O(depth**2)) to process a deep hierarchy without changing the working directory or using openat-like functions. What's wrong with changing the working directory? - it makes the code non-reentrant. Imagine a function that processes a directory hierarchy. In order to deal with an arbitrarily deep hierarchy, it uses open and fchdir as it traverses the tree. If this function is called in a multi-threaded application, other threads may find the current working directory changed out from under them. - it may make it impossible to return to the original working directory. For example, consider the command `rm -r /tmp/deep dot-relative'. Removing /tmp/deep may require changing into /tmp/deep then /tmp/deep/sub, then /tmp/deep/sub/2, ... If the initial working directory was not readable (so couldn't be opened) and getcwd fails, then there is no way to return and remove. Yes, this is contrived :-) Note that glibc (as of Nov 11, 2005) provides openat et. al., but that its implementation relies on the /proc file system, which isn't always usable, even on systems with the required kernel options: e.g., in a restrictive chroot environment. Why does openat have to be implemented in the kernel? Because any emulation cannot be 100% effective. Gnulib's save_cwd fails if the working directory is not readable and too deep for getcwd. restore_cwd can fail if save_cwd failed to get a file descriptor and the original directory is gone or inaccessible. So, is there any interest in adding these functions, independent of the attribute-related debate? Jim [1] The list of functions Solaris provides: openat, fchownat, fstatat, futimesat, renameat, unlinkat. Plus, a library-level function: fdopendir. In rewriting GNU rm (coreutils/src/remove.c) to use these functions, I've identified one more that is required: euidaccessat. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
openat status: new glibc emulation
A few days ago, Ulrich Drepper and I were talking, and I mentioned
the openat[*] problem (that Solaris has it, but Linux doesn't, and
that it'd be so nice to be able to use it in places like fts.c,
mkdir-p.c, remove.c, etc.). Sure, we have replacement functions
in gnulib's lib/openat.c, and they work almost all of the time,
but not always. Well, he found a way to implement it using /proc
file system support, and now it's in glibc. No fchdir needed.
This means that an openat-based rm command can now work on Linux as
well as on Solaris, when run from an unreadable working directory.
So progressive glibc-based systems will soon provide openat et al.
Thanks, Uli!
FYI, the trick is to realize that openat (FD, filename, ...) is
the same as calling open ("/proc/self/fd/NNN/FILENAME", ...) --
when you have /proc support.
This is a great complement to the existing save_cwd/restore_cwd
technique, since it lets us avoid changing away from the current
directory most of the time -- the only drawback is that glibc's
openat emulation relies on procfs support. If /proc is unavailable
or inaccessible, that emulation fails, so we still need to fall
back on using save_cwd and restore_cwd.
Note: with this new openat emulation in glibc, we'll have to
change gl_FUNC_OPENAT not to test solely for the presence of the
openat function, since the gnulib emulation has to supersede
the glibc one.
I've just change coreutils' lib/openat.c to do this:
2005-11-12 Jim Meyering <[EMAIL PROTECTED]>
Emulate openat-family functions using Linux's procfs, if possible.
Idea and some code based on Ulrich Drepper's glibc changes.
* openat.c: (BUILD_PROC_NAME): New macro.
Include , , "alloca.h" and "intprops.h".
(rpl_openat): Emulate by trying to open /proc/self/fd/%d/%s,
before falling back on save_cwd and restore_cwd.
(fdopendir, fstatat, unlinkat): Likewise.
These openat-family functions should be implemented in every kernel,
and I suppose eventually, they will be -- in the ones that matter.
In the mean time, the overhead of using their replacements is minimal,
and they work under almost all circumstances, even on systems without
procfs support.
[*] I've harped about this before. E.g., here:
<http://lists.gnu.org/archive/html/bug-coreutils/2005-04/msg00124.html>
Plus, I've finally written to the linux-kernel list,
<http://www.ussg.iu.edu/hypermail/linux/kernel/0511.1/1648.html>
requesting that they consider adding support for openat et al.
___
bug-gnulib mailing list
bug-gnulib@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: gethrxtime: fall back on gettime?
Paul Eggert <[EMAIL PROTECTED]> wrote: > Jim Meyering <[EMAIL PROTECTED]> writes: > >> Assuming someone cares about the affected systems, >> I'd be happy to let them do it. > > But in the meantime, everyone else who wanted to run on mingw would be > left high and dry, as coreutils wouldn't build. > > Is there some middle ground here, where we can generate a warning for > people building on old-fashioned systems without sub-second clocks, > without refusing to build coreutils entirely? Admittedly warnings are > often ignored, but I hope you get the idea. > > Perhaps something like the following? > > #ifdef OK_TO_USE_1S_CLOCK > ts->tv_sec = time (NULL); > ts->tv_nsec = 0; > #else > #error "Only 1-second nominal clock resolution found. Is that intended? > If so, recompile with -DOK_TO_USE_1S_CLOCK" > #endif I like it. Thanks! ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
mkstemp-safer.c must include
I found this bug by inspection. The problem is that on systems for which m4/mkstemp.c would normally make an application use the replacement function, an application calling mkstemp_safer would end up using the buggy version, because mkstemp-safer.c didn't include (for the mkstemp -> mkstemp_rpl definition). In coreutils I added a pre-release check target to ensure that this sort of thing doesn't happen again. We need something similar for gnulib. See below for more detail. 2005-11-14 Jim Meyering <[EMAIL PROTECTED]> * mkstemp-safer.c: Include , required for possible replacement of mkstemp. Index: lib/mkstemp-safer.c === RCS file: /cvsroot/gnulib/gnulib/lib/mkstemp-safer.c,v retrieving revision 1.1 diff -u -p -r1.1 mkstemp-safer.c --- lib/mkstemp-safer.c 27 Aug 2005 20:46:51 - 1.1 +++ lib/mkstemp-safer.c 14 Nov 2005 07:36:39 - @@ -18,6 +18,10 @@ /* Written by Paul Eggert. */ +#ifdef HAVE_CONFIG_H +# include +#endif + #include "stdlib-safer.h" #include Here's the list of coreutils/lib/*.c exceptions (i.e., files that are currently allowed *not* to include ) lib/bcopy.c lib/c-strtold.c lib/fnmatch_loop.c lib/full-read.c lib/imaxtostr.c lib/mempcpy.c lib/memset.c lib/offtostr.c lib/regcomp.c lib/regex_internal.c lib/regexec.c lib/safe-write.c lib/strtoll.c lib/strtoul.c lib/strtoull.c lib/strtoumax.c lib/umaxtostr.c lib/xstrtoul.c Several of these are legitimate exceptions, e.g., for the 3-5-line files that merely include some other .c file which *does* include . But others I've just grandfathered in. Of course, the list of affected files in gnulib is longer. Run this: grep -L 'include ' lib/*.c ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: mkstemp-safer.c must include
Jim Meyering <[EMAIL PROTECTED]> wrote: > The problem is that on systems for which m4/mkstemp.c > would normally make an application use the replacement > function, an application calling mkstemp_safer would end up > using the buggy version, because mkstemp-safer.c didn't > include (for the mkstemp -> mkstemp_rpl definition). > > In coreutils I added a pre-release check target > to ensure that this sort of thing doesn't happen again. > We [*really*] need something similar for gnulib. FYI, the consequences are not negligible. I've just added this coreutils NEWS entry on both the trunk and the b5_9x branch: ** Bug fixes sort and tac now use the replacement mkstemp function, and hence are no longer subject to limitations (of 26 or 32, on the maximum number of files from a given template) on HP-UX 10.20, SunOS 4.1.4, Solaris 2.5.1 OSF1/Tru64 V4.0F. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
c99+ pragma is ignored by gcc -std=gnu99
Hi Paul, In compiling coreutils with -std=gnu99, I see this new warning: xstrtod.c:33: warning: ignoring #pragma STDC FENV_ACCESS Why was this code needed? /* Tell the compiler that non-default rounding modes are used. */ #if 199901 <= __STDC_VERSION__ #pragma STDC FENV_ACCESS ON #endif The warning makes me think it's not useful -- at least not with gcc and -std=gnu99. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: c99+ pragma is ignored by gcc -std=gnu99
Paul Eggert <[EMAIL PROTECTED]> wrote: > However, for now the simplest thing to do is to remove the pragma so I > installed the following patch, in both gnulib and coreutils. > > 2005-11-15 Paul Eggert <[EMAIL PROTECTED]> > > * xstrtod.c: Don't bother with #pragma STDC FENV_ACCESS ON, as > coreutils no longer futzes with rounding modes. Thanks. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: bugs in dirname module
Paul Eggert <[EMAIL PROTECTED]> wrote:
> If we go this route, then base_name(F) cannot in general yield a
> suffix of F even on Unix systems, since we would want dir_name("a/b/")
> == "a/b" and base_name("a/b/") == ".". Hence base_name will need to
> allocate memory in general, even on Unix. On Cygwin it will need it
> to compute "./a:b".
That's a big departure from established tradition.
Do you really want to call the result base_name?
The first few uses of base_name in the coreutils that I looked at
do not seem amenable to the proposed change in semantics. E.g.,
/* Return true if the last component of NAME is `.' or `..'
This is so we don't try to recurse on `././././. ...' */
static bool
basename_is_dot_or_dotdot (const char *name)
{
char const *base = base_name (name);
return DOT_OR_DOTDOT (base);
}
> Also, src/dirname.c and src/basename.c will have to be modified to
> strip redundant trailing slashes before invoking dir_name and
> base_name.
Examples like the above (from remove.c) would also require removing
trailing slashes. So we'd have to make a writable copy and remove
trailing slashes in order to be able to use the new base_name function,
which would return a malloc'd result. And we'd have to free it, too,
of course.
That doesn't sound like an improvement.
___
bug-gnulib mailing list
bug-gnulib@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: style question - const char *
Paul Eggert <[EMAIL PROTECTED]> wrote: > Eric Blake <[EMAIL PROTECTED]> writes: > >> Is there a preference for 'const char *' over 'char const *'? > > I prefer putting type qualifiers like "const" after the types they > modify, as that's more consistent. For example, "char * const *" puts As you've probably noticed, I too prefer that. For the same reason: syntactic consistency. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: bugs in dirname module
[EMAIL PROTECTED] (Eric Blake) wrote: ... > so I am prepared to provide > this segregation into base_name() vs. last_component() > as part of my patch. I'd go along with that. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
FYI: new openat-like function: mkdirat
Thinking about how to make thread-safe the directory-creating parts
of cp -r, mv, tar, cpio, and even mkdir -p (i.e., don't change
the initial working directory), while remaining efficient even for
deep hierarchies, I realized that we need a new function, mkdirat,
which I've just checked in to coreutils/lib/mkdirat.c.
Unlike the other openat-like functions, this one is in a separate
file, since it's compiled unconditionally, which is because no
system provides it, yet... But if my mentioning it to Ulrich works
as quickly this time as it did for openat et al, it'll be in glibc
by week's end :-)
So far, only on Linux+PROC_FS can we emulate this without resorting
to the save_cwd/fchdir/restore_cwd crutch. If anyone knows how to do
the same thing using /proc on some other type of system, please tell.
Note that Solaris seems not to provide this function -- I wonder why.
2005-11-30 Jim Meyering <[EMAIL PROTECTED]>
* mkdirat.c (mkdirat): New file and function.
* openat.h (mkdirat): Declare.
2005-11-30 Jim Meyering <[EMAIL PROTECTED]>
* openat.m4 (gl_FUNC_OPENAT): Require and compile mkdirat.c.
Here is the important part of the new file:
/* Solaris 10 has no function like this.
Create a subdirectory, FILE, with mode MODE, in the directory
open on descriptor FD. If possible, do it without changing the
working directory. Otherwise, resort to using save_cwd/fchdir,
then mkdir/restore_cwd. If either the save_cwd or the restore_cwd
fails, then give a diagnostic and exit nonzero. */
int
mkdirat (int fd, char const *file, mode_t mode)
{
struct saved_cwd saved_cwd;
int saved_errno;
int err;
if (fd == AT_FDCWD || IS_ABSOLUTE_FILE_NAME (file))
return mkdir (file, mode);
{
char *proc_file;
BUILD_PROC_NAME (proc_file, fd, file);
err = mkdir (proc_file, mode);
/* If the syscall succeeds, or if it fails with an unexpected
errno value, then return right away. Otherwise, fall through
and resort to using save_cwd/restore_cwd. */
if (0 <= err || ! EXPECTED_ERRNO (errno))
return err;
}
if (save_cwd (&saved_cwd) != 0)
openat_save_fail (errno);
if (fchdir (fd) != 0)
{
saved_errno = errno;
free_cwd (&saved_cwd);
errno = saved_errno;
return -1;
}
err = mkdir (file, mode);
saved_errno = (err < 0 ? errno : 0);
if (restore_cwd (&saved_cwd) != 0)
openat_restore_fail (errno);
free_cwd (&saved_cwd);
if (saved_errno)
errno = saved_errno;
return err;
}
___
bug-gnulib mailing list
bug-gnulib@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: FYI: new openat-like function: mkdirat
Roland McGrath <[EMAIL PROTECTED]> wrote: > I think that the Solaris *at functions were really primarily intended for > use with O_XATTR to get at "file attribute" magic pseudo-directories rather > than to optimize use of normal directories and files. Probably mkdirat > doesn't make sense in Solaris attribute pseudo-directories. But mkdirat is > as useful in general as any of those *at additions, so I'd say we might as > well have it. Good! Thanks. cp, cpio, mv, and tar currently use mkfifo and mknod, so you might want to add mkfifoat and mknodat to the list, too. I haven't looked too closely at find, but its -execdir predicate makes me think having exec*at functions would be useful, too. But can glibc provide those without kernel support? ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: FYI: new openat-like function: mkdirat
Roland McGrath <[EMAIL PROTECTED]> wrote:
>> cp, cpio, mv, and tar currently use mkfifo and mknod,
>> so you might want to add mkfifoat and mknodat to the list, too.
>
> I suppose, though those are used by so few programs it is a bit more
> questionable.
Either way, it doesn't change much, since on Linux an application
can always use mkfifo ("/proc/self/fd/N/foobar", ... to work around
the absence of mkfifoat.
>> I haven't looked too closely at find, but its -execdir predicate
>> makes me think having exec*at functions would be useful, too.
>> But can glibc provide those without kernel support?
>
> We can certainly implement any such calls easily on the Hurd. :-)
> On Linux, off hand I think that the /proc/self/fd/N/foobar method works
> across the board, though I am not really sure.
I've just realized there is some ambiguity in my suggesting `exec*at'.
Unlike all of the other functions we've considered, these
have two things that may be fd-relative: the first argument
and the working directory. I was considering only the working
directory part, which doesn't fit the /proc/self/fd/N/foobar mold.
Find's -execdir predicate execs the specified command from its
current (varies through the traversal) directory, using directory
entry names as arguments.
So I guess the exec*at business would ultimately be more complicated,
with two file descriptor parameters: one identifying the working
directory, and another by which to interpret the first parameter
if it's a relative file name.
___
bug-gnulib mailing list
bug-gnulib@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: FYI: new openat-like function: mkdirat
Roland McGrath <[EMAIL PROTECTED]> wrote: >> So I guess the exec*at business would ultimately be more complicated, >> with two file descriptor parameters: one identifying the working >> directory, and another by which to interpret the first parameter >> if it's a relative file name. > > It seems adequate to just use chdir/fchdir for changing cwd, and then > execveat given the file name (you can get an fd for the original cwd before > chdir, for relative paths in exec). Using chdir/fchdir is _usually_ adequate. But what about the other times? Sometimes you *cannot* get an fd (or an absolute name) for an initial cwd. Changing cwd is problematic whenever: - your code must be thread-safe - it would be impossible to restore the initial working directory, once it's been changed -- thereafter, no reference to a `.'-relative name can be resolved. With the openat-style functions (as implemented with Linux/proc or in Solaris kernels), there is no need to change the initial working directory at all. > But note that we already have fexecve. Thanks. I didn't know about that. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: FYI: new openat-like function: mkdirat
Daniel Jacobowitz <[EMAIL PROTECTED]> wrote: > You're talking about exec. If you're going to use execve anyway, > there's no way you need your old initial working directory back, is > there? Right. As James pointed out, the forked child can simply call fchdir just before execvp, just as find already does. So there's no need for any new exec* function, after all. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
argp --help infloop bug
You can make any argp-using program infloop simply by running it with --help and with something like ARGP_HELP_FMT=rmargin=a in the environment. Or use a valid (but small) width: ARGP_HELP_FMT=rmargin=2 $ time ARGP_HELP_FMT=rmargin=2 tar --help > /dev/null ARGP_HELP_FMT=rmargin=2 tar --help > /dev/null 35.49s user 0.17s system 97% cpu 36.648 total [Exit 130 (INT)] ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: argp --help infloop bug
"Sergey Poznyakoff" <[EMAIL PROTECTED]> wrote: > Jim Meyering <[EMAIL PROTECTED]> wrote: > >> You can make any argp-using program infloop simply by running it >> with --help and with something like ARGP_HELP_FMT=rmargin=a in the >> environment. Or use a valid (but small) width: ARGP_HELP_FMT=rmargin=2 > > Yes, indeed. Thanks for reporting. I will fix it. FYI, I've also filed it against glibc: http://sourceware.org/bugzilla/show_bug.cgi?id=2016 ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: one more warning in argp-help
Eric Blake <[EMAIL PROTECTED]> wrote: > I noticed on cygwin that I was getting a warning for buf being declared at > line argp-help.c:1895 but not used. This patch also fixes a lot of > trailing whitespace; let me know if you don't want whitespace patches. It's not my call here, but I'll give you my opinion anyhow :-) IMHO, this is pretty important. Removing trailing blanks is a worthy goal, but it is better never to mix white-space-only changes and any other type of change. The white- space-only hunks make it hard to find the significant part of the patch. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
xtime.h's xtime_nsec: s/int/long int/?
Hi Paul, I noticed that xtime.h's xtime_nsec function
uses/returns `int': static inline int xtime_nsec (xtime_t t) {
int ns = t % XTIME_PRECISION; if (ns < 0) ns += XTIME_PRECISION;
return ns; } Shouldn't it use/return `long int', to be consistent
with the type of the tv_nsec member of struct timespec? From
timespec.h: struct timespec { time_t tv_sec; long tv_nsec; };
The same goes for xtime_make's `ns' paramter. Here's a proposed
patch: 2005-12-14 Jim Meyering <[EMAIL PROTECTED]>
* xtime.h (xtime_nsec): Use `long int', not `int', to be
consistent with type of timespec.tv_nsec.
(xtime_make): Likewise for `ns' parameter.
Index: lib/xtime.h
===
RCS file: /fetish/cu/lib/xtime.h,v
retrieving revision 1.4
diff -u -p -r1.4 xtime.h
--- lib/xtime.h 29 Sep 2005 16:51:40 - 1.4
+++ lib/xtime.h 14 Dec 2005 14:23:44 -
@@ -41,7 +41,7 @@ typedef long int xtime_t;
/* Return an extended time value that contains S seconds and NS
nanoseconds, without any overflow checking. */
static inline xtime_t
-xtime_make (xtime_t s, int ns)
+xtime_make (xtime_t s, long int ns)
{
if (XTIME_PRECISION == 1)
return s;
@@ -75,10 +75,10 @@ xtime_nonnegative_nsec (xtime_t t)
}
/* Return the number of nanoseconds in T. */
-static inline int
+static inline long int
xtime_nsec (xtime_t t)
{
- int ns = t % XTIME_PRECISION;
+ long int ns = t % XTIME_PRECISION;
if (ns < 0)
ns += XTIME_PRECISION;
return ns;
___
bug-gnulib mailing list
bug-gnulib@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gnulib
FYI: new module: fprintftime
I've just checked in the files that complete the implementation of the new fprintftime module. In coreutils, both date and du now use fprintftime. The strftime.c [FPRINTFTIME] changes have been here for a while, and all of this code has been in coreutils for a couple of months. Here are the ChangeLog entries I wrote for coreutils: * strftime.c [FPRINTFTIME] (fprintftime): Provide a new interface: size_t fprintftime (FILE *fp, char const *fmt, struct tm const *tm, int utc, int nanoseconds); Background: date should not have to allocate a megabyte of virtual memory to handle a format argument like +%1048575T. When implemented with strftime, it must allocate such a buffer, use strftime to fill it in, print it, then free it. With fprintftime, it simply prints everything and exits. With no need for memory allocation, that's one fewer way to fail. * fprintftime.c, fprintftime.h: New files. ChangeLog 2005-12-16 Jim Meyering <[EMAIL PROTECTED]> * modules/fprintftime: New module. * MODULES.html.sh (Date and time ): Add fprintftime. Index: m4/ChangeLog 2005-12-16 Jim Meyering <[EMAIL PROTECTED]> * fprintftime.m4: New file. Index: lib/ChangeLog 2005-12-16 Jim Meyering <[EMAIL PROTECTED]> * fprintftime.c, fprintftime.h: New files. Index: MODULES.html.sh === RCS file: /sources/gnulib/gnulib/MODULES.html.sh,v retrieving revision 1.110 retrieving revision 1.111 diff -u -p -u -r1.110 -r1.111 --- MODULES.html.sh 11 Oct 2005 18:50:37 - 1.110 +++ MODULES.html.sh 16 Dec 2005 15:05:12 - 1.111 @@ -1456,6 +1456,7 @@ func_all_modules () func_echo "$element" func_begin_table + func_module fprintftime func_module strftime func_end_table Index: modules/fprintftime === RCS file: modules/fprintftime diff -N modules/fprintftime --- /dev/null 1 Jan 1970 00:00:00 - +++ modules/fprintftime 16 Dec 2005 15:04:59 - 1.1 @@ -0,0 +1,24 @@ +Description: +like nstrftime, but output the formatted date to a FILE* stream + +Files: +lib/fprintftime.h +lib/fprintftime.c +m4/fprintftime.m4 + +Depends-on: +strftime + +configure.ac: +gl_FPRINTFTIME + +Makefile.am: + +Include: +"fprintftime.h" + +License: +GPL + +Maintainer: +Jim Meyering Index: lib/fprintftime.c === RCS file: lib/fprintftime.c diff -N lib/fprintftime.c --- /dev/null 1 Jan 1970 00:00:00 - +++ lib/fprintftime.c 16 Dec 2005 15:07:03 - 1.1 @@ -0,0 +1,7 @@ +#ifdef HAVE_CONFIG_H +# include +#endif + +#include "fprintftime.h" +#define FPRINTFTIME 1 +#include "strftime.c" Index: lib/fprintftime.h === RCS file: lib/fprintftime.h diff -N lib/fprintftime.h --- /dev/null 1 Jan 1970 00:00:00 - +++ lib/fprintftime.h 16 Dec 2005 15:06:54 - 1.1 @@ -0,0 +1,12 @@ +#include +#include + +/* A cross between fprintf and nstrftime, that prints directly + to the output stream, without the need for the potentially + large buffer that nstrftime would require. + + Output to stream FP the result of formatting (according to the + nstrftime format string, FMT) the time data, *TM, and the UTC + and NANOSECONDS values. */ +size_t fprintftime (FILE *fp, char const *fmt, struct tm const *tm, + int utc, int nanoseconds); Index: m4/fprintftime.m4 === RCS file: m4/fprintftime.m4 diff -N m4/fprintftime.m4 --- /dev/null 1 Jan 1970 00:00:00 - +++ m4/fprintftime.m4 16 Dec 2005 15:05:47 - 1.1 @@ -0,0 +1,11 @@ +#serial 1 +dnl Copyright (C) 2005 Free Software Foundation, Inc. +dnl This file is free software; the Free Software Foundation +dnl gives unlimited permission to copy and/or distribute it, +dnl with or without modifications, as long as this notice is preserved. + +AC_DEFUN([gl_FPRINTFTIME], +[ + AC_LIBSOURCES([fprintftime.c, fprintftime.h]) + AC_LIBOBJ([fprintftime]) +]) ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: FYI: new module: fprintftime
[EMAIL PROTECTED] (James Youngman) wrote: > On Fri, Dec 16, 2005 at 04:22:06PM +0100, Jim Meyering wrote: >> I've just checked in the files that complete the >> implementation of the new fprintftime module. >> In coreutils, both date and du now use fprintftime. > > Any plan to have 'ls' do so too? Of course there is the problem that > the format changes. No. ls handles the problem a little differently. It enforces a maximum buffer length of 1000 bytes, which is probably more appropriate in this case. Otherwise, you'd be able to make an ftp server generate way too much network traffic with a few simple `dir' commands. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: dirname module calls AC_LIBOBJ unconditionally
[EMAIL PROTECTED] (Eric Blake) wrote: > So experience in gnulib has shown that slightly different semantics, > with dir_name that always mallocs, and (when my patch from a > month ago is approved) base_name that mallocs and last_component As far as I know, we're still waiting for confirmation from the FSF that they have your copyright assignment papers. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: xtime.h's xtime_nsec: s/int/long int/? (resend)
[I sent this message a few weeks ago, but it was
mistakenly MIME-format=flowed, so not very readable]
Hi Paul,
I noticed that xtime.h's xtime_nsec function uses/returns `int':
/* Return the number of nanoseconds in T. */
static inline long int
xtime_nsec (xtime_t t)
{
long int ns = t % XTIME_PRECISION;
if (ns < 0)
ns += XTIME_PRECISION;
return ns;
}
Shouldn't it use/return `long int', to be consistent with the type
of the tv_nsec member of struct timespec? From timespec.h:
struct timespec { time_t tv_sec; long tv_nsec; };
The same goes for xtime_make's `ns' paramter. Here's a proposed
patch:
2005-12-14 Jim Meyering <[EMAIL PROTECTED]>
* xtime.h (xtime_nsec): Use `long int', not `int', to be
consistent with type of timespec.tv_nsec.
(xtime_make): Likewise for `ns' parameter.
Index: lib/xtime.h
===
RCS file: /fetish/cu/lib/xtime.h,v
retrieving revision 1.4
diff -u -p -r1.4 xtime.h
--- lib/xtime.h 29 Sep 2005 16:51:40 - 1.4
+++ lib/xtime.h 14 Dec 2005 14:23:44 -
@@ -41,7 +41,7 @@ typedef long int xtime_t;
/* Return an extended time value that contains S seconds and NS
nanoseconds, without any overflow checking. */
static inline xtime_t
-xtime_make (xtime_t s, int ns)
+xtime_make (xtime_t s, long int ns)
{
if (XTIME_PRECISION == 1)
return s;
@@ -75,10 +75,10 @@ xtime_nonnegative_nsec (xtime_t t)
}
/* Return the number of nanoseconds in T. */
-static inline int
+static inline long int
xtime_nsec (xtime_t t)
{
- int ns = t % XTIME_PRECISION;
+ long int ns = t % XTIME_PRECISION;
if (ns < 0)
ns += XTIME_PRECISION;
return ns;
___
bug-gnulib mailing list
bug-gnulib@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: xtime.h's xtime_nsec: s/int/long int/? (resend)
Paul Eggert <[EMAIL PROTECTED]> wrote:
> Jim Meyering <[EMAIL PROTECTED]> writes:
>
>> [I sent this message a few weeks ago, but it was
>> mistakenly MIME-format=flowed, so not very readable]
>
> Odd, I sent a reply (only to bug-gnulib), but I see it's not archived
> so I guess it didn't get through the spam filters. Here it is again.
>
> =
>
> POSIX originally made tv_nsec 'long int' because 'int' might have been
> 16 bits. Nowadays, though, 'int' is guaranteed to be at least 32 bits
> (both by POSIX and by the GNU coding standards), so it's OK nowadays
> to use 'int' to store a tv_nsec number.
>
> I thought it a bit nicer to celebrate the fact that we don't have to
> worry about 16-bit hosts, and to use plain 'int' here. It's not a big
> deal either way, though, and if there's a problem with using 'int' we
> can change it to 'long int'.
I'm not worried about the absolute width of `int' -- just about
its width compared to that of the timespec.tv_nsec member.
Since that member has type long,
struct timespec
{
time_t tv_sec;
long tv_nsec;
};
the corresponding xtime.h parameters/functions should, too.
This came up when I tried to print an xtime_nsec value with the
same format (%ld) that I'd use to print timespec.tv_nsec member.
But that gives a warning about the potential mismatch.
___
bug-gnulib mailing list
bug-gnulib@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: version-etc upgrade to 2006
Paul Eggert <[EMAIL PROTECTED]> wrote: > I installed this: > Sync from coreutils. Thanks for all of the merge work! ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: changes for openat, mkdir-p, lchmod
It looks like I've subverted things a little: the lib/ChangeLog entry for fts.c and openat.[ch] below correspond to a change I haven't yet committed. I think it's complete, but it's rather ambitious, and requires changes (albeit small) in every program that uses fts. So I'm taking my time. I committed it (only the ChangeLog entry) by mistake when I made the version-etc.c change. I'll back it out of coreutils and gnulib now. Sorry about that. Paul Eggert <[EMAIL PROTECTED]> wrote: > I installed this to sync gnulib from coreutils. This is the biggest > change hunk; the changes all tend to depend on each other. It's still > mutating but I think this snapshot will work with other programs (it > works with GNU tar, anyway). > > The lchmod business is a bit tricky, since it uses chmod as a > substitute for lchmod. Callers are supposed to check that files are > not symbolic links before using lchmod, which obviously leads to race > conditions, but that's the best we can do on hosts that lack lchmod. > > 2006-01-09 Paul Eggert <[EMAIL PROTECTED]> ... > 2006-01-09 Jim Meyering <[EMAIL PROTECTED]> > > Sync from coreutils. > > Rewrite fts.c not to change the current working directory, > by using openat, fstatat, fdopendir, etc.. > > * lib/fts.c [! _LIBC]: Include "openat.h", "unistd--.h", and > "fcntl--.h". > [_LIBC] (fchdir): Don't undef or define; no longer used. > (FCHDIR): Define in terms of cwd_advance_fd rather than fchdir. > Now, this `function' always succeeds, and consumes its file descriptor > parameter -- so callers must not close such FDs. Update callers. > (diropen_fd, opendirat, cwd_advance_fd): New functions. > (diropen): Add parameter, SP. Adjust all callers. > Implement using diropen_fd, rather than open. > (fts_open): Initialize new member, fts_cwd_fd. > Remove fts_rft-setting code. > (fts_close): Close fts_cwd_fd, if necessary. > (__opendir2): Define in terms of opendir or opendirat, > depending on whether the FST_NOCHDIR flag is set. > (fts_build): Since fts_safe_changedir consumes its FD, and since > this code must do `closedir(dirp)', dup the dirfd(dirp) argument, > and close the dup'd file descriptor upon failure. > (fts_stat): Use fstatat(...AT_SYMLINK_NOFOLLOW) in place of lstat. > (fts_safe_changedir): Tweak semantics to reflect that this function > now calls cwd_advance_fd and hence consumes its FD argument. > * lib/fts_.h [struct FTS] (fts_cwd_fd): New member. > (fts_rft): Remove now-unused member. > > * lib/openat.c (fchownat): New function. > * lib/openat.h (fchmodat, fchownat): Declare. > (chmodat, lchmodat): Define convenience functions. > (chownat, lchownat): Likewise. ... ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: bug in readutmp module
Bruno Haible <[EMAIL PROTECTED]> wrote: > The reason is that m4/readutmp.m4 invokes gl_FUNC_FREE, but m4/free.m4 is not > part of this module or its dependencies. > > Here is a fix. OK to commit? Hi Bruno! Thanks for working on this. Adding the module dependency is fine. However, I'm reluctant to remove the AC_REQUIRE, since that would make the code+.m4 combination depend silently on having a particular implementation of free. I know that there are many other instances where dependencies have moved from .m4 file to the corresponding modules/ file, and everyone should use gnulib-tool, but ... Thinking about it some more, it seems backwards to move the dependency information from the .m4 file to the module file. I think of the .m4 file as recording dependencies inherent in the corresponding source files. How about if we leave the now-redundant AC_REQUIRE in place for now? Maybe someone will make gnulib-tool automatically detect such dependencies. > 2006-01-08 Bruno Haible <[EMAIL PROTECTED]> > > * m4/readutmp.m4 (gl_READUTMP): Don't require gl_FUNC_FREE. Use a > module dependency instead. > * modules/readutmp: Depend on module free. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
a real fts.c bug + fix
I discovered a long-standing bug in fts.c yesterday:
2006-01-11 Jim Meyering <[EMAIL PROTECTED]>
* fts.c (fts_stat): When following a symlink-to-directory,
don't necessarily interpret stat-fails+lstat-succeeds as indicating
a dangling symlink. That can also happen at least for ELOOP.
The fix: return FTS_SLNONE only when the stat errno is ENOENT.
FYI, this bug predates the inclusion of fts.c in coreutils.
I've included the test case, below.
If any of you know of a system with file name resolution code that
doesn't fail for a chain of 400 symlinks, or for which you get a
different diagnostic than `Too many levels of symbolic links' (ELOOP),
please provide details.
2006-01-11 Jim Meyering <[EMAIL PROTECTED]>
* tests/du/long-sloop: New file. Test for today's fts.c bug fix.
That bug could make du -L, chgrp -L, or chown -L fail to diagnose
a very long sequence of symbolic links (not necessarily a loop).
* tests/du/Makefile.am (TESTS): Add long-sloop.
Index: lib/fts.c
===
RCS file: /fetish/cu/lib/fts.c,v
retrieving revision 1.42
retrieving revision 1.43
diff -u -p -u -r1.42 -r1.43
--- lib/fts.c 11 Jan 2006 16:29:35 - 1.42
+++ lib/fts.c 11 Jan 2006 21:00:36 - 1.43
@@ -1069,7 +1069,8 @@ fts_stat(FTS *sp, register FTSENT *p, bo
if (ISSET(FTS_LOGICAL) || follow) {
if (stat(p->fts_accpath, sbp)) {
saved_errno = errno;
- if (!lstat(p->fts_accpath, sbp)) {
+ if (errno == ENOENT
+ && lstat(p->fts_accpath, sbp) == 0) {
__set_errno (0);
return (FTS_SLNONE);
}
Index: tests/du/long-sloop
===
RCS file: tests/du/long-sloop
diff -N tests/du/long-sloop
--- /dev/null 1 Jan 1970 00:00:00 -
+++ tests/du/long-sloop 12 Jan 2006 09:25:33 - 1.3
@@ -0,0 +1,68 @@
+#!/bin/sh
+# Use du to exercise a corner of fts's FTS_LOGICAL code.
+# Show that du fails with ELOOP (Too many levels of symbolic links)
+# when it encounters that condition.
+
+if test "$VERBOSE" = yes; then
+ set -x
+ du --version
+fi
+
+. $srcdir/../lang-default
+
+pwd=`pwd`
+t0=`echo "$0"|sed 's,.*/,,'`.tmp; tmp=$t0/$$
+trap 'status=$?; cd $pwd; chmod -R u+rwx $t0; rm -rf $t0 && exit $status' 0
+trap '(exit $?); exit $?' 1 2 13 15
+
+framework_failure=0
+mkdir -p $tmp || framework_failure=1
+cd $tmp || framework_failure=1
+
+# Create lots of directories, each containing a single symlink
+# pointing at the next directory in the list.
+
+# This number should be larger than the number of symlinks allowed in
+# file name resolution, but not too large as a number of entries
+# in a single directory.
+n=400
+
+dir_list=`seq $n`
+mkdir $dir_list || framework_failure=1
+for i in $dir_list; do
+ ip1=`expr $i + 1`
+ ln -s ../$ip1 $i/s || framework_failure=1
+done
+
+if test $framework_failure = 1; then
+ echo "$0: failure in testing framework" 1>&2
+ (exit 1); exit 1
+fi
+
+# If a system can handle this many symlinks in a file name,
+# just skip this test.
+file=1`printf %${n}s ' '|sed 's, ,/s,g'`
+cat $file > /dev/null 2>&1 && \
+ {
+cat <&2
+$0: Your systems appears to be able to handle more than $n symlinks
+in file name resolution, so skipping this test.
+EOF
+(exit 77); exit 77
+ }
+
+fail=0
+
+# With coreutils-5.93 there was no failure.
+# With coreutils-5.94 we get a diagnostic like this:
+# du: cannot access `1/s/s/s/.../s': Too many levels of symbolic links
+du -L 1 > /dev/null 2> out1 && fail=1
+sed "s,1/s/s/s/[/s]*','," out1 > out || fail=1
+cat <<\EOF > exp || fail=1
+du: cannot access `': Too many levels of symbolic links
+EOF
+
+cmp out exp || fail=1
+test $fail = 1 && diff out exp 2> /dev/null
+
+(exit $fail); exit $fail
___
bug-gnulib mailing list
bug-gnulib@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: a real fts.c bug + fix
Paul Eggert <[EMAIL PROTECTED]> wrote:
> Jim Meyering <[EMAIL PROTECTED]> writes:
>
>> If any of you know of a system with file name resolution code that
>> doesn't fail for a chain of 400 symlinks, or for which you get a
>> different diagnostic than `Too many levels of symbolic links' (ELOOP),
>> please provide details.
>
> On Solaris 8 through 10 (at least), perror says this for ELOOP:
>
> Number of symbolic links encountered during path name traversal exceeds
> MAXSYMLINKS
>
> which causes the coreutils tests to fail (at least, this was true as
> of about 24 hours ago, but I hadn't gotten around to looking into it
> yet). It could be a System V ism. glibc used to do the same thing,
> when it supported SunOS.
Thanks for checking that!
Here's an untested patch:
2006-01-12 Jim Meyering <[EMAIL PROTECTED]>
* tests/du/long-sloop: Adjust not to hard-code the expected
diagnostic corresponding to ELOOP. Solaris' diagnostic differs
from that of Linux/libc. Reported by Paul Eggert.
Index: long-sloop
===
RCS file: /fetish/cu/tests/du/long-sloop,v
retrieving revision 1.5
retrieving revision 1.6
diff -u -p -u -r1.5 -r1.6
--- long-sloop 12 Jan 2006 14:45:15 - 1.5
+++ long-sloop 12 Jan 2006 18:08:18 - 1.6
@@ -42,8 +42,15 @@ fi
# If a system can handle this many symlinks in a file name,
# just skip this test.
+
+# The following also serves to record in `err' the string
+# corresponding to strerror (ELOOP). This is necessary because while
+# Linux/libc gives `Too many levels of symbolic links', Solaris
+# renders it as `Number of symbolic links encountered during path
+# name traversal exceeds MAXSYMLINKS'.
+
file=1`printf %${n}s ' '|sed 's, ,/s,g'`
-cat $file > /dev/null 2>&1 && \
+cat $file > /dev/null 2> err && \
{
cat <&2
$0: Your systems appears to be able to handle more than $n symlinks
@@ -51,6 +58,7 @@ in file name resolution, so skipping thi
EOF
(exit 77); exit 77
}
+too_many=`sed 's/.*: //' err`
fail=0
@@ -58,10 +66,9 @@ fail=0
# With coreutils-5.94 we get a diagnostic like this:
# du: cannot access `1/s/s/s/.../s': Too many levels of symbolic links
du -L 1 > /dev/null 2> out1 && fail=1
-sed "s,1/s/s/s/[/s]*','," out1 > out || fail=1
-cat <<\EOF > exp || fail=1
-du: cannot access `': Too many levels of symbolic links
-EOF
+sed "s, .1/s/s/s/[/s]*',," out1 > out || fail=1
+
+echo "du: cannot access: $too_many" > exp || fail=1
cmp out exp || fail=1
test $fail = 1 && diff out exp 2> /dev/null
___
bug-gnulib mailing list
bug-gnulib@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gnulib
making fts thread-safe (no more fchdir)
[ I've checked in a pretty big (conceptually, at least) coreutils change today. Here's some of the background and justification, along with the actual patch. ] I started my little cwd/thread-safety crusade with GNU rm. Fixing the core function to be thread safe meant rewriting remove.c not to change the current working directory, while keeping it efficient for very deep hierarchies. However, note that thread-safety wasn't the only motivation for this work. A big one was simply to make the code more robust. The moment a program changes the working directory, it risks not being able to return to it. And if it fails to return to the original working directory, it can't very well continue e.g., trying to remove "."-relative names. With today's change, I've done the same thing for du, chmod, chown, and chgrp, all of which rely on fts to perform a hierarchy traversal. The core of the change is to make fts maintain a file descriptor that is open on a *virtual* working directory. So now, rather than using chdir or fchdir (which would change the per-process current working directory) to traverse a hierarchy, fts simply advances its virtual cwd-FD to each new directory. However, this new functionality requires an API change and, for best results, some kernel-supported functions named e.g., openat, fstatat, fchmodat, etc. FTS API change: == This changes the fts API. Before, callers (those not using FTS_NOCHDIR) could expect fts_read to change the current working directory so that a simple directory entry name (fts_accpath) could be used to access files in the directory in question. Now, the current working directory is never changed, and instead, each caller must use an openat-like function to access that same name. The only difference is that they must also use the new member, fts_cwd_fd, which is a file descriptor open on the virtual current working directory. So, whereas chmod(1) used to do this: if (chmod (file, new_mode) == 0) with the new API, now it does this: if (chmodat (fts->fts_cwd_fd, file, new_mode) == 0) In both cases, file is defined like this: char const *file = ent->fts_accpath; For now, this change is only in the coreutils. I don't know how many other gnulib clients (other than GNU find) use fts. If you know of a package that uses gnulib's fts[*], but that would have a hard time switching to the new API, please let me know. As you can see below, switching du.c was a no-op, and converting the other three (chmod, chgrp, chown) was very easy. [*] If you're using glibc's fts, then you should switch; it has many limitations and bugs not present in the gnulib version. Efficiency: == On systems with kernel support for functions like openat, fstatat, etc., these changes result in a small performance improvement. Solaris 9 and 10 provide most of the functions in the kernel. For Linux, these new functions will appear soon: look for the at-functions patches on LKML. E.g., <http://google.com/search?q=lkml+openat+at-functions> In the mean time, on systems like Linux with enough PROC_FS support that gnulib's openat emulation works well, there is a small performance degradation due to the overhead of building and decoding each /proc/self/fd/... file name. There is also some overhead involved in simulating fdopendir, but that will go away on glibc-based systems once people start using glibc-2.4, which provides fdopendir. On systems with neither kernel openat support nor /proc support, there will be more overhead. I haven't measured it and confess that I'm not too concerned. But we needn't worry much about efficiency. Most of the effects I've measured on Linux are insignificant in real usage (either there's so little work to do that we don't care either way, or these minor changes are buried under the cost of lots of I/O). The actual effects can be measured reliably only when all inodes are already in cache. If you do care a lot about efficiency and want to do something even on Linux while waiting for glibc-2.4, let me know: I have a glibc-specific fdopendir replacement that works -- using it removes about half of the added overhead. The caveat is that it uses knowledge of libc internals and hard-codes things it probably shouldn't. Writing an autoconf test to see if this replacement works might prove tricky. Here are the actual diffs: lib/fts.c | 217 +++--- lib/fts_.h| 14 +- m4/fts.m4 |3 src/chmod.c |3 src/chown-core.c | 14 +- tests/du/long-from-unreadable | 70 +++++ 6 files changed, 233 insertions(+), 88 deletions(-) === coreutils/lib === 2006-01-17 Jim Meyering <[EMAIL PROTECTED]> Rewrite fts.c not to change the current working direct
Re: making fts thread-safe (no more fchdir)
[EMAIL PROTECTED] (Eric Blake) wrote: >> FTS API change: >> == >> >> This changes the fts API. Before, callers (those not using FTS_NOCHDIR) >> could expect fts_read to change the current working directory so that >> a simple directory entry name (fts_accpath) could be used to access >> files in the directory in question. Now, the current working directory >> is never changed, and instead, each caller must use an openat-like >> function to access that same name. The only difference is that they >> must also use the new member, fts_cwd_fd, which is a file descriptor >> open on the virtual current working directory. > > I'm a little uncomfortable with this, since fts might be accepted into > a future version of POSIX. It is not a good idea for us to break API > with the semantics used by native versions. ... It would be a shame if POSIX were to standardize a version of fts that cannot be used in programs that are robust, efficient and thread-safe. I certainly would not use it. Of course, I thought long and hard before deciding to change the API, and I admit it is certainly possible to support both the old and the new ones, and it's not even that hard. However, I'm not sure it's worth the added complexity. Part of the problem is that the classic API is inherently buggy: it changes the current working directory. Now that we have portable (albeit not thread-safe) emulation of the openat-like functions, I find it hard to justify spending more time to obfuscate further an already-complicated bit of code. If rewriting client applications to use the new API requires lots of effort, or if there turn out to be clients that are hard to convert, I'll be more sympathetic to the cause. FYI, I did most of the work to add FTS_CWDFD, just to see -- and it's not so bad, but the result is certainly less maintainable. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: making fts thread-safe (no more fchdir)
[EMAIL PROTECTED] (James Youngman) wrote: > On Tue, Jan 17, 2006 at 10:33:18PM +0100, Jim Meyering wrote: >> FTS API change: >> == >> >> This changes the fts API. > > Is there a way for autoconf-using gnulib client s to select only the > gnulib version? I'm happy to adopt the change, as long as I don;t run > the risk of having 'configure' wrongly select the wrong fts() > implementation. Hi Jay, There's no need to worry, since the gnulib fts module uses lib/fts.c and fts_.h exclusively. I made it do that from the start, because some of the first changes I made were to the ABI. Some of the glibc and NetBSD problems that come to mind: - inappropriate member types in fts.h, e.g., `short fts_pathlen' This usually limits the max file name length to be 32K. In gnulib, the type is size_t. - cycle detection while traversing an N-level tree takes O(N**2) time This makes fts take forever for a deep (say, 50K-level) hierarchy. With gnulib's fts, it takes only O(N) time -- a big difference. Implementing this meant adding a new struct member or two. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: making fts thread-safe (no more fchdir)
I wrote:
> FYI, I did most of the work to add FTS_CWDFD, just to see -- and it's
> not so bad, but the result is certainly less maintainable.
After saying the above, I figured I should produce the actual code,
so here it is. For now at least, I don't plan to check in these changes.
This patch restores most of the code removed by the latest
delta, in order to add an fts_open option, FTS_CWDFD, to
enable the new behavior (it may not be used with FTS_NOCHDIR).
This restores the original default behavior of using chdir/fchdir
when possible.
In restoring that removed code, I spotted a minor lingering bug.
Note the `// FIXME ...' comment, below.
In the version of fts.c before the thread-safe one, that close()
call would have clobbered errno. Of course, such a comment
would never be checked in -- not deliberately, at least.
FWIW, `make check' passes with and without this patch,
but the new test, tests/du/long-from-unreadable, fails if you
don't use the FTS_CWDFD option in xfts.c.
fts.c | 145 +
fts_.h |4 +
xfts.c |4 -
3 files changed, 116 insertions(+), 37 deletions(-)
As I said, I'm reluctant to add this complexity,
without some evidence that it will be used.
Index: lib/fts_.h
===
RCS file: /fetish/cu/lib/fts_.h,v
retrieving revision 1.19
diff -u -p -r1.19 fts_.h
--- lib/fts_.h 17 Jan 2006 17:24:29 - 1.19
+++ lib/fts_.h 18 Jan 2006 21:21:36 -
@@ -72,6 +72,7 @@ typedef struct {
struct _ftsent **fts_array; /* sort array */
dev_t fts_dev; /* starting device # */
char *fts_path; /* file name for this descent */
+ int fts_rfd;/* fd for root */
int fts_cwd_fd; /* the file descriptor on which the
virtual cwd is open, or AT_FDCWD */
size_t fts_pathlen; /* sizeof(path) */
@@ -112,8 +113,9 @@ typedef struct {
So, when FTS_LOGICAL is selected, we have to use a different
mode of cycle detection: FTS_TIGHT_CYCLE_CHECK. */
# define FTS_TIGHT_CYCLE_CHECK 0x0100
+# define FTS_CWDFD 0x0200 /* FIXME */
-# define FTS_OPTIONMASK0x01ff /* valid user option mask */
+# define FTS_OPTIONMASK0x03ff /* valid user option mask */
# define FTS_NAMEONLY 0x1000 /* (private) child names only */
# define FTS_STOP 0x2000 /* (private) unrecoverable error */
Index: lib/fts.c
===
RCS file: /fetish/cu/lib/fts.c,v
retrieving revision 1.44
diff -u -p -r1.44 fts.c
--- lib/fts.c 17 Jan 2006 17:24:14 - 1.44
+++ lib/fts.c 19 Jan 2006 08:08:02 -
@@ -105,6 +105,8 @@ static char sccsid[] = "@(#)fts.c 8.6 (B
# define close __close
# undef closedir
# define closedir __closedir
+# undef fchdir
+# define fchdir __fchdir
# undef open
# define open __open
# undef opendir
@@ -178,8 +180,14 @@ static void free_dir (FTS *fts) {}
#define ISSET(opt) (sp->fts_options & (opt))
#define SET(opt) (sp->fts_options |= (opt))
-#define FCHDIR(sp, fd) (!ISSET(FTS_NOCHDIR) \
-&& (cwd_advance_fd (sp, fd), 0))
+#define RESTORE_INITIAL_CWD(sp)FCHDIR (sp, (ISSET (FTS_CWDFD) \
+? AT_FDCWD\
+: sp->fts_rfd))
+
+#define FCHDIR(sp, fd) (!ISSET(FTS_NOCHDIR)\
+&& (ISSET(FTS_CWDFD) \
+ ? (cwd_advance_fd (sp, fd), 0) \
+: fchdir(fd)))
/* fts_build flags */
@@ -262,7 +270,9 @@ static inline int
internal_function
diropen (FTS const *sp, char const *dir)
{
- return diropen_fd (sp->fts_cwd_fd, dir);
+ return (ISSET(FTS_CWDFD)
+ ? diropen_fd (sp->fts_cwd_fd, dir)
+ : open (dir, O_RDONLY | O_DIRECTORY | O_NOCTTY | O_NONBLOCK));
}
FTS *
@@ -281,6 +291,10 @@ fts_open (char * const *argv,
__set_errno (EINVAL);
return (NULL);
}
+ if ((options & FTS_NOCHDIR) && (options & FTS_CWDFD)) {
+ __set_errno (EINVAL);
+ return (NULL);
+ }
/* Allocate/initialize the stream */
if ((sp = malloc(sizeof(FTS))) == NULL)
@@ -290,12 +304,14 @@ fts_open (char * const *argv,
sp->fts_options = options;
/* Logical walks turn on NOCHDIR; symbolic links are too hard. */
- if (ISSET(FTS_LOGICAL))
+ if (ISSET(FTS_LOGICAL)) {
SET(FTS_NOCHDIR);
+ CLR(FTS_CWDFD);
+ }
/* Initialize fts_cwd_fd. */
sp->fts_cwd_fd = AT_FDCWD;
- if ( ! ISSET(FTS_NOCHDIR) && ! HAVE_OPENAT_SUPPORT)
+ if ( ISSET(FTS_CWDFD) && ! HAVE_OPENAT_SUPPORT)
{
Re: socket.h
Simon Josefsson <[EMAIL PROTECTED]> wrote: > For some reason, mingw32 uses non-POSIX names for shutdown's ... > --- socket_.h 09 Jan 2006 17:13:09 +0100 1.1 > +++ socket_.h 19 Jan 2006 14:39:07 +0100 > @@ -34,4 +34,15 @@ > # include > #endif > > +/* For shutdown(). */ > +#if !defined(SHUT_RD) && defined (SD_RECEIVE) > +# define SHUT_RD SD_RECEIVE > +#endif > +#if !defined(SHUT_WR) && defined (SD_SEND) > +# define SHUT_WR 1 > +#endif > +#if !defined(SHUT_RDWR) && defined (SD_BOTH) Hi Simon, Over the years, I've removed all unnecessary parentheses like the ones above from nearly every file in coreutils. This might be religious, but I find that those parentheses provide no benefit. Although coreutils doesn't use that module, it might be worthwhile to start following the same guideline in gnulib. Any objection to removing them? Index: lib/socket_.h === RCS file: /sources/gnulib/gnulib/lib/socket_.h,v retrieving revision 1.2 diff -u -p -r1.2 socket_.h --- lib/socket_.h 19 Jan 2006 13:45:37 - 1.2 +++ lib/socket_.h 23 Jan 2006 22:44:24 - @@ -35,13 +35,13 @@ #endif /* For shutdown(). */ -#if !defined(SHUT_RD) && defined (SD_RECEIVE) +#if !defined SHUT_RD && defined SD_RECEIVE # define SHUT_RD SD_RECEIVE #endif -#if !defined(SHUT_WR) && defined (SD_SEND) +#if !defined SHUT_WR && defined SD_SEND # define SHUT_WR 1 #endif -#if !defined(SHUT_RDWR) && defined (SD_BOTH) +#if !defined SHUT_RDWR && defined SD_BOTH # define SHUT_RDWR 2 #endif ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: making fts thread-safe (no more fchdir)
Eric Blake <[EMAIL PROTECTED]> wrote: > According to Jim Meyering on 1/19/2006 3:39 AM: >> As I said, I'm reluctant to add this complexity, >> without some evidence that it will be used. > > How about a simpler patch to gnulib: define FTS_CWDFD, then make gnulib > fts_open() fail unless FTS_CWDFD or FTS_NOCHDIR is set. In other words, Thanks for the suggestion. However, I've resolved bite the bullet and provide full API compatibility after all. That should help others (glibc) to adopt the new mode. I'll straighten things out in coreutils before inflicting any changes on gnulib. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: socket.h
Simon Josefsson <[EMAIL PROTECTED]> wrote: >> Any objection to removing [useless parentheses]? > > No, please install them. Ok. I've checked that in. > I agree. I wish 'indent' could fix this > too. Maybe it can? Even if I agree many code writing ideas given > here, I forget them all the time. I tend to forget, too, so have automated quite a few policy checks, over the years. You might try adding some checks like those in coreutils' Makefile.maint. Here are the syntax-check (sc) target names: sc_cast_of_argument_to_free sc_cast_of_x_alloc_return_value sc_cast_of_alloca_return_value sc_dd_max_sym_length sc_error_exit_success sc_file_system sc_no_if_have_config_h sc_obsolete_symbols sc_prohibit_atoi_atof sc_prohibit_jm_in_m4 sc_prohibit_assert_without_use sc_require_config_h sc_root_tests sc_space_tab sc_sun_os_names sc_system_h_headers sc_tight_scope sc_trailing_blank sc_unmarked_diagnostics sc_useless_cpp_parens For example, recently I noticed a file that included assert.h, but that didn't use assert anywhere. Obviously, it shouldn't include . The above sc_prohibit_assert_without_use rule checks for that, and found 4 or 5 more offending .c files in coreutils. If you agree with the spirit of a rule, but want to grant an exception or two, list the offending files (or regexp) in the corresponding .x-sc-* file. These tests are run only at `make distcheck' time, so I don't worry much about portability to deficient versions of programs like grep. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: openat-priv.h needs intprops.h
"Mark D. Baushke" <[EMAIL PROTECTED]> wrote: > Hi Jim, > > The openat provided file openat-priv.h tries to > include "intprops.h" but that file is not listed > in the modules/openat file as a dependency. > > The following patch seems to fix this problem for me. > > There is probably a better way to do it, but I will > leave that to you. Hi Mark, Thank you. I've applied your patch as well as this one: (Yes, it's redundant. Eventually, I hope to find the time to make this sort of duplication unnecessary, without sacrificing the security provided by recording the dependency also in the .m4 file.) 2006-01-24 Jim Meyering <[EMAIL PROTECTED]> * openat.m4 (gl_FUNC_OPENAT): Add AC_LIBSOURCES([intprops.h]). Reported by Mark D. Baushke. Index: m4/openat.m4 === RCS file: /sources/gnulib/gnulib/m4/openat.m4,v retrieving revision 1.4 retrieving revision 1.5 diff -u -p -u -r1.4 -r1.5 --- m4/openat.m49 Jan 2006 23:13:57 - 1.4 +++ m4/openat.m424 Jan 2006 19:15:21 - 1.5 @@ -1,7 +1,7 @@ -#serial 7 +#serial 8 # See if we need to use our replacement for Solaris' openat function. -dnl Copyright (C) 2004, 2005 Free Software Foundation, Inc. +dnl Copyright (C) 2004, 2005, 2006 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, dnl with or without modifications, as long as this notice is preserved. @@ -12,6 +12,7 @@ AC_DEFUN([gl_FUNC_OPENAT], [ AC_LIBSOURCES([openat.c, openat.h, openat-priv.h, openat-die.c]) AC_LIBSOURCES([mkdirat.c]) + AC_LIBSOURCES([intprops.h]) # No system provides a mkdirat function; compile it unconditionally. AC_LIBOBJ([mkdirat]) ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
removing unnecessary parentheses in #if defined (FOO)
Bruno Haible <[EMAIL PROTECTED]> wrote: > Paul Eggert wrote: >> I don't see any technical reason to prefer the parentheses. > > While I agree that there are no technical reason to put the parentheses, > I wouldn't be religious on the issue, because the majority of the C > programmers does it the other way. The same argument as for "const char *": > http://lists.gnu.org/archive/html/bug-gnulib/2006-01/msg00024.html IMHO, this is in a different class than the `char const *' vs. `const char *' preference. Personally, I'm not *too* religious about it, but you must admit the parentheses in `#if defined (SYM)' add next to nothing in readability, and actually detract as soon as you end up adding another layer of parentheses: #if (defined (S1) || defined (S2)) && defined (S3) There, even with short-named symbols, we'd come close to having to split the expression onto another line, while without the unnecessary parentheses, we gain 6 columns. But more importantly, it's more readable without the unnecessary syntax: #if (defined S1 || defined S2) && defined S3 ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: fts.c compilation error
Bruno Haible <[EMAIL PROTECTED]> wrote:
> A gnulib testdir with modules fts and quotearg fails to compile on IRIX 6.5
> with CC="cc -O":
>
> cc -O -DHAVE_CONFIG_H -I. -I. -I.. -g -c fts.c
>
> "fts.c", line 244: error(1241): declaration may not appear after executable
> statement in block
> size_t maxarglen = fts_maxarglen(argv);
> ^
Thanks.
I've just checked in a fix for that, along with a few real bug fixes:
[Hmm.. I see I've also included the change adding O_NOCTTY and O_NONBLOCK.
I'll update the ChangeLog entry to include that. ]
2006-01-21 Jim Meyering <[EMAIL PROTECTED]>
Sync from the stable (b5) branch of coreutils:
* fts.c (fts_children): Don't let close() clobber errno from
failed fchdir().
* fts.c (fts_stat): When following a symlink-to-directory,
don't necessarily interpret stat-fails+lstat-succeeds as indicating
a dangling symlink. That can also happen at least for ELOOP.
The fix: return FTS_SLNONE only when the stat errno is ENOENT.
FYI, this bug predates the inclusion of fts.c in coreutils.
* fts.c (fts_open): Put new maxarglen declaration and uses
in their own block, so pre-c99 compilers don't object.
Avoid the double-free (first in fts_read, second in fts_close) that
would occur when an `active' directory is made inaccessible (e.g.,
via chmod a-x) during a traversal.
* fts.c (fts_read): After a failed fchdir, update sp->fts_cur
before returning. Reproduce this failure by
mkdir -p a/b; cd a; chmod a-x . b
Reported by Stavros Passas.
Index: lib/fts.c
===
RCS file: /sources/gnulib/gnulib/lib/fts.c,v
retrieving revision 1.9
retrieving revision 1.10
diff -u -p -u -r1.9 -r1.10
--- lib/fts.c 10 Jan 2006 21:31:01 - 1.9
+++ lib/fts.c 25 Jan 2006 16:45:04 - 1.10
@@ -203,7 +203,10 @@ static int
internal_function
diropen (char const *dir)
{
- return open (dir, O_RDONLY | O_DIRECTORY | O_NOCTTY | O_NONBLOCK);
+ int fd = open (dir, O_RDONLY | O_DIRECTORY);
+ if (fd < 0)
+fd = open (dir, O_WRONLY | O_DIRECTORY);
+ return fd;
}
FTS *
@@ -241,9 +244,11 @@ fts_open (char * const *argv,
#ifndef MAXPATHLEN
# define MAXPATHLEN 1024
#endif
- size_t maxarglen = fts_maxarglen(argv);
- if (! fts_palloc(sp, MAX(maxarglen, MAXPATHLEN)))
- goto mem1;
+ {
+ size_t maxarglen = fts_maxarglen(argv);
+ if (! fts_palloc(sp, MAX(maxarglen, MAXPATHLEN)))
+ goto mem1;
+ }
/* Allocate/initialize root's parent. */
if ((parent = fts_alloc(sp, "", 0)) == NULL)
@@ -693,7 +698,9 @@ fts_children (register FTS *sp, int inst
return (sp->fts_child = NULL);
sp->fts_child = fts_build(sp, instr);
if (fchdir(fd)) {
+ int saved_errno = errno;
(void)close(fd);
+ __set_errno (saved_errno);
return (NULL);
}
(void)close(fd);
@@ -1066,7 +1073,8 @@ fts_stat(FTS *sp, register FTSENT *p, bo
if (ISSET(FTS_LOGICAL) || follow) {
if (stat(p->fts_accpath, sbp)) {
saved_errno = errno;
- if (!lstat(p->fts_accpath, sbp)) {
+ if (errno == ENOENT
+ && lstat(p->fts_accpath, sbp) == 0) {
__set_errno (0);
return (FTS_SLNONE);
}
___
bug-gnulib mailing list
bug-gnulib@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: module 'fts-lgpl' not complete
Bruno Haible <[EMAIL PROTECTED]> wrote: > Building the module 'fts-lgpl' on Linux/glibc fails like this: > > gcc -DHAVE_CONFIG_H -I. -I/packages/megatestdir/fts-lgpl/lib -I.. -g -O2 > -c /packages/megatestdir/fts-lgpl/lib/fts.c > /packages/megatestdir/fts-lgpl/lib/fts.c:75:20: lstat.h: No such file or > directory > > The reason is that the 'lstat' module is GPL. Hi Bruno, > Jim, would you agree to put the 'lstat' module under LGPL if we cut its > dependency from the 'xalloc' module? Sounds reasonable. > Actually, using allocsa instead of > xmalloc will also speed it up. What do you think about these two alternative > patches? The first one looks ok, but just using malloc would be simpler. IMHO, performance shouldn't be a concern here. This is a replacement function to work around deficient systems, after all. Regarding the second patch, I see no explanation for why it makes such a fundamental change (not appending `.'?). ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: fts.c compilation error
Paul Eggert <[EMAIL PROTECTED]> wrote: > Jim Meyering <[EMAIL PROTECTED]> writes: > >> [Hmm.. I see I've also included the change adding O_NOCTTY and O_NONBLOCK. >> I'll update the ChangeLog entry to include that. ] > > But the change you checked in removed those rather than adding those. > It also reintroduces the performance bug where we try to combine > O_WRONLY | O_DIRECTORY, which always fails. I assume this was > inadvertent, and that this should be reverted. Here's the part > I'm talking about: > >> - return open (dir, O_RDONLY | O_DIRECTORY | O_NOCTTY | O_NONBLOCK); >> + int fd = open (dir, O_RDONLY | O_DIRECTORY); >> + if (fd < 0) >> +fd = open (dir, O_WRONLY | O_DIRECTORY); >> + return fd; Thanks for spotting that. I'll deal with it in a few days, if you don't beat me to it. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: module 'fts-lgpl' not complete
Bruno Haible <[EMAIL PROTECTED]> wrote:
>> Regarding the second patch, I see no explanation for why it
>> makes such a fundamental change (not appending `.'?).
>
> Actually the end of that function was a bit incomplete. It should probably
> look like this:
...
Thanks for the suggestion.
I've applied a similar patch for coreutils.
One difference is the use of ENOTDIR rather than EINVAL,
since that what lstat does for names like "non-dir/.".
I don't particularly like using stat to simulate lstat, but
in this case it seems worthwhile and safe.
I'll propagate this change to gnulib once it's undergone a little testing.
2006-02-02 Jim Meyering <[EMAIL PROTECTED]>
Eliminate the unwelcome (albeit unlikely) possibility of xmalloc
failure on deficient systems, and simplify gnulib lgpl dependencies.
* lstat.c (rpl_lstat): Rewrite to use stat() in place of the
xmalloc/lstat combination. Based on a patch from Bruno Haible.
Index: lib/lstat.c
===
RCS file: /fetish/cu/lib/lstat.c,v
retrieving revision 1.10
retrieving revision 1.11
diff -c -r1.10 -r1.11
*** lib/lstat.c 22 Sep 2005 06:05:39 - 1.10
--- lib/lstat.c 2 Feb 2006 21:25:06 - 1.11
***
*** 1,6
/* Work around a bug of lstat on some systems
!Copyright (C) 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005 Free
Software Foundation, Inc.
This program is free software; you can redistribute it and/or modify
--- 1,6
/* Work around a bug of lstat on some systems
!Copyright (C) 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006
Free
Software Foundation, Inc.
This program is free software; you can redistribute it and/or modify
***
*** 30,57
#include
#include
- #include
#include
#include "stat-macros.h"
- #include "xalloc.h"
/* lstat works differently on Linux and Solaris systems. POSIX (see
!`pathname resolution' in the glossary) requires that programs like `ls'
!take into consideration the fact that FILE has a trailing slash when
!FILE is a symbolic link. On Linux systems, the lstat function already
!has the desired semantics (in treating `lstat("symlink/",sbuf)' just like
!`lstat("symlink/.",sbuf)', but on Solaris it does not.
If FILE has a trailing slash and specifies a symbolic link,
!then append a `.' to FILE and call lstat a second time. */
int
rpl_lstat (const char *file, struct stat *sbuf)
{
size_t len;
- char *new_file;
-
int lstat_result = lstat (file, sbuf);
if (lstat_result != 0 || !S_ISLNK (sbuf->st_mode))
--- 30,57
#include
#include
#include
+ #include
#include "stat-macros.h"
/* lstat works differently on Linux and Solaris systems. POSIX (see
!`pathname resolution' in the glossary) requires that programs like
!`ls' take into consideration the fact that FILE has a trailing slash
!when FILE is a symbolic link. On Linux and Solaris 10 systems, the
!lstat function already has the desired semantics (in treating
!`lstat ("symlink/", sbuf)' just like `lstat ("symlink/.", sbuf)',
!but on Solaris 9 and earlier it does not.
If FILE has a trailing slash and specifies a symbolic link,
!then use stat() to get more info on the referent of FILE.
!If the referent is a non-directory, then set errno to ENOTDIR
!and return -1. Otherwise, return stat's result. */
int
rpl_lstat (const char *file, struct stat *sbuf)
{
size_t len;
int lstat_result = lstat (file, sbuf);
if (lstat_result != 0 || !S_ISLNK (sbuf->st_mode))
***
*** 59,77
len = strlen (file);
if (len == 0 || file[len - 1] != '/')
! return lstat_result;
/* FILE refers to a symbolic link and the name ends with a slash.
! Append a `.' to FILE and repeat the lstat call. */
!
! /* Add one for the `.' we'll append, and one more for the trailing NUL. */
! new_file = xmalloc (len + 1 + 1);
! memcpy (new_file, file, len);
! new_file[len] = '.';
! new_file[len + 1] = 0;
!
! lstat_result = lstat (new_file, sbuf);
! free (new_file);
! return lstat_result;
}
--- 59,80
len = strlen (file);
if (len == 0 || file[len - 1] != '/')
! return 0;
/* FILE refers to a symbolic link and the name ends with a slash.
! Call stat() to get info about the link's referent. */
! /* If stat fails, then we do the same. */
! if (stat (file, sbuf) != 0)
! return -1;
!
! /* If FILE references a directory, return 0. */
! if (S_ISDIR (sbuf->st_mode))
! return 0;
!
rules, rules, and more (code policy) rules
Simon Josefsson <[EMAIL PROTECTED]> wrote: > Jim Meyering <[EMAIL PROTECTED]> writes: >> I tend to forget, too, so have automated quite a few policy checks, >> over the years. You might try adding some checks like those in coreutils' >> Makefile.maint. Here are the syntax-check (sc) target names: > > These are very useful tests. I'd like to adopt them in my projects. > Is there some way they could be moved to gnulib? And perhaps > modularize them somewhat. For example, even if I currently use CVS > for my projects, separating out the tests that assume CVS seems like a > good idea. Hi Simon, Nearly all of those rules -- certainly the new ones -- require a version control system. For coreutils, I'm still using cvs, but it should be easy to adapt to others. The main functionality required is to be able to list all of the version-controlled files so that we don't get false positives on non-version-controlled (e.g., generated) files that happen to be in a working directory. Currently that's done by the build-aux/cvsu script. Tweaking things to work also for svn, monotone, git, arch, mercurial, svk, etc. shouldn't be hard. You're welcome to generalize/modularize/whatever things and put the result in gnulib. Be aware that some of the rules enforce controversial (to Bruno, at least :-) policies, so it may not be reasonable to apply them to all of gnulib. But that's why every rule has an exclusion mechanism. Also, if you want to skip some of these syntax-check tests altogether, just define a Make variable, local-checks-to-skip, and set it to the corresponding list of rule names. Hmm... I saw that the above wouldn't have worked the way I intended, so I checked in this small change for coreutils/Makefile.maint: * Makefile.maint (local-checks-available): Define in terms of the expansion, $(syntax-check-rules), rather than the single, top-level target `syntax-check', so that it's easier to exclude individual rules (via $(local-checks-to-skip)). > Scripts like gnupload, announce-gen, etc may also be useful to move to > gnulib? Sure! It'd help us all stay in sync. gnupload started in automake, and is now used by quite a few other projects. Plus, I know of a few projects that are using older versions of announce-gen. Have you just volunteered? :-) ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: rules, rules, and more (code policy) rules
Simon Josefsson <[EMAIL PROTECTED]> wrote: > I'm going through Coreutils GNUmakefile and Makefile.maint to identify > useful rules. Some question pop up: > > 1) > > Is this rule generally safe? Does it assume GNU tar? Is there a real > problem solved by this, or is it just "nice"? > > # Make tar archive easier to reproduce. > export TAR_OPTIONS = --owner=0 --group=0 --numeric-owner Those options help minimize unnecessary differences between tar archives. > Further, shouldn't automake set this, if it is safe? They're all gnu-tar-specific. In Makefile.maint, I've tried to keep things simple, and independent of automake/autoconf/etc. However, I do assume that you (the developer) have GNU tools like tar, grep, and make. > 2) > > The following is not safe, --rsyncable is a new feature. > > # Do not save the original name or timestamp in the .tar.gz file. > GZIP_ENV = '--no-name --best --rsyncable' > > Perhaps this, and the previous case, should be moved to a m4 macro, to > find out whether the parameters are supported or not. Thoughts? Rather, how about keeping the tests stand-alone? For example, here's the change I've just made so that the rule works even when gzip doesn't support the --rsyncable option: Index: Makefile.maint === RCS file: /fetish/cu/Makefile.maint,v retrieving revision 1.226 diff -u -p -r1.226 Makefile.maint --- Makefile.maint 8 Feb 2006 12:44:36 - 1.226 +++ Makefile.maint 10 Feb 2006 17:43:20 - @@ -24,7 +24,9 @@ ME := Makefile.maint # Do not save the original name or timestamp in the .tar.gz file. -GZIP_ENV = '--no-name --best --rsyncable' +gzip_rsyncable := \ + $(shell gzip --help|grep rsyncable >/dev/null && echo --rsyncable) +GZIP_ENV = '--no-name --best $(gzip_rsyncable)' CVS = cvs ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: rules, rules, and more (code policy) rules
Simon Josefsson <[EMAIL PROTECTED]> wrote: > A lot of the tests look like: > > sc_cast_of_argument_to_free: > @grep -E '\ > I.e., the paths and filenames are hardcoded. Some are definitely tailored to the classic gnu lib,src lay-out -- or were just written a long time ago :-) Just using $(CVS_LIST) might be better. > Using 'find . -name *.[chly]` seem more appropriate. Or? > > Some tests do: > > sc_space_tab: > @( $(CVS_LIST) ) > /dev/null 2>&1 || : && \ > > where CVS_LIST is: > > # cvsu is part of the cvsutils package: http://www.red-bean.com/cvsutils/ > CVS_LIST = $(srcdir)/build-aux/cvsu --find --types=AFGM > > This has the problem of being tied to cvs. Even if that could be > fixed, I'm not sure there is any advantage over the above solution. > Sometimes testing generated source code files is useful too. It's useful if you `own' the tool that does the generating or otherwise contributes the violation. But if you don't (autoconf, automake, some .m4 macro), then it's just annoying. > I'm > thinking of foo.h.in and generated source code files (libidn has a few > of these). You won't get that if you only test all files in CVS. > > I'll go with > > C_SOURCE_LIST=`find . -name *.[chly]' That approach bothered me when I had alternate versions of code sitting in my working directory not checked in. But I suppose it's worth revisiting. I suggest you rewrite it to look like this instead: C_SOURCE_LIST = $(find . -name *.[chly]) Using $(...), such a variable can be expanded e.g., within `...`, and you don't have to worry about double quotes. And spaces around the `=' make it a little more readable, imho. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: rules, rules, and more (code policy) rules
Paul Eggert <[EMAIL PROTECTED]> wrote: > 2006-02-10 Paul Eggert <[EMAIL PROTECTED]> > * Makefile.maint (CVS_LIST): Don't assume cvsu is available. > (CVS_LIST_EXCEPT): New macro, to simplify exception-processing. > Most uses of CVS_LIST changed to use CVS_LIST_EXCEPT. ... Thanks for the clean-up! > (syntax-check-rules): Bring back sc_changelong. (Hmm, why did it > go away? was that an accident?) It certainly was. As penance, I've finally removed that list altogether. It was annoying always to have to add each new rule name to the list of all syntax-check rules. Now, you just create a new rule, and as long as its name starts with `sc_' it'll be used: 2006-02-11 Jim Meyering <[EMAIL PROTECTED]> * Makefile.maint (syntax-check-rules): Automatically derive this list of sc_-prefixed rule names. Index: Makefile.maint === RCS file: /fetish/cu/Makefile.maint,v retrieving revision 1.228 retrieving revision 1.230 diff -u -p -r1.228 -r1.230 --- Makefile.maint 11 Feb 2006 06:05:23 - 1.228 +++ Makefile.maint 11 Feb 2006 07:38:25 - 1.230 @@ -91,30 +91,9 @@ local-checks-available = \ local-check = $(filter-out $(local-checks-to-skip), $(local-checks-available)) +# Collect the names of rules starting with `sc_'. +syntax-check-rules := $(shell sed -n 's/^\(sc_[a-zA-Z0-9_-]*\):.*/\1/p' $(ME)) .PHONY: $(syntax-check-rules) -syntax-check-rules = \ - sc_cast_of_argument_to_free \ - sc_cast_of_x_alloc_return_value \ - sc_cast_of_alloca_return_value \ - sc_changelog \ - sc_dd_max_sym_length \ - sc_error_exit_success \ - sc_file_system \ - sc_no_if_have_config_h \ - sc_obsolete_symbols \ - sc_prohibit_atoi_atof \ - sc_prohibit_jm_in_m4 \ - sc_prohibit_assert_without_use \ - sc_require_config_h \ - sc_root_tests \ - sc_space_tab \ - sc_sun_os_names \ - sc_system_h_headers \ - sc_tight_scope \ - sc_trailing_blank \ - sc_two_space_separator_in_usage \ - sc_unmarked_diagnostics \ - sc_useless_cpp_parens syntax-check: $(syntax-check-rules) # @grep -nE '# *include <(limits|std(def|arg|bool))\.h>' \ ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: rules, rules, and more (code policy) rules
Simon Josefsson <[EMAIL PROTECTED]> wrote: > I think CVS_LIST has some disadvantages: > > * Unrelated to the core problem. The core problem is "how to list all > C code or header files". Even if you use CVS_LIST, you'll have to > filter it further, excluding generated files the maintainer has no > control over, and include generated files the maintainer has control > over. > > * Coupled to a revision system. One cause of this is that it is > impossible to distribute the tests and run them without access to > the CVS version. `find . -name *.[chly]' work fine without CVS > access. This can be useful for beginners, or for automated testing. Not sure what you mean by `CVS access'. Those rules (and cvsu in particular) do not require access to a CVS repository. However, they do rely on the CVS/ directories that you get any time you check out a working copy. > In comparison, `find . -name *.[chly]' (or, of course, something > fancier, but you'll get the idea -- it should not depend on CVS), you > have: As I said, it's easy to extend to cover other version control systems. > * Easier to predict files will be tested automatically or not. ? > * Faster (?) IMHO, any speed difference is insignificant. > * Works without CVS access > > I don't see any major disadvantages with the find-approach. Anyone In coreutils, I'd have to add far more exclusions, especially for some of the generated test-related files. More below. > else? An exclusion/inclusion system is required with any approach, > and for most of my projects, the number of files to include/exclude is > probably equal. (Rather, I think the find-approach will cause less > manual interventions for my projects, I have several generated source > files that I control.) > >>> I'm >>> thinking of foo.h.in and generated source code files (libidn has a few >>> of these). You won't get that if you only test all files in CVS. >>> >>> I'll go with >>> >>> C_SOURCE_LIST=`find . -name *.[chly]' >> >> That approach bothered me when I had alternate versions of code >> sitting in my working directory not checked in. But I suppose it's >> worth revisiting. > > Explain? Don't you want to test those versions too? Presumably, > these tests are only used when you are looking to fix stylistic If I'm considering a version of a tool like fts.c and am not ready to check it in, should I have to add exclusions for it already? Should I check in those changes to .x-sc* files? Of course not. If I created a sample input file to help debug something, do I want to be annoyed by a `make distcheck' failure because I didn't remember to remove that file? Definitely not. Another problem is the length of the command line. With coreutils, cvsu output still fits within the `...`-imposed maximum, but `find . -type f' output is too large. These are some of the reasons I rejected the use of a blind `find . ...' command to generate the list of files to check. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: rules, rules, and more (code policy) rules
Simon Josefsson <[EMAIL PROTECTED]> wrote: >>> # Make tar archive easier to reproduce. >>> export TAR_OPTIONS = --owner=0 --group=0 --numeric-owner >> >> Those options help minimize unnecessary differences between tar archives. ... > Still, I think the TAR stuff is different. You can't set TAR_OPTIONS > in gnulib.mk, because it is not portable. I see a couple of options: > > 1) Move the tar options stuff to automake proper. > 2) Make automake adhere to a TAR_OPTIONS make variable. > 3) Write a wrapper-script for 'tar' that is included, which >will set those variables if possible. What are you trying to accomplish? Making this minor optimization work also for developers who don't have GNU tar does not justify splitting things into separate files. Why do you want a TAR_OPTIONS variable? Can't you just define AMTAR to include whatever options you want? If portability is an issue, then do the same thing here that I did with gzip's --rsyncable option. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: rules, rules, and more (code policy) rules
Simon Josefsson <[EMAIL PROTECTED]> wrote: > I'm now using this (with a couple of modifications) in GNU SASL. > > One thing that struck me as very useful, is the ability to invoke > tests without having to autoreconf + configure. ... If I forget to run ./configure ..., I'd rather get a warning than have GNUmakefile run it for me. Providing the rules might be nice, as long as they're hooked to some nonstandard target. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: rules, rules, and more (code policy) rules
Simon Josefsson <[EMAIL PROTECTED]> wrote: > Simon Josefsson <[EMAIL PROTECTED]> writes: > >>> If I forget to run ./configure ..., I'd rather get a warning >>> than have GNUmakefile run it for me. >>> >>> Providing the rules might be nice, as long as they're hooked to some >>> nonstandard target. >> >> Yep, I agree. It should be possible to override this in Makefile.cfg, >> for those of us how want to invoke autoreconf + configure >> automatically. > > Here is an updated patch. Ok to install? Fine by me, with one small change that I've just made in coreutils GNUmakefile: 2006-02-13 Jim Meyering <[EMAIL PROTECTED]> * GNUmakefile (all): Emit diagnostics to stderr, not stdout. Index: GNUmakefile === RCS file: /fetish/cu/GNUmakefile,v retrieving revision 1.9 diff -u -p -r1.9 GNUmakefile --- GNUmakefile 14 May 2005 07:58:31 - 1.9 +++ GNUmakefile 13 Feb 2006 18:28:39 - @@ -46,8 +46,8 @@ include $(srcdir)/Makefile.maint else all: - @echo There seems to be no Makefile in this directory. - @echo "You must run ./configure before running \`make'." + @echo There seems to be no Makefile in this directory. 1>&2 + @echo "You must run ./configure before running \`make'." 1>&2 @exit 1 endif ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: rules, rules, and more (code policy) rules
Simon Josefsson <[EMAIL PROTECTED]> wrote: > Simon Josefsson <[EMAIL PROTECTED]> writes: > >> * build-aux/Makefile.maint: > > I will hurt my fingers over this name (and the related Makefile.cfg), > when completing for Makefile.am, so I would prefer to have names that > doesn't begin with 'Makefile'. > > gnulib.mk and gnulib-cfg.mk? > > gnulib.mk and local.mk? (Avoids auto-completion problem in previous)? > > maintainer.mk and local-maintainer.mk? Ok. How about maint.mk and maint-cfg.mk? Spelling out `maintainer' reminds me of automake's maintainer mode, and I'd rather forget my part in that :-) ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: warning in AC_CHECK_DECL
Eric Blake <[EMAIL PROTECTED]> wrote: >> Followup - with the earlier patch fixed, now AC_FUNC_STRERROR_R has a >> warning, that was making the -Wall -Werror compilation think that >> strerror_r returned int instead of char* on cygwin. >> >> 2006-02-16 Eric Blake <[EMAIL PROTECTED]> >> >> * lib/autoconf/functions.m4 (AC_FUNC_STRERROR_R): Avoid unused >> variable warning. > > With my proposed patch to AC_FUNC_STRERROR_R, gnulib's m4/strerror_r.m4 is > now out of date. Either we need to update the various gnulib macros > borrowed from CVS autoconf to override bugs in autoconf 2.59, or we need > to release autoconf 2.60. What are the remaining issues in the way of > releasing autoconf 2.60? It'd be great to see autoconf-2.60 soon, but do bear in mind that running configure with CFLAGS='-Wall -Werror' is extreme: using -Werror makes the process very fragile. I've found it useful to configure with banal options, and then to compile with stricter ones like those, but even then, I use -Werror only on recent glibc/Linux-based systems. Sometimes, especially in autoconf code snippets, it's best not to try to remove all compile warnings. Of course, if your using -Werror exposes easy-to-fix warnings, e.g., about obviously unused variables, that's great and we should fix them. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: rules, rules, and more (code policy) rules
Simon Josefsson <[EMAIL PROTECTED]> wrote: >>> Ok. >>> How about maint.mk and maint-cfg.mk? >> >> Sounds good. Installed. > > Auto-completing maint* is causing me problem already... while this is > very minor, it also occurred to me that "cfg" is rather incorrect. > maint-cfg.mk will likely contain a lot of local maintainer rules. > > How about maint.mk and local.mk? I confess that I don't like `local.mk'. Naming it maint-anything.mk reminds us that it is maintenance-related (and associated with maint.mk), so people won't expect it to adhere to the usual make-it-maximally-portable rules. Remember that we'll have to live with the name long after its contents have stabilized and you no longer have to edit it. Since completion seems to be the sticking point, can't you just create a symlink to it (with a name you can type conveniently) in your working directory? ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: hash.c compilation error
Davide Angelocola <[EMAIL PROTECTED]> wrote: > hash.c with USE_OBSTACK fails to compile with gcc 3.3.4: > hash.c:38:16: #if with no expression Define it to `1' if you want to enable that: i.e., -DUSE_OBSTACK=1 on the command line or #define USE_OBSTACK 1 ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: play nice with AC_CACHE_CHECK
Ralf Wildenhues <[EMAIL PROTECTED]> wrote: > 1) readline.m4 gives different results with cached reruns: for example, > on x86_64-unknown-linux-gnu: > > ./configure -C > -> LIBREADLINE="-lreadline -lncurses" > ./config.status --recheck > -> LIBREADLINE="-lreadline" > > This is because the commands to set the cache variable > `gl_cv_lib_readline' also adjust LIBREADLINE and LTLIBREADLINE. > > The patch below fixes that by (ab)using the cache variable to hold the > test result contents, making the COMMANDS-TO-SET-IT argument of the > AC_CACHE_CHECK macro side-effect free. Do you think the cache variable > should be renamed (for users keeping the cache over the update; not that > it was working well before anyway)? Good catch. I'm glad you didn't rename the cache variable. Having a conforming, well-known name is worth more than avoiding the possibility of a few ephemeral user problems. Besides, there may well be packages that test $gl_cv_lib_readline, and changing the name would make them fail. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: config.h inclusion leftovers
Ralf Wildenhues <[EMAIL PROTECTED]> wrote: > The following patch changes the last files over to the agreed-to style > for inclusion of `config.h'. > > * lib/mkdtemp.c, lib/setenv.c, lib/unsetenv.c: Normalize > inclusion of `config.h'. Thanks. Applied. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: config.h inclusion leftovers
Ralf Wildenhues <[EMAIL PROTECTED]> wrote: > The following patch changes the last files over to the agreed-to style > for inclusion of `config.h'. > > * lib/mkdtemp.c, lib/setenv.c, lib/unsetenv.c: Normalize > inclusion of `config.h'. Applied. Hmm. Just noticed that those files are normally mirrored from gettext (see gnulib/config/srclist.txt). Bruno, would you accept Ralf's patch so we don't have to decouple those files? I noticed some more small differences, when comparing to coreutils copies. We include unconditionally everywhere else. Why not do so here, too? * mkdtemp.c, setenv.c, unsetenv.c: Include unconditionally. How about this additional patch? Index: setenv.c === RCS file: /sources/gnulib/gnulib/lib/setenv.c,v retrieving revision 1.14 diff -u -p -r1.14 setenv.c --- setenv.c24 Feb 2006 10:09:59 - 1.14 +++ setenv.c24 Feb 2006 10:12:13 - @@ -1,4 +1,4 @@ -/* Copyright (C) 1992,1995-1999,2000-2003 Free Software Foundation, Inc. +/* Copyright (C) 1992,1995-1999,2000-2003,2005 Free Software Foundation, Inc. This file is part of the GNU C Library. This program is free software; you can redistribute it and/or modify @@ -27,9 +27,7 @@ #include #include -#if _LIBC || HAVE_UNISTD_H -# include -#endif +#include #if !_LIBC # include "allocsa.h" Index: unsetenv.c === RCS file: /sources/gnulib/gnulib/lib/unsetenv.c,v retrieving revision 1.6 diff -u -p -r1.6 unsetenv.c --- unsetenv.c 24 Feb 2006 10:09:59 - 1.6 +++ unsetenv.c 24 Feb 2006 10:12:13 - @@ -1,4 +1,4 @@ -/* Copyright (C) 1992,1995-1999,2000-2002 Free Software Foundation, Inc. +/* Copyright (C) 1992,1995-1999,2000-2002,2005 Free Software Foundation, Inc. This file is part of the GNU C Library. This program is free software; you can redistribute it and/or modify @@ -29,9 +29,7 @@ extern int errno; #include #include -#if _LIBC || HAVE_UNISTD_H -# include -#endif +#include #if !_LIBC # define __environ environ ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: translations
[EMAIL PROTECTED] (Karl Berry) wrote: > This isn't a gnulib question, but this seems like the best set of people > to ask :) -- I've recently gotten a couple new po files for Texinfo, but > submitted via email instead of through the translation project. > > I've been telling them to go through the translation project, but now I > wonder if that's the general practice, or whether you-all have accepted > translations "on the fly"? > > (I know the turbulent history here, no need to rehash it; I'm just > wondering about current procedures.) Hi Karl, For coreutils, I request that changes/additions go through the translation project. AFAIK, that is the norm. Using the translation project helps because - it requires that they pass the automated tests of the TP robot - it makes the translations available in what may be a more central location. It's probably easier (and less work for us) for translators to update them there. - some packages get the .po files directly from the TP site. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: config.h inclusion leftovers
[EMAIL PROTECTED] (Karl Berry) wrote: > Hmm. Just noticed that those files are normally mirrored from > gettext (see gnulib/config/srclist.txt). > > Yes, although I haven't "auto"updated yet because of those differences. > > Bruno, would you accept Ralf's patch so we don't have to > decouple those files? > > Per Bruno, the checking/mirroring for gettext happens off the latest > gettext *release*, not its development sources. So even if Bruno > accepts the patches, we have to decouple those files until the next > release, if we want the changes in gnulib now. (Personally I'd rather > keep mirroring them.) Ok. I've reverted the changes to those three files, for now :( If we can't clean up such little nit-picky details because of such a constraint, then maybe it's time to remove the constraint. Does anyone object to gnulib getting setenv.c and unsetenv.c from coreutils instead? I had it the `right' way six months ago, and reluctantly changed to the `#ifdef HAVE_CONFIG_H' to stay in sync with gnulib. That's backwards. gnulib should be setting the standard, not toeing some arbitrary line. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: make getopt.c build in visual studio
Simon Josefsson <[EMAIL PROTECTED]> wrote: > With this, getopt actually do seem to compile and work in Visual > Studio. Tested with the CLI in libtasn1, part of GnuTLS, with long > options. > > I'll install this shortly unless someone protests. > > 2006-02-28 Simon Josefsson <[EMAIL PROTECTED]> > > * getopt.c: Protect #include of unistd.h, for MSVS. Hi Simon, I agree with Eric. These days, we should include unistd.h unconditionally -- and we _can_, in nearly every other environment. It should be easy to create an empty lib/unistd.h, if necessary. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: `#error' vs string literal
Ralf Wildenhues <[EMAIL PROTECTED]> wrote: > Another thing struck me: > > * Paul Eggert wrote on Wed, Mar 01, 2006 at 01:48:54AM CET: >> (AC_HEADER_STDBOOL): Don't assume "#error" works. Glad you spotted that. ... > This technique is not followed consistently in gnulib (unlike Autoconf). > I remember that `#error' does not provoke failure everywhere, but don't > remember the offending compiler/system. > > But there is even a patch to the contrary in gnulib, from coreutils: > > | 2005-08-27 Jim Meyering <[EMAIL PROTECTED]> > | > | * md5.c: Use `#error' rather than a string literal to provoke > failure. > | * sha1.c: Likewise. > > I could not find corresponding discussion or bug reports. What's the > gist of this? There had been uses of #error in coreutils for so long without complaint, so I felt comfortable with making the above change. Paul, have you found a reasonable porting target (always the key :-) on which #error doesn't work? ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
rm -r can be tricked into reporting non-existent cycle
I wrote: > 2006-03-10 Jim Meyering <[EMAIL PROTECTED]> > > * src/remove.c (AD_pop_and_chdir): Fix a bug whereby a user with > write access to a directory being removed could cause the removal > of that directory to fail with an erroneous diagnostic about a > directory cycle. Reported by Vineet Chadha. For those who haven't seen it yet, here's a copy of the original report and fix: http://meyering.net/2006-03-10-rm-bug.txt It might appear on Gmane's mirror, soon (the number is just a guess): (the GNU mailman bug-coreutils archive appears to be about 4 days behind) http://article.gmane.org/gmane.comp.gnu.core-utils.bugs/6600 In CVS, the code is here: http://cvs.savannah.gnu.org/viewcvs/coreutils/src/remove.c?root=coreutils This newly-exposed cycle-detection weakness means that the gnulib cycle-check module needs an extension -- a mechanism that's not quite as ugly as what I did in remove.c. Having a single function taking both parent and child dev/inode pairs sounds rather onerous, but maybe cleanest. If it ends up being too much trouble or expense, clients like the fts-cycle module can rely solely on the existing O(depth)-memory (rather than O(1)) cycle-detection code. Bear in mind that while this bug may affect even fts-using programs like chgrp, chown, chmod (but not du, since it uses FTS_TIGHT_CYCLE_CHECK) that don't modify the hierarchy structure, it's much harder to provoke the failure when the coprocess must manage to remove a key directory as well as create lots of new ones. Technically, even the canonicalize module's use of cycle_check is susceptible, when traversing symlinks -- and should probably be fixed -- but the typical window is pretty darn small there. In this case, even I have doubts about whether it's worth the cost of a full-blown set (implemented via hash.c) just to detect this highly unlikely failure mode. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: more syntax checks
Simon Josefsson <[EMAIL PROTECTED]> wrote: ... Hi Simon, These are all already done in Makefile.maint :-) > There should be some mechanism to exclude certain files on a per-rule > basis. I'm not yet sure how to do this. Ideas appreciated. That's why coreutils' Makefile.maint uses .x-sc* files and an extra grep to filter each file list. > It should also be possible to add local syntax checks rules in > maint-cfg.mk, and have them used automatically. Mandate a specific target naming convention. Then it's easy to process all *.mk files in order to extract the matching target names. Makefile.maint does this: # Collect the names of rules starting with `sc_'. syntax-check-rules := $(shell sed -n 's/^\(sc_[a-zA-Z0-9_-]*\):.*/\1/p' $(ME)) > It should also be possible to disable certain syntax checks rules that > doesn't work well for a specific project. You can filter out such rules using a per-project setting of local-checks-to-skip = ... in Makefile.cfg. If you want to use only a few rules, you should be able simply (and concisely) to override the default definition of the checks to run. I haven't tried it, but with Makefile.maint, setting local-checks-available might be enough. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: indent target for maint.mk
Simon Josefsson <[EMAIL PROTECTED]> wrote: > Installed. > > 2006-03-03 Simon Josefsson <[EMAIL PROTECTED]> > > * build-aux/maint.mk: Add indent target. > > --- maint.mk 15 Feb 2006 11:40:27 +0100 1.2 > +++ maint.mk 03 Mar 2006 14:29:53 +0100 > @@ -50,3 +50,8 @@ > .PHONY: $(syntax-check-rules) > > syntax-check: $(syntax-check-rules) > + > +INDENT_SOURCES ?= $(C_SOURCES) > +.PHONY: indent > +indent: > + indent $(C_SOURCES) I don't like rules that modify source files like that. A little dangerous. What if I have a big pending change and this mixes in additional, unrelated changes. Messy. It's such a simple rule, I wonder if it's worth the small risk. I would find more useful a rule that tells me which files are not properly indented. E.g., send indent output to a temporary file, then compare that with the original. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: removing asctime_r, ctime_r from the time_r module
Paul Eggert <[EMAIL PROTECTED]> wrote: > I recently redisovered the fact that actime_r and ctime_r, like > asctime and ctime, are unsafe functions in the same sense that gets is > unsafe: they can overrun their output buffers and there's no simple > way for the user to detect in advance whether this will happen. Even in glibc, until a few months ago, those functions could overrun the classic (and recommended) 26-byte buffer. I reported the bugs here: http://sourceware.org/bugzilla/show_bug.cgi?id=1460 http://sourceware.org/bugzilla/show_bug.cgi?id=1459 > GNU apps shouldn't use these functions, and I propose that we remove > these function emulations from gnulib, as follows. Any objections? Good idea. Thanks for doing that. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: Bug#355810: coreutils: pwd fails on bind mount
Kenshi Muto <[EMAIL PROTECTED]> wrote:
> At Wed, 08 Mar 2006 08:59:55 +0100,
> Jim Meyering wrote:
>> Kenshi Muto <[EMAIL PROTECTED]> wrote:
>> So far, I am unable to reproduce the failure on ia32.
...
>> also, please run the failing command under strace, e.g.
>>
>> cd /home/kmuto/d-i
>> strace -o /tmp/log /bin/pwd
>
> $ strace -o /tmp/log /bin/pwd outputs following to stderr:
> umovestr: Input/output error
...
> /bin/pwd: couldn't find directory entry in `../../..' with matching i-node
Thanks.
Your strace log showed that /bin/pwd was not calling getcwd.
Yet when I build coreutils-5.94 myself on leisner.debian.org
and try the same experiment, that binary does call getcwd, and
works just fine,
leisner$ dchroot unstable /home/meyering/coreutils-5.94/src/pwd
Executing pwd in chroot: /org/leisner.debian.org/chroot/sid
/home/meyering
leisner$ dchroot unstable /bin/pwd
Executing pwd in chroot: /org/leisner.debian.org/chroot/sid
pwd: couldn't find directory entry in `../..' with matching i-node
[Exit 1]
I suspect that the arm binary was built against an older version of
libc -- one that made coreutils use its getcwd.c replacement.
I confirmed the theory by forcing coreutils to use the replacement getcwd:
gl_cv_func_getcwd_null=no gl_cv_func_getcwd_path_max=no ./configure
Then the resulting binary fails (in leisner's chroot), just as
in the example above.
Debugging that failure, I found the source of the problem: a subtle flaw
in lib/getcwd.c (which is based on glibc's sysdeps/posix/getcwd.c).
See the comments in the patch for details.
(fyi, it looks big, but most of it is due to an indentation change)
Technically, this is grounds for changing m4/getcwd.m4 to detect
the deficiency in glibc's getcwd, but performing the actual test
would be tricky, to say the least, and the replacement would be
of only marginal value, since it'd come into play only in a chroot
when the working directory name is too long for the getcwd syscall
to work. So I won't bother right now.
I've applied the following only in coreutils, for starters:
2006-03-19 Jim Meyering <[EMAIL PROTECTED]>
Work even in a chroot where d_ino values for entries in "/"
don't match the stat.st_ino values for the same names.
* getcwd.c (__getcwd): When no d_ino value matches the target inode
number, iterate through all entries again, using lstat instead.
Reported by Kenshi Muto in http://bugs.debian.org/355810.
Index: lib/getcwd.c
===
RCS file: /fetish/cu/lib/getcwd.c,v
retrieving revision 1.19
retrieving revision 1.20
diff -u -p -u -r1.19 -r1.20
--- lib/getcwd.c19 Mar 2006 17:18:32 - 1.19
+++ lib/getcwd.c19 Mar 2006 18:27:51 - 1.20
@@ -211,6 +211,7 @@ __getcwd (char *buf, size_t size)
int parent_status;
size_t dirroom;
size_t namlen;
+ bool use_d_ino = true;
/* Look at the parent directory. */
#ifdef AT_FDCWD
@@ -257,6 +258,21 @@ __getcwd (char *buf, size_t size)
NULL. */
__set_errno (0);
d = __readdir (dirstream);
+
+ /* When we've iterated through all directory entries without finding
+one with a matching d_ino, rewind the stream and consider each
+name again, but this time, using lstat. This is necessary in a
+chroot on at least one system (glibc-2.3.6 + linux 2.6.12), where
+.., ../.., ../../.., etc. all had the same device number, yet the
+d_ino values for entries in / did not match those obtained
+via lstat. */
+ if (d == NULL && errno == 0 && use_d_ino)
+ {
+ use_d_ino = false;
+ rewinddir (dirstream);
+ d = __readdir (dirstream);
+ }
+
if (d == NULL)
{
if (errno == 0)
@@ -269,58 +285,65 @@ __getcwd (char *buf, size_t size)
(d->d_name[1] == '\0' ||
(d->d_name[1] == '.' && d->d_name[2] == '\0')))
continue;
- if (MATCHING_INO (d, thisino) || mount_point)
+
+ if (use_d_ino)
{
- int entry_status;
+ bool match = (MATCHING_INO (d, thisino) || mount_point);
+ if (! match)
+ continue;
+ }
+
+ {
+ int entry_status;
#ifdef AT_FDCWD
- entry_status = fstatat (fd, d->d_name, &st, AT_SYMLINK_NOFOLLOW);
+ entry_status = fstatat (fd, d->d_name, &st, AT_SYMLINK_NOFOLLOW);
#else
- /* Compute size needed for this file name, or for the file
-name ".." in the same directory, whichever is larger.
-Room for ".."
Re: bug in use of AC_HEADER_DIRENT
Eric Blake <[EMAIL PROTECTED]> wrote:
> By the way, my patch added an "-*- Autoconf -*-" modeline, so that it
> was easier to edit in emacs (for example, AC_ macros are colorized by
> font-lock, and emacs looks for [] instead of `' as quoting pairs). Is
> it worth doing this to other m4 files in gnulib?
I wouldn't. Why don't you just add
("\\.m4$" . autoconf-mode)
to Emacs' auto-mode-alist?
The vast majority of .m4 files I edit contain autoconf code.
___
bug-gnulib mailing list
bug-gnulib@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: gnulib regex problem
Paul Eggert <[EMAIL PROTECTED]> wrote: ... > 2006-03-16 Paul Eggert <[EMAIL PROTECTED]> > > * lib/regex.h (regoff_t) [defined _REGEX_LARGE_OFFSETS]: > Typedef to long int, not to off_, as POSIX will likely change > in that direction. > > * m4/regex.m4 (gl_REGEX): Don't check for off_t, since the code > no longer needs it. Instead, check that regoff_t is as least > as wide as ptrdiff_t. > > Don't define _REGEX_WIDE_OFFSETS unless using the included regex, > so that our regex.h stays compatible with the installed regex. > This is helpful for installers who configure --without-included-regex. > Problem reported by Emanuele Giaquinta. I needed this additional change. Applied in both gnulib and coreutils: 2006-03-17 Jim Meyering <[EMAIL PROTECTED]> * regex.m4 (gl_REGEX): Fix typo in last change: s/_REGEX_WIDE_OFFSETS/_REGEX_LARGE_OFFSETS/. Index: m4/regex.m4 === RCS file: /fetish/cu/m4/regex.m4,v retrieving revision 1.43 retrieving revision 1.44 diff -u -p -u -r1.43 -r1.44 --- m4/regex.m4 17 Mar 2006 07:33:06 - 1.43 +++ m4/regex.m4 17 Mar 2006 10:07:28 - 1.44 @@ -1,4 +1,4 @@ -#serial 32 +#serial 33 # Copyright (C) 1996, 1997, 1998, 1999, 2000, 2001, 2003, 2004, 2005, # 2006 Free Software Foundation, Inc. @@ -116,7 +116,7 @@ AC_DEFUN([gl_REGEX], esac if test $ac_use_included_regex = yes; then -AC_DEFINE([_REGEX_WIDE_OFFSETS], 1, +AC_DEFINE([_REGEX_LARGE_OFFSETS], 1, [Define if you want regoff_t to be at least as wide POSIX requires.]) AC_DEFINE([re_syntax_options], [rpl_re_syntax_options], [Define to rpl_re_syntax_options if the replacement should be used.]) ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: prepare for autoconf-2.60
Ralf Wildenhues <[EMAIL PROTECTED]> wrote: > * Paul Eggert wrote on Thu, Apr 20, 2006 at 01:50:48AM CEST: >> Bruno Haible <[EMAIL PROTECTED]> writes: >> >> > + But about AC_CHECK_DECLS_ONCE: note that in >> > + autoconf >= 2.60 the symbol separator is a comma, whereas here it is >> > + whitespace. >> >> I hadn't noticed this incompatibility. It is a real hassle. > > One solution is to forbid _any_ Gnulib (or other non-Autoconf-provided) > macro to start with `AC_'. I thought this was a convention agreed upon > long ago (much longer than, say, `build-aux' as a directory name). Then > one could even write an autoupdate rule to transform gl_CHECK_DECLS_ONCE > into AC_CHECK_DECLS_ONCE and change the white-space separated list into > a M4 list at the same time (with reasonably safe heuristics).[1] That sounds fine, but there is a small downside. I've found it useful to provide fixed/improved versions of autoconf-defined AC_* macros. All I need to do is include a new .m4 file that undefs and redefines the offending macro. Then, once an official version of autoconf is available with the fix, I simply remove the interim .m4 file. That way, there is no need to rename every use of the AC_ macro in question, then to rename them back, once it's fixed. But with any luck, autoconf is stable enough these days that even if we do have to rename things once in a while, it won't be much of a burden.
Re: comments in getdelim.h
Simon Josefsson <[EMAIL PROTECTED]> wrote: >> May I add comments to getdelim.h? > > Hi! There is one in getdelim.c already. Maybe remove it? I'm not > sure what the policy is on placing function documentation, although I > usually keep them with the actual function. > > Or we could have two function descripts, one in getdelim.c and one in > getdelim.h, I don't mind. > > Perhaps we could discuss this generally, agree on a principle on > document it? My preference is as above, but it is not a strong one. Hi Simon, We've had this debate at least once before. The conclusion was that we agreed to disagree. Bruno prefers to put documentation in .h files near the public declaration, most (all?) others prefer to put it in the .c files (nearer the actual implementation). One proposal is to keep it in both places, but that violates the no-duplication tenet, since there is no easy way to keep them in sync. So far, we're at an impasse. Things owned by Bruno follow his approach. Most others put public function documentation near the definition. Don't succumb :-) Jim
adding bison's warning.m4
Any objection to adding a slight generalization of bison's warning.m4, below?
I'm going to use it in coreutils, so figured others might want to use it, too.
BTW, who's the author? Paul?
First, here's how bison uses it:
AC_ARG_ENABLE(gcc-warnings,
[ --enable-gcc-warnings turn on lots of GCC warnings (not recommended)],
[case "${enableval}" in
yes|no) ;;
*) AC_MSG_ERROR([bad value ${enableval} for gcc-warnings option]) ;;
esac],
[enableval=no])
if test "${enableval}" = yes; then
BISON_WARNING(-Werror)
AC_SUBST([WERROR_CFLAGS], [$WARNING_CFLAGS])
WARNING_CFLAGS=
BISON_WARNING(-W)
BISON_WARNING(-Wall)
BISON_WARNING(-Wcast-align)
BISON_WARNING(-Wcast-qual)
BISON_WARNING(-Wformat)
BISON_WARNING(-Wwrite-strings)
AC_SUBST([WARNING_CXXFLAGS], [$WARNING_CFLAGS])
# The following warnings are not suitable for C++.
BISON_WARNING(-Wbad-function-cast)
BISON_WARNING(-Wmissing-declarations)
BISON_WARNING(-Wmissing-prototypes)
BISON_WARNING(-Wshadow)
BISON_WARNING(-Wstrict-prototypes)
AC_DEFINE([lint], 1, [Define to 1 if the compiler is checking for lint.])
fi
Obviously not for the faint of heart.
Here's the new file:
# serial 2
# Find valid warning flags for the C Compiler.
#
# Copyright (C) 2001, 2002, 2006 Free Software Foundation, Inc.
#
# 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 of the License, 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 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
AC_DEFUN([gl_C_WARNING],
[AC_MSG_CHECKING(whether compiler accepts $1)
AC_SUBST(WARNING_CFLAGS)
ac_save_CFLAGS="$CFLAGS"
CFLAGS="$CFLAGS $1"
AC_TRY_COMPILE(,
[int x;],
WARNING_CFLAGS="$WARNING_CFLAGS $1"
AC_MSG_RESULT(yes),
AC_MSG_RESULT(no))
CFLAGS="$ac_save_CFLAGS"
])
Re: adding bison's warning.m4
Paul Eggert <[EMAIL PROTECTED]> wrote: > Jim Meyering <[EMAIL PROTECTED]> writes: > >> Any objection to adding a slight generalization of bison's warning.m4, below? > > No objection here. But the only non-whitespace change in the file you I've added it. > sent was to change its name from BISON_WARNING to gl_C_WARNING, so > that "slight generalization" has me confused. Did you intend some > other change too? No. > One idea, since we're going to change the name anyway: how about > changing the name from gl_WARNING_CFLAGS instead, since it operates by > setting CFLAGS and it sets WARNING_CFLAGS? I prefer that name, too. >> BTW, who's the author? Paul? > > I think it's Jesse Thilo. At least, he checked it into CVS in 2000, > and was the principal maintainer of Bison until 2000. I tried to reach him, but neither his pobox.com nor gnu.org addresses works.
Re: [patch] 5.95: getgrouplist
Tim Waugh <[EMAIL PROTECTED]> wrote: > Here is a patch that makes use of getgrouplist() if it is available, > for better efficiency. Hi Tim, Thanks for posting that. Making this improvement is mentioned in the TODO file, but there's a caveat. Using getgrouplist on libc-2.3.2 is documented to fail. From the man page: BUGS The glibc 2.3.2 implementation of this function is broken: it overwrites memory when the actual number of groups is larger than *ngroups. I confirmed that back in Oct 2004. I tried some simple code to use it, and it would *always* segfault. Looked like it was tramping on a libc-internal buffer. Here's the entry from coreutils/TODO: Implement Ulrich Drepper's suggestion to use getgrouplist rather than getugroups. This affects only `id', but makes a big difference on systems with many users and/or groups, and makes id usable once again on systems where access restrictions make getugroups fail. But first we'll need a run-test (either in an autoconf macro or at run time) to avoid the segfault bug in libc-2.3.2's getgrouplist. In that case, we'd revert to using a new (to-be-written) getgrouplist module that does most of what `id' already does. It'd be great if someone would write a gnulib-style getgrouplist replacement function that provides a poor-man's implementation (using something like coreutils' existing code) for systems that lack a useful function by that name. Jim
Re: getloadavg module broken
Bruno Haible <[EMAIL PROTECTED]> wrote: > Hi Jim, > > The module description of module 'getloadavg' appears to be missing the > lib/getloadavg.c source file. > > To reproduce: > $ ./gnulib-tool --dir=/smb/orlando/gnuprog/testdir --create-testdir > `./gnulib-tool --list` > ... > checking for working getline function... yes > configure: error: ././getloadavg.c is missing > > This is with autoconf-2.59 and automake-1.9.6. Hi Bruno, That is because it's looking in the wrong place. This patch fixes the immediate problem, but may cause trouble for people who put the libobj directory elsewhere. If any of you know of such a package, please let me know. Index: modules/getloadavg === RCS file: /sources/gnulib/gnulib/modules/getloadavg,v retrieving revision 1.11 diff -u -p -r1.11 getloadavg --- modules/getloadavg 26 Feb 2006 08:45:03 - 1.11 +++ modules/getloadavg 18 May 2006 22:05:42 - @@ -13,6 +13,7 @@ stdbool fcntl-safer configure.ac: +AC_CONFIG_LIBOBJ_DIR([lib]) AC_FUNC_GETLOADAVG Makefile.am:
Re: nanosleep module and mingw32
Simon Josefsson <[EMAIL PROTECTED]> wrote: ... > Wouldn't this be a good situation to have nanosleep depend on the > unistd module, and make the replacement unistd.h include winsock2.h on > mingw32 platforms? After all, nanosleep.c include unistd.h, and > unistd.h define select on some platforms. Yes, I'd prefer that. Did you (Simon) just volunteer? :)
Re: comment in getugroups.c
Bruno Haible <[EMAIL PROTECTED]> wrote: > This fixes an outdated comment in getugroups.c. getgrent et al. are part > of POSIX/XSI since the Base Specifications Version 5. Applied. Thanks.
Re: gsort problem
Paul Eggert <[EMAIL PROTECTED]> wrote: > "Simon Wing-Tang" <[EMAIL PROTECTED]> writes: > >> This time it sorted correctly and created the temporary files over >> 2GB as follows > > Thanks for testing it. I installed that patch (enclosed again below) > into gnulib and the coreutils CVS trunk. Jim, is it OK if I also > install this into the coreutils b5_9x branch? Yes. Thanks for dealing with that.
Re: getgrouplist
Bruno Haible <[EMAIL PROTECTED]> wrote:
> Jim Meyering wrote:
>> It'd be great if someone would write a gnulib-style getgrouplist
>> replacement function that provides a poor-man's implementation (using
>> something like coreutils' existing code) for systems that lack a useful
>> function by that name.
>
> Here is an implementation of the getgrouplist module you ask for.
>
> It is based on the getugroups.c file.
>
> About the glibc-2.3.2 bug workaround:
> - I don't know how to write an autoconf test for the bug (take e.g. a
> system where all users are only in their primary group).
Thanks for working on that.
How about an autoconf run test that does this:
#include
#include
int
main ()
{
int ng = 0;
gid_t buf = 0;
getgrouplist ("root", 0, &buf, &ng);
return buf != 0;
}
If at all possible, I'd like to avoid trying to detect the bug at run time.
That would have another advantage: for the few glibc-2.3.2 (debian sarge)
systems on which I've run the test program, none failed, so I suspect it's
been fixed there. We shouldn't penalize such systems.
Here's the code (from the man page) that used to get a seg fault.
#include
#include
#include
#include
int
main ()
{
int i, ng = 0;
char *user = "who"; /* username here */
gid_t *groups = NULL;
struct passwd *pw = getpwnam (user);
if (pw == NULL)
return 0;
if (getgrouplist (user, pw->pw_gid, NULL, &ng) < 0)
{
groups = (gid_t *) malloc (ng * sizeof (gid_t));
getgrouplist (user, pw->pw_gid, groups, &ng);
}
for (i = 0; i < ng; i++)
printf ("%d\n", groups[i]);
return 0;
}
-
Can any of you try the above programs on systems with 2.3.x or older
to see if they fail (nonzero exit status for the first, crash for the
second)? If you do try, please report back either way. If affected
systems are rare enough, maybe we don't need to worry about that bug
any more.
Re: Changes to the W32 part of the getpass module
Simon Josefsson <[EMAIL PROTECTED]> wrote: > Martin Lambers <[EMAIL PROTECTED]> writes: > >> Hi! >> >> I'd like to propose the following changes to the getpass module. They >> only affect the W32 part of that module. >> The first change updates the test for the native W32 API. >> The second change adds missing includes, thus fixing compilation >> warnings. > > I'm not the official maintainer of getpass, but I think the patch is > OK and suggest this ChangeLog entry: > > 2006-05-26 Martin Lambers <[EMAIL PROTECTED]> > > * getpass.c: Updates the test for the native W32 API, and adds > missing includes, thus fixing compilation warnings. > > Of course, in general, it would be better to avoid having a > win32-specific implementation of the function, but pending work > towards that goal, we shouldn't stall any improvements to the current > approach... Thanks, Simon. I agree. Would you please check it in?
Re: OSF/4.0D strtold
Paul Eggert <[EMAIL PROTECTED]> wrote: > Ralf Wildenhues <[EMAIL PROTECTED]> writes: > >> * c-strtod.m4 (gl_C99_STRTOLD): Use a link test rather than a >> compile test, for Tru64 4.0D. > > Thanks; I installed that on both gnulib and coreutils trunk. > > Jim, OK if I put it into coreutils b5_9x? Yes. Thanks again.
Re: strfile: new module
Bruno Haible <[EMAIL PROTECTED]> wrote:
> /* Read the contents of an input stream, and return it, terminated with a NUL
>byte. */
> char* fread_file (FILE* stream)
> {
How about an interface that provides a length as well as a malloc'd
buffer, so that it works also when the file contains NUL bytes?
Likewise on the output side.
Re: regex.c not 64-bit clean (?)
[EMAIL PROTECTED] (Eric Blake) wrote: ... > configuring with -Werror. Once configured, though, running > make with -Werror is good policy for portability checks. > > What I meant to ask you to try, rather than > 'configure CFLAGS=-Werror; make', was: > > $ ./configure CFLAGS=-Wall > $ make CFLAGS='-Wall -Werror' For general software development, I find that it helps to add at least -O or -O2 to the list. With optimization enabled, gcc performs more analysis, and will detect e.g., variables that may be used defined, that it wouldn't otherwise detect.
Re: assume errno
[EMAIL PROTECTED] (Karl Berry) wrote: > > OK to apply? These are the last five references within gnulib where > > we did not assume the existance of errno. > > unsetenv was synced from coreutils. Do we prefer errno consistency or > syncing? Help? Hi Karl, We want both :-) I've just fixed the one in coreutils.
Re: Openat and localcharset issues on Darwin
"Sergey Poznyakoff" <[EMAIL PROTECTED]> wrote: > Hello, > > The following gnulib-related message was obtained while compiling GNU > tar on powerpc-apple-darwin8.6.0 with GCC 4.1.1 and native ld: > > openat.c: In function 'rpl_openat': > openat.c:60: warning: 'mode_t' is promoted to 'int' when passed through '...' > openat.c:60: warning: (so you should pass 'int' not 'mode_t' to 'va_arg') > openat.c:60: note: if this code is reached, the program will abort > > /usr/bin/ld: warning multiple definitions of symbol _locale_charset > ../lib/libtar.a(localcharset.o) definition of _locale_charset in section > (__TEXT,__text) > /usr/lib/libiconv.dylib(localcharset.o) definition of _locale_charset Hi Sergey, Does changing this line: if (sizeof (mode_t) < sizeof (int)) to this: if (sizeof (mode_t) <= sizeof (int)) avoid the warning?
Re: AC_FUNC_STRFTIME
"Derek R. Price" <[EMAIL PROTECTED]> wrote: > Autoconf 2.60 declares the AC_FUNC_STRFTIME macro obsolescent since > practical porting targets without a strftime function no longer exist. > > 2006-06-28 Derek R. Price <[EMAIL PROTECTED]> > > * lib/strftime.c: Assume strftime() exists. > * m4/strftime.m4: Don't call AC_FUNC_STRFTIME. Those look good to me. Barring objections, I'll apply them tomorrow. Thanks. FYI, in spite of the fact that the module file lists "glibc" as the maintainer, we broke sync last year with my FPRINTFTIME changes.
Re: memcmp module
Paul Eggert <[EMAIL PROTECTED]> wrote: > "Derek R. Price" <[EMAIL PROTECTED]> writes: > >> The memcmp module is only useful with AC_FUNC_MEMCMP, which Autoconf >> 2.60 declares obsolescent. No other GNULIB modules reference this >> module, so could it be removed? > > I think I'd rather keep this around a leettle while longer, since > it depends on C libraries, not the C compilers themselves, and on > a freestanding C89 system the libraries might be dodgy. > > If we want to remove memcmp, we should also look at the list of C89 > modules in MODULES.html.sh (assert, dummy, exit, atexit, etc.) and > perhaps remove them as well. Some of them we'd have to keep (e.g., > mktime, which works around many common bugs even in C99 hosts) but > others could go I suppose. > > PS. By the way, when you remove m4/c-bs-a.m4, please don't forget to > update MODULES.html.sh accordingly. How about leaving it in for a while longer, but adding some sort of assertion that will trigger if ever configure detects it's needed. Here's what I did recently for coreutils: Note that it does provide a way to continue on with the build. 2006-06-18 Jim Meyering <[EMAIL PROTECTED]> * ftruncate.m4 (gl_FUNC_FTRUNCATE): If ftruncate is missing, make configure fail, and request a bug report to inform us about it. Add a comment that, barring reports to the contrary, in 2007 we'll assume ftruncate is universally available. Index: m4/ftruncate.m4 === RCS file: /fetish/cu/m4/ftruncate.m4,v retrieving revision 1.9 retrieving revision 1.10 diff -u -p -u -r1.9 -r1.10 --- m4/ftruncate.m4 22 Sep 2005 06:05:39 - 1.9 +++ m4/ftruncate.m4 18 Jun 2006 14:00:35 - 1.10 @@ -1,17 +1,29 @@ -#serial 8 +#serial 9 # See if we need to emulate a missing ftruncate function using fcntl or chsize. -# Copyright (C) 2000, 2001, 2003, 2004, 2005 Free Software Foundation, Inc. +# Copyright (C) 2000, 2001, 2003-2006 Free Software Foundation, Inc. # This file is free software; the Free Software Foundation # gives unlimited permission to copy and/or distribute it, # with or without modifications, as long as this notice is preserved. +# FIXME: remove this macro, along with all uses of HAVE_FTRUNCATE in 2007, +# if the check below provokes no reports. + AC_DEFUN([gl_FUNC_FTRUNCATE], [ AC_REPLACE_FUNCS(ftruncate) if test $ac_cv_func_ftruncate = no; then gl_PREREQ_FTRUNCATE +# If someone lacks ftruncate, make configure fail, and request +# a bug report to inform us about it. +if test x"$SKIP_FTRUNCATE_CHECK" != xyes; then + AC_MSG_FAILURE([Your system lacks the ftruncate function. + Please report this, along with the output of "uname -a", to the + bug-coreutils@gnu.org mailing list. To continue past this point, + rerun configure with SKIP_FTRUNCATE_CHECK=yes set in the environment. + E.g., env SKIP_FTRUNCATE_CHECK=yes ./configure]) +fi fi ])
Re: AC_FUNC_STRFTIME
"Derek R. Price" <[EMAIL PROTECTED]> wrote: > Autoconf 2.60 declares the AC_FUNC_STRFTIME macro obsolescent since > practical porting targets without a strftime function no longer exist. > > 2006-06-28 Derek R. Price <[EMAIL PROTECTED]> > > * lib/strftime.c: Assume strftime() exists. > * m4/strftime.m4: Don't call AC_FUNC_STRFTIME. Thanks. I've applied that. The only change was to increment the serial number in strftime.m4.
Re: memcmp module
> *snip* > >> +if test x"$SKIP_FTRUNCATE_CHECK" != xyes; then >> + AC_MSG_FAILURE([Your system lacks the ftruncate function. >> + Please report this, along with the output of "uname -a", to the >> + bug-coreutils@gnu.org mailing list. To continue past this point, >> + rerun configure with SKIP_FTRUNCATE_CHECK=yes set in the environment. >> + E.g., env SKIP_FTRUNCATE_CHECK=yes ./configure]) > > Please do not teach users to set environment variables rather than pass > arguments to configure. > ./configure SKIP_FTRUNCATE_CHECK=yes > > has the same effect for your code, but happens to still work with a > later > ./config.status --recheck > > which may be issued by the user, or triggered by some makefile rules. Good point. I've adjusted it. Thanks.
Re: cycle-check.h fix imported from coreutils
Ralf Wildenhues <[EMAIL PROTECTED]> wrote: > * quoting myself: >> * modules/same-inode: New module, comprising same-inode.h. >> * modules/cycle-check: Depend on it, don't list same-inode.h. >> * modules/mkdir-p, modules/same: Likewise. > > Never mind this patch. It needs either a bunch of m4/ updates to list > same-inode.h in AC_LIBSOURCES and dependencies or something along the > way... I like the idea. Do you plan to pursue it? If you do, for starters, it'd help to include the .m4 file in the Files: section and the m4 macro name in the configure.ac: section: Files: lib/same-inode.h m4/same-inode.m4 configure.ac: gl_SAME_INODE
HAVE_UNISTD_H used in progreloc.c
Hi Bruno, I just grepped for HAVE_UNISTD_H in gnulib/ and found this single remaining use. Any reason to keep it? Index: lib/progreloc.c === RCS file: /sources/gnulib/gnulib/lib/progreloc.c,v retrieving revision 1.8 diff -u -p -r1.8 progreloc.c --- lib/progreloc.c 19 Sep 2005 17:28:14 - 1.8 +++ lib/progreloc.c 7 Jul 2006 07:15:19 - @@ -1,5 +1,5 @@ /* Provide relocatable programs. - Copyright (C) 2003-2004 Free Software Foundation, Inc. + Copyright (C) 2003-2004, 2006 Free Software Foundation, Inc. Written by Bruno Haible <[EMAIL PROTECTED]>, 2003. This program is free software; you can redistribute it and/or modify @@ -29,9 +29,7 @@ #include #include #include -#if HAVE_UNISTD_H -# include -#endif +#include #include #if defined _WIN32 || defined __WIN32__
FYI, more doubled words
I've just done this:
* lib/argp-pv.c: Remove a doubled word in a comment.
* lib/check-version.c (check_version): Likewise.
* lib/javacomp.c (compile_java_class): Likewise.
* m4/glob.m4: Likewise.
Now, you too can check from the convenience of your own home.
You can use this script to find them:
#!/usr/bin/perl -n0
# Find doubled occurrences of words (e.g. in a TeX document).
# We often write "the the" with the duplicate words on separate lines.
# Written by Jim Meyering.
# $Id: doubleword,v 1.2 2006-07-09 11:42:01 meyering Exp $
use strict;
use warnings;
my %exclude = map { $_ => 1 } qw(fi shift m4 dnl long);
if (/^(.*\b(\w+)\s+\2\b.*)/m)
{
my ($text, $word) = ($1, $2);
exists $exclude{$word}
and next;
# Avoid false-positive matches like these:
# struct s s = (struct s) { 1, 2 };],
# struct pkcs5 pkcs5[] =
# struct saved_cwd saved_cwd;
# struct uparams uparams = {
# enum quoting_style quoting_style,
# *file, enum backup_type backup_type)
# enum read_header read_header (bool raw_extended_headers)
$text =~ /(union|enum|struct)\s+$word\s+$word(\[\d*\])?\s*[,;=()]/
and next;
# Also, avoid FP multi-line matches like these:
#
# #ifdef STATIC
# STATIC
#
# # if defined RANDOM_BITS
# RANDOM_BITS (random_time_bits);
$text =~ /^\s*\#\s*if(def|\s+defined)\s+$word\s+$word\b/m
and next;
# FIXME: make this a _real_ script and allow adding exceptions
# via command line specified regexps.
$text =~ /--$word\s+$word\b/m
and next;
print "$ARGV: $text\n"
}
Index: lib/argp-pv.c
===
RCS file: /sources/gnulib/gnulib/lib/argp-pv.c,v
retrieving revision 1.3
diff -u -p -r1.3 argp-pv.c
--- lib/argp-pv.c 14 May 2005 06:03:57 - 1.3
+++ lib/argp-pv.c 9 Jul 2006 10:17:34 -
@@ -1,5 +1,5 @@
/* Default definition for ARGP_PROGRAM_VERSION.
- Copyright (C) 1996, 1997, 1999 Free Software Foundation, Inc.
+ Copyright (C) 1996, 1997, 1999, 2006 Free Software Foundation, Inc.
This file is part of the GNU C Library.
Written by Miles Bader <[EMAIL PROTECTED]>.
@@ -19,6 +19,6 @@
/* If set by the user program to a non-zero value, then a default option
--version is added (unless the ARGP_NO_HELP flag is used), which will
- print this this string followed by a newline and exit (unless the
+ print this string followed by a newline and exit (unless the
ARGP_NO_EXIT flag is used). Overridden by ARGP_PROGRAM_VERSION_HOOK. */
const char *argp_program_version;
Index: lib/check-version.c
===
RCS file: /sources/gnulib/gnulib/lib/check-version.c,v
retrieving revision 1.4
diff -u -p -r1.4 check-version.c
--- lib/check-version.c 19 Sep 2005 17:28:14 - 1.4
+++ lib/check-version.c 9 Jul 2006 10:17:34 -
@@ -1,5 +1,5 @@
/* check-version.h --- Check version string compatibility.
- Copyright (C) 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005 Free
+ Copyright (C) 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006 Free
Software Foundation, Inc.
This program is free software; you can redistribute it and/or modify
@@ -30,12 +30,11 @@
/* Get specification. */
#include "check-version.h"
-/* Check that the the version of the library (i.e., the CPP symbol
- * VERSION) is at minimum the requested one in REQ_VERSION (typically
- * found in a header file) and return the version string. Return NULL
- * if the condition is not satisfied. If a NULL is passed to this
- * function, no check is done, but the version string is simply
- * returned.
+/* Check that the version of the library (i.e., the CPP symbol VERSION)
+ * is at minimum the requested one in REQ_VERSION (typically found in
+ * a header file) and return the version string. Return NULL if the
+ * condition is not satisfied. If a NULL is passed to this function,
+ * no check is done, but the version string is simply returned.
*/
const char *
check_version (const char *req_version)
Index: lib/javacomp.c
===
RCS file: /sources/gnulib/gnulib/lib/javacomp.c,v
retrieving revision 1.4
diff -u -p -r1.4 javacomp.c
--- lib/javacomp.c 26 Apr 2006 15:55:46 - 1.4
+++ lib/javacomp.c 9 Jul 2006 10:17:34 -
@@ -1,5 +1,5 @@
/* Compile a Java program.
- Copyright (C) 2001-2003 Free Software Foundation, Inc.
+ Copyright (C) 2001-2003, 2006 Free Software Foundation, Inc.
Written by Bruno Haible <[EMAIL PROTECTED]>, 2001.
This program is free software; you can redistribute it and/or modify
@@ -97,7 +97,7 @@ compile_java_class (const char * const *
{
/* Because $JAVAC may consist of a command and options, we use the
shell. Because $JAVAC has been set by the user, we leave all
- all enviro
Re: closeout bug?
Eric Blake <[EMAIL PROTECTED]> wrote: > POSIX states that: > If, at normal process termination, a function registered by the atexit() > function is called and a portable application needs to stop further exit() > processing, it must call the _exit() function or the _Exit() function or one > of > the functions which cause abnormal process termination. > > However, in the closeout module, close_stdout() invokes error() which invokes > exit(), and I have seen a lot of uses of atexit(close_stdout) in GNU programs > that use the closeout module. > > Is this potential bug worth addressing? Or is it portable in practice to use > exit() inside an atexit() handler to change the exit status, in spite of the > warning from POSIX? It must be portable in practice. There are tests of this behavior that are run as part of coreutils' "make check" (see tests/help-version), so I doubt we'll see any problem.
Re: stdint.m4 tweak
Bruno Haible <[EMAIL PROTECTED]> wrote:
> Hi Paul,
>
> ok to apply this code simplification?
>
> diff -r -c3 --unidirectional-new-file --exclude=CVS
> gnulib-20060722/m4/stdint.m4 gnulib-20060722-modified/m4/stdint.m4
> *** gnulib-20060722/m4/stdint.m4 2006-07-11 13:54:20.0 +0200
> --- gnulib-20060722-modified/m4/stdint.m4 2006-07-23 03:00:45.0
> +0200
> ***
> *** 296,302
> extern $gltype foo;
> extern $gltype1 foo;])],
> [eval gl_cv_type_${gltype}_suffix=\$glsuf])
> ! eval test \"\$gl_cv_type_${gltype}_suffix\" != no && break
> done])
> GLTYPE=`echo $gltype | tr 'abcdefghijklmnopqrstuvwxyz '
> 'ABCDEFGHIJKLMNOPQRSTUVWXYZ_'`
> eval result=\$gl_cv_type_${gltype}_suffix
> --- 321,328
> extern $gltype foo;
> extern $gltype1 foo;])],
> [eval gl_cv_type_${gltype}_suffix=\$glsuf])
> ! eval result=\$gl_cv_type_${gltype}_suffix
> ! test "$result" != no && break
> done])
> GLTYPE=`echo $gltype | tr 'abcdefghijklmnopqrstuvwxyz '
> 'ABCDEFGHIJKLMNOPQRSTUVWXYZ_'`
> eval result=\$gl_cv_type_${gltype}_suffix
Hi Bruno,
That looks like a fine simplification.
I see that there are many other uses of "$result" in that file.
How about using a name that doesn't impinge on the configure.ac
writer's name space? E.g., s/result/gl_result/g in stdint.m4
Re: va_copy, new module 'stdarg'
Bruno Haible <[EMAIL PROTECTED]> wrote: > The stdarg.m4 macro is now tested (it's used in gettext-0.15). Any objections > to this patch? That looks fine. Thank you!
Re: shell variable namespaces
Bruno Haible <[EMAIL PROTECTED]> wrote: > Jim Meyering wrote: >> I see that there are many other uses of "$result" in that file. >> How about using a name that doesn't impinge on the configure.ac >> writer's name space? E.g., s/result/gl_result/g in stdint.m4 > > There are many more shell variables used in macros that don't have a > particular prefix. > > acl.m4 use_acl > csharpcomp.m4 error > eoverflow.m4have_eoverflow > gc.m4 libgcrypt > gettext.m4 is_woe32dll > host-os.m4 os > intdiv0.m4 value > javacomp.m4 source_version target_version goodcode failcode \ > cfversion > lib-link.m4 with_gnu_ld wl libext shlibext \ > hardcode_libdir_flag_spec hardcode_libdir_separator \ > hardcode_direct hardcode_minus_L enable_rpath \ > use_additional additional_includedir \ > additional_libdir rpathdirs ltrpathdirs \ > names_already_handled names_next_round \ > names_this_round already_handled uppername value \ > found_dir found_la found_so found_a haveit basedir \ > dir alldirs next > lib-prefix.m4 searchpath > ls-mntd-fs.m4 getfsstat_includes > nanosleep.m4nanosleep_save_libs > perl.m4 candidate_perl_names perl_specified found > po.m4 srcdirpre useit desiredlanguages lang frobbedlang \ > presentlang > ptrdiff_max.m4 result > signalblocking.m4 signals_not_posix > size_max.m4 result > stdint.m4 result Thanks for identifying those. > 1) Do you think it's worth to change them all? Did you experience a >collision between your variables in configure.ac and one in *.m4? Yes, I have experienced collisions (though none recently). That's why I brought it up. They are far more likely with short and generic names like $found, $result and $dir. Although I wouldn't worry about the long ones, it would be nice to rename them use a prefix, too. > 2) When using such namespaces, moving macros from/to gnulib needs renamings. >Unnecessary diffs. IMHO, the important thing is that there be *some* prefix. Having a mix of e.g., gl_ and gt_ prefixes shouldn't cause trouble.
Re: purpose of *-safer?
Bruno Haible <[EMAIL PROTECTED]> wrote: > Eric Blake wrote: >> POSIX requires [n]>&- and [n]<&- redirection operators to close >> the respective stream, even when n is 0, 1, or 2. POSIX allows an >> implementation to supply replacement file descriptors when exec'ing a >> setuid or setgid program. But in the normal case, implementations really >> do allow you to start life with any of the three standard streams closed. > > Thanks for explaining. I wasn't aware that sh has built-in operators > for doing this. > >> that doesn't mean GNU programs can't be robust against it. > > OK, but what is the correct behaviour? Signal an error? > > $ cp --help >&- ; echo $? > cp: write error: Bad file descriptor > 1 > > or treat it like /dev/null? > > $ cp --help >&- ; echo $? > 0 That's easy: Don't ever hide a conceptual write failure. Reporting the error is the desired behavior. It's no accident that cp (and all other coreutils) work the way they do. In your simple example, it's "obvious" what is intended, and you might argue that masking the error is better, but what if a similar command fails as part of a script that is inadvertently run without a stdout file descriptor? The user of the script should be able to expect a non-zero exit status and, if possible, a diagnostic.
Re: purpose of *-safer?
Paul Eggert <[EMAIL PROTECTED]> wrote:
>>> And wouldn't there be an easier workaround: At the beginning of main(),
>>> use fcntl() to determine whether 0,1,2 are closed, and if so, replace
>>> them with open("/dev/null") ?
>>
>> Possibly. And if we did, it would make more sense to open fd 0 as write
>> only and fd 1 as read only, to be more likely to catch attempts to use
>> these streams when the user intended them to be closed.
>
> Jim did that in coreutils/lib/stdopen.c, I think with the idea of
> migrating it into gnulib if there was demand.
Right.
> Hmm, but this code
> currently isn't being used in coreutils. I don't offhand recall why.
It's not needed, now, afaik.
> Here's what I do recall. I swept coreutils for the sort of problem
> that stdopen would cure and fixed then with stdio-safer etc. Jim
> wrote stdopen.c in response, since this would be simpler than all
> those painstaking sweeps.
>
> If I missed nothing in my sweeps (an unlikely prospect!), then
> invoking stdopen merely adds a small amount of bloat to coreutils, and
> is unnecessary. A more-important argument against stdopen is that
> weird invocations like "cat /dev/fd/2 2>&-" would do the wrong thing.
In general, I think it's slightly better from a maintenance standpoint to
use stdopen.c than to make your code use the *-safer modules. The latter
requires constant vigilance, or, better, some automation to guard against
additions of unsafe functions. On the efficiency front, it's a trade-off.
stdopen.c comes with a small but constant overhead, while using the
*_safer functions, you incur the overhead of per-function-call wrappers.
And of course, stdopen.c is not an option in a library context.
race condition gnulib-tool's mostlyclean-local rule
For some modules (e.g., sys_stat), gnulib-tool generates a mostlyclean-local rule like this: mostlyclean-local: @test -z "$(MOSTLYCLEANDIRS)" ||\ for dir in $(MOSTLYCLEANDIRS); do \ if test -d $$dir; then \ echo "rmdir $$dir"; rmdir $$dir; \ fi; \ done It works fine when Make rules are run in sequence. But I always run make in parallel, e.g., with -j3. That causes the rmdir to fail if not all MOSTLYCLEANFILES have been removed when the above rmdir runs. But if you look right afterwards, you find that it is indeed empty. FYI, this triggered a failure in coreutils "make distcheck" with some changes I'm working on. One way to fix it is by changing automake to generate a slightly different rule. Currently it emits this: mostlyclean-am: mostlyclean-compile mostlyclean-generic \ mostlyclean-local The problem would be solved if automake were to emit this instead: mostlyclean-am: mostlyclean-compile mostlyclean-generic $(MAKE) mostlyclean-local While I wait for an official automake fix, I'm using the more aggressive -- i.e., slightly risky -- rule in my Makefile.am: mostlyclean-local: @test -z "$(MOSTLYCLEANDIRS)" || rm -rf $(MOSTLYCLEANDIRS)
