Re: [bug-gnulib] New GNULIB glob module?
Paul Eggert wrote: >Derek Price <[EMAIL PROTECTED]> writes: > > > >>>This seems a bit brittle. Why not simply try to compile this program? >>> >>> #include >>> char a[_GNU_GLOB_INTERFACE_VERSION == 1 ? 1 : -1]; >>> >>> >>Because I like to avoid runtime tests if I can avoid it, since they >>cannot be used when cross-compiling. >> >> > >The code that I proposed does not have any runtime tests, since the >array is allocated statically. Tests like this are often used during >cross-compiling. > > Ah, I didn't realize the optimizer could be relied on for code like this. Fixed. Patch delayed until completion of today's email processing. Regards, Derek ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: [bug-gnulib] New GNULIB glob module?
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Paul Eggert wrote: >First, already we have something bogus: that __BEGIN_DECLS. It must >be protected by "#ifdef _LIBC", since random C environments don't have >it. Similarly for __END_DECLS. Done. >The simplest fix would be to do something like this: > >#if defined _LIBC || HAVE_SYS_CDEFS_H ># include >#endif > >with the appropriate change to glob.m4. Actually, this change doesn't appear to be necessary once I protect the __BEGIN_DECLS and __END_DECLS stuff. I've removed the #include of . >But let's step back a second. Why are we worried about building >gnulib glob.c under glibc? It will never happen, right? So perhaps >we needn't worry about this problem at all. Won't it? I thought the idea was that when you and I settled on something that looked good I would attempt to send it back to the libc folks so that the files wouldn't need to be maintained separately. So far, our work contains at the least some cleanup, a bug fix, and an efficiency improvement that should be useful in libc. >With that in mind, how about if we simply remove those #undef's? If >the problem recurs we can put them back in again. Done. Patch still delayed until I'm done with my email. Regards, Derek -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.0 (Cygwin) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFCijnMLD1OTBfyMaQRAklUAKDo25YXS7Hk5kqlYe1AzXICeBBLfwCfahXg oD3soIvk5Shqw50Mfm4dgRY= =/2W5 -END PGP SIGNATURE- ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: [bug-gnulib] New GNULIB glob module?
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Paul Eggert wrote: >Derek Price <[EMAIL PROTECTED]> writes: > >>Are you sure? You asked me to restore similar parens around bit-ands >>back at several other locations despite other work that changed the >>lines, in an earlier email. Not that I disagree now. I actually prefer >>the version without the unnecessary parens around the bit-and. I just >>think we should be consistent. > > >It's a minor point, but expressions like (a & b && c) are assumed to >be confusing, as they depend on obscure precedence rules, whereas >expressions like (a & b ? c : d) are not confusing in the same way: >there's only one way to parse them, even if you have forgotten the >precedence. Okay, parens removed. Delaying new patch until I've processed the rest of your emails. Cheers, Derek -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.0 (Cygwin) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFCii0TLD1OTBfyMaQRAgE0AKCMyzvr5P31kyXB2aDTDdUSOTp6HQCg2+dm CjcxyJKKkqFEyexvpoMajtE= =h/Cn -END PGP SIGNATURE- ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
[bug-gnulib] Re: [PATCH]: fix warning in the hash module
Yoann Vandoorselaere <[EMAIL PROTECTED]> wrote: > On Tue, 2005-05-17 at 14:21 +0200, Jim Meyering wrote: >> Yoann Vandoorselaere <[EMAIL PROTECTED]> wrote: ... > This requirement is technically wrong since you allow modification of > the input argument through the user provided callback. I am well acquainted with the problem, but please bear in mind that it is a theoretical one. There is an overriding practical reason for the current hash_insert and hash_delete signatures: What if we were to convert to the const-less signatures you prefer, and then an application wants to call hash_insert with a variable of type `char const *'? -- of course, the key-freer function is NULL in that case. Then, it's not just the relatively obscure -Wcast-qual that evokes a warning, but instead it's gcc's more commonly used -W option: warning: passing argument 2 of 'hash_insert' discards qualifiers from pointer target type And since there are applications that do precisely that, I prefer to incur a little bit of technically unclean code under the covers than to require every such caller to cast-away their perfectly reasonable `const' attributes. > Additionally, the last warning is not unavoidable, you can adapt the > union patch to fix up this problem Yes, I see what you mean. But I don't think avoiding four -Wcast-qual warnings is worth the ugliness induced by adding 40+ uses of a union. > (but I still maintain that you should > not use const at all). > > - strdup return a copy of the string. It's expected it won't return a > const. Not strdup, of course :) My point was that sometimes casting away `const' is unavoidable. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: [bug-gnulib] gcc -Wall warning for minmax.h
Paul Eggert wrote: >Derek Price <[EMAIL PROTECTED]> writes: > > > > >>+#ifdef LIMITS_H_HAS_MINMAX >>+# include >>+#elif SYS_PARAM_H_HAS_MINMAX >>+# include >>+#endif >> >> > >This doesn't work if and both define MIN. > > Hrm. Okay, I've fixed this, though I think such a system that is also broken with respect to the current "minmax.h" (where _and_ both define MIN _without protecting the definitions with #ifndef MIN_) is not likely to occur in practice since it would require a system where the order of inclusion of and mattered. >>+ AC_CHECK_HEADERS_ONCE([sys/param.h], >>+[gl_have_sys_param_h=:], [gl_have_sys_param_h=false])dnl >> >> > >You don't need to check whether sys/param.h exists. All you >need to do is to see whether the following program compiles: > >#include >int x = MIN (1, 2); > >Similarly for . You shouldn't need to use EGREP_CPP. > > Okay, I see that now. Fixed. Corrected patch attached. Regards, Derek Index: m4/minmax.m4 === RCS file: m4/minmax.m4 diff -N m4/minmax.m4 --- /dev/null 1 Jan 1970 00:00:00 - +++ m4/minmax.m417 May 2005 16:44:20 - @@ -0,0 +1,31 @@ +# minmax.m4 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_MINMAX], [gl_PREREQ_MINMAX]) + +# Prerequisites of minmax.h. +AC_DEFUN([gl_PREREQ_MINMAX], +[ + AC_COMPILE_IFELSE( +[#include +int x = MIN (1, 2);], +[gl_limits_h_has_minmax=:], [gl_limits_h_has_minmax=false]) + + if $gl_limits_h_has_minmax; then +AC_DEFINE(LIMITS_H_HAS_MINMAX, 1, + [Define to 1 if defines the MIN and MAX macros.]) + else +AC_COMPILE_IFELSE( +[#include +int x = MIN (1, 2);], + [gl_sys_param_h_has_minmax=:], [gl_sys_param_h_has_minmax=false]) + +if $gl_sys_param_h_has_minmax; then + AC_DEFINE(SYS_PARAM_H_HAS_MINMAX, 1, + [Define to 1 if defines the MIN and MAX macros.]) +fi + fi + :]) Index: lib/minmax.h === RCS file: /cvsroot/gnulib/gnulib/lib/minmax.h,v retrieving revision 1.4 diff -u -p -r1.4 minmax.h --- lib/minmax.h14 May 2005 06:03:58 - 1.4 +++ lib/minmax.h17 May 2005 16:44:20 - @@ -23,9 +23,15 @@ MIN, MAX macro redefinitions on some systems; the workaround is to #include this file as the last one among the #include list. */ -/* Before we define the following symbols we get the file - since otherwise we get redefinitions on some systems. */ -#include +/* Before we define the following symbols we try to get the + or files since otherwise we get redefinitions on some + systems. */ +#ifdef LIMITS_H_HAS_MINMAX +# include +#endif +#ifdef SYS_PARAM_H_HAS_MINMAX +# include +#endif /* Note: MIN and MAX should be used with two arguments of the same type. They might not return the minimum and maximum of their two Index: modules/minmax === RCS file: /cvsroot/gnulib/gnulib/modules/minmax,v retrieving revision 1.3 diff -u -p -r1.3 minmax --- modules/minmax 28 Sep 2004 20:15:39 - 1.3 +++ modules/minmax 17 May 2005 16:44:20 - @@ -3,10 +3,12 @@ MIN, MAX macros. Files: lib/minmax.h +m4/minmax.m4 Depends-on: configure.ac: +gl_MINMAX Makefile.am: lib_SOURCES += minmax.h ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
[bug-gnulib] Bugs in chown module
Looking at m4/chown.m4, the test for defining CHOWN_FAILS_TO_HONOR_ID_OF_NEGATIVE_ONE uses the wrong sense, since ac_cv_func_chown_works is yes iff not cross-compiling and chown(f,-1,-1) didn't change the struct stat owners. Furthermore, in lib/chown.c, when this C macro is defined, the code violates POSIX by not returning -1 on failure. One potential issue not solved by this patch is whether rpl_chown should use getuid/getgid rather than stat. Another potential issue is that when cross-compiling, AC_FUNC_CHOWN defaults to no, but gl_FUNC_CHOWN_FOLLOWS_SYMLINK defaults to yes, meaning that rpl_chown will be active, and will try to call chown. Are there any systems targetted by gnulib that don't even have a chown(2)? (This is not an issue on native compiles, since if chown doesn't exist, gl_FUNC_CHOWN_FOLLOWS_SYMLINK will be no, and rpl_chown then falls back on fchown which is properly stubbed). lib/ChangeLog: 2005-05-17 Eric Blake <[EMAIL PROTECTED]> (tiny change) * chown.c (rpl_chown): Return -1 on failure. m4/ChangeLog: 2005-05-17 Eric Blake <[EMAIL PROTECTED]> (tiny change) * chown.m4 (gl_FUNC_CHOWN): Correct sense of test for honoring IDs of -1. -- Eric Blake gnulib.patch Description: Binary data ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
[bug-gnulib] same, utimecmp should depend on minmax
Patch attached. I noticed that getcwd.c and regex.c also use MIN & MAX
without depending on minmax. Is this because they are shared with
glibc? Both use #ifdef _LIBC in places, though I noticed that getcwd.c
isn't in config/srclist. Would it be appropriate to have getcwd &/or
regex depend on minmax as well?
2005-05-15 Derek Price <[EMAIL PROTECTED]>
* lib/same.c, lib/utimecmp.c: Remove MIN & MAX definitions in favor
of #include "minmax.h".
* modules/same, modules/utimecmp: Depend on minmax.
Regards,
Derek
Index: lib/same.c
===
RCS file: /cvsroot/gnulib/gnulib/lib/same.c,v
retrieving revision 1.16
diff -u -p -r1.16 same.c
--- lib/same.c 14 May 2005 06:03:58 - 1.16
+++ lib/same.c 15 May 2005 18:42:26 -
@@ -44,12 +44,9 @@
#include "same.h"
#include "dirname.h"
#include "error.h"
+#include "minmax.h"
#include "xalloc.h"
-#ifndef MIN
-# define MIN(a, b) ((a) < (b) ? (a) : (b))
-#endif
-
#define SAME_INODE(Stat_buf_1, Stat_buf_2) \
((Stat_buf_1).st_ino == (Stat_buf_2).st_ino \
&& (Stat_buf_1).st_dev == (Stat_buf_2).st_dev)
Index: lib/utimecmp.c
===
RCS file: /cvsroot/gnulib/gnulib/lib/utimecmp.c,v
retrieving revision 1.3
diff -u -p -r1.3 utimecmp.c
--- lib/utimecmp.c 14 May 2005 06:03:58 - 1.3
+++ lib/utimecmp.c 15 May 2005 18:42:26 -
@@ -36,6 +36,7 @@
#include
#include "hash.h"
#include "intprops.h"
+#include "minmax.h"
#include "timespec.h"
#include "utimens.h"
#include "xalloc.h"
@@ -43,10 +44,6 @@
/* Verify a requirement at compile-time (unlike assert, which is runtime). */
#define verify(name, assertion) struct name { char a[(assertion) ? 1 : -1]; }
-#ifndef MAX
-# define MAX(a, b) ((a) > (b) ? (a) : (b))
-#endif
-
#ifndef SIZE_MAX
# define SIZE_MAX ((size_t) -1)
#endif
Index: modules/same
===
RCS file: /cvsroot/gnulib/gnulib/modules/same,v
retrieving revision 1.6
diff -u -p -r1.6 same
--- modules/same21 Mar 2005 22:07:25 - 1.6
+++ modules/same15 May 2005 18:42:26 -
@@ -12,6 +12,7 @@ xalloc
error
dirname
stdbool
+minmax
configure.ac:
gl_SAME
Index: modules/utimecmp
===
RCS file: /cvsroot/gnulib/gnulib/modules/utimecmp,v
retrieving revision 1.5
diff -u -p -r1.5 utimecmp
--- modules/utimecmp6 May 2005 17:22:45 - 1.5
+++ modules/utimecmp15 May 2005 18:42:26 -
@@ -13,6 +13,7 @@ timespec
utimens
xalloc
stdbool
+minmax
configure.ac:
gl_UTIMECMP
___
bug-gnulib mailing list
bug-gnulib@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gnulib
[bug-gnulib] Re: coreutils FTS inclusion
Yoann Vandoorselaere <[EMAIL PROTECTED]> wrote: >>From a first look, the fts module file is lacking lstat, realloc and > malloc dependencies (which also would need to have their license changed > from GPL to LGPL). Thanks. I'll add lstat. It doesn't seem to require the malloc module, since there is no potential for malloc (0). It would need realloc only if it might call realloc (p, s) where either P is NULL or S is zero. Did you see such a case? ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
[bug-gnulib] Re: [PATCH]: fix warning in the hash module
On Mon, 2005-05-16 at 15:20 +0200, Jim Meyering wrote:
> Yoann Vandoorselaere <[EMAIL PROTECTED]> wrote:
> > This patch fix constness warning in the GnuLib hash module.
>
> I'm all for avoiding warnings, but not when it detracts from what I
> think of as the correctness of an interface, as it would in this case.
>
> Those `const void *entry' parameters constitute a promise
> by the hash module that it will not try to write through
> those pointers. Removing `const' here might make users of
> these functions wonder whether the functions write through
> the void pointers.
Since the interface let the user provide a function for deleting data
(which thus does not use const), you might then as well workaround this
issue using:
union {
const void *val_ro;
void *val_rw;
} user_data;
And use the later in order to provide the deletion callback with a not
const pointer.
Proof of concept patch attached.
--
Yoann Vandoorselaere | Responsable R&D / CTO | PreludeIDS Technologies
Tel: +33 (0)8 70 70 21 58 Fax: +33(0)4 78 42 21 58
http://www.prelude-ids.com
Index: lib/hash.c
===
RCS file: /cvsroot/gnulib/gnulib/lib/hash.c,v
retrieving revision 1.38
diff -u -r1.38 hash.c
--- lib/hash.c 14 May 2005 06:03:58 - 1.38
+++ lib/hash.c 16 May 2005 13:41:52 -
@@ -178,7 +178,7 @@
for (bucket = table->bucket; bucket < table->bucket_limit; bucket++)
{
- if (bucket->data)
+ if (bucket->user_data.ro_data)
{
struct hash_entry const *cursor = bucket;
size_t bucket_length = 1;
@@ -206,7 +206,7 @@
for (bucket = table->bucket; bucket < table->bucket_limit; bucket++)
{
- if (bucket->data)
+ if (bucket->user_data.ro_data)
{
struct hash_entry const *cursor = bucket;
@@ -256,12 +256,12 @@
if (! (bucket < table->bucket_limit))
abort ();
- if (bucket->data == NULL)
+ if (bucket->user_data.ro_data == NULL)
return NULL;
for (cursor = bucket; cursor; cursor = cursor->next)
-if (table->comparator (entry, cursor->data))
- return cursor->data;
+if (table->comparator (entry, cursor->user_data.ro_data))
+ return cursor->user_data.rw_data;
return NULL;
}
@@ -286,8 +286,8 @@
for (bucket = table->bucket; ; bucket++)
if (! (bucket < table->bucket_limit))
abort ();
-else if (bucket->data)
- return bucket->data;
+else if (bucket->user_data.ro_data)
+ return bucket->user_data.rw_data;
}
/* Return the user data for the entry following ENTRY, where ENTRY has been
@@ -306,13 +306,13 @@
/* Find next entry in the same bucket. */
for (cursor = bucket; cursor; cursor = cursor->next)
-if (cursor->data == entry && cursor->next)
- return cursor->next->data;
+if (cursor->user_data.ro_data == entry && cursor->next)
+ return cursor->next->user_data.rw_data;
/* Find first entry in any subsequent bucket. */
while (++bucket < table->bucket_limit)
-if (bucket->data)
- return bucket->data;
+if (bucket->user_data.ro_data)
+ return bucket->user_data.rw_data;
/* None found. */
return NULL;
@@ -332,13 +332,13 @@
for (bucket = table->bucket; bucket < table->bucket_limit; bucket++)
{
- if (bucket->data)
+ if (bucket->user_data.ro_data)
{
for (cursor = bucket; cursor; cursor = cursor->next)
{
if (counter >= buffer_size)
return counter;
- buffer[counter++] = cursor->data;
+ buffer[counter++] = cursor->user_data.rw_data;
}
}
}
@@ -364,11 +364,11 @@
for (bucket = table->bucket; bucket < table->bucket_limit; bucket++)
{
- if (bucket->data)
+ if (bucket->user_data.ro_data)
{
for (cursor = bucket; cursor; cursor = cursor->next)
{
- if (!(*processor) (cursor->data, processor_data))
+ if (!(*processor) (cursor->user_data.rw_data, processor_data))
return counter;
counter++;
}
@@ -608,7 +608,7 @@
for (bucket = table->bucket; bucket < table->bucket_limit; bucket++)
{
- if (bucket->data)
+ if (bucket->user_data.ro_data)
{
struct hash_entry *cursor;
struct hash_entry *next;
@@ -617,8 +617,8 @@
for (cursor = bucket->next; cursor; cursor = next)
{
if (table->data_freer)
- (*table->data_freer) (cursor->data);
- cursor->data = NULL;
+ (*table->data_freer) (cursor->user_data.rw_data);
+ cursor->user_data.ro_data = NULL;
next = cursor->next;
/* Relinking is done one entry at a time, as it is to be expected
@@ -629,8 +629,8 @@
/* Free the bucket head. */
if (table->data_freer)
- (*table->data_freer) (bucket->data);
- bucket->data = NULL;
+ (*table->data_freer) (bucket->user_data.rw_data);
+ bucket->user_data.ro_data = NULL;
bucket->next = NULL;
}
}
@@ -656,11 +656,11 @@
{
for (bucket = table->bucket; bucket < table->bucket_limit; bucket++)
{
-
Re: [bug-gnulib] gcc -Wall warning for minmax.h
Derek Price <[EMAIL PROTECTED]> writes: > +#ifdef LIMITS_H_HAS_MINMAX > +# include > +#elif SYS_PARAM_H_HAS_MINMAX > +# include > +#endif This doesn't work if and both define MIN. > + AC_CHECK_HEADERS_ONCE([sys/param.h], > +[gl_have_sys_param_h=:], [gl_have_sys_param_h=false])dnl You don't need to check whether sys/param.h exists. All you need to do is to see whether the following program compiles: #include int x = MIN (1, 2); Similarly for . You shouldn't need to use EGREP_CPP. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
[bug-gnulib] Re: [PATCH]: fix warning in the hash module
On Tue, 2005-05-17 at 08:59 +0200, Jim Meyering wrote:
> Yoann Vandoorselaere <[EMAIL PROTECTED]> wrote:
> > On Mon, 2005-05-16 at 15:20 +0200, Jim Meyering wrote:
> >> Yoann Vandoorselaere <[EMAIL PROTECTED]> wrote:
> >> > This patch fix constness warning in the GnuLib hash module.
> >>
> >> I'm all for avoiding warnings, but not when it detracts from what I
> >> think of as the correctness of an interface, as it would in this case.
> >>
> >> Those `const void *entry' parameters constitute a promise
> >> by the hash module that it will not try to write through
> >> those pointers. Removing `const' here might make users of
> >> these functions wonder whether the functions write through
> >> the void pointers.
> >
> > Since the interface let the user provide a function for deleting data
> > (which thus does not use const), you might then as well workaround this
> > issue using:
> >
> > union {
> > const void *val_ro;
> > void *val_rw;
> > } user_data;
> >
> > And use the later in order to provide the deletion callback with a not
> > const pointer.
> > Proof of concept patch attached.
>
> I'll try to make it clearer this time. The promise is that the
> named functions and their callees will not dereference through
> the const pointers. Those `const' attributes say nothing about
> what other functions (like the user-supplied deletion callback)
> will do to the underlying data.
>
> This is similar to using a const pointer to iterate through
> non-const data because you're performing a search or doing some
> other read-only job. The added (some might say `unnecessary')
> const makes the code a little more readable and might also give
> the compiler clues to let it generate better code.
I actually wonder if you looked at the second patch, since the point is
that it make no modification whatsoever to your original function
prototype, when here you appear to think that it does. The promise you
are talking about is respected.
Now, from my point of view this last patch is a workaround and the basic
API is not correct: You should not use const, it is not technically
correct as the user might free the data from the hash interface
callback.
> Thanks for the patch, but IMHO such a change is not necessary
> and would make hash.c (and its interfaces) less readable.
The interface won't be less readable because it is not modified in this
version of the patch.
However, I still consider this version of the patch to be a hack, when
the real bug here is that you should not use const since you permit the
user to free() the data from the interface callback. The interface
should not use const at all.
> Can't you just turn off that particular warning when compiling hash.c?
I'm providing a patch since from my point of view, the culprit is the
code. Of course I could turn off the warning, but that's not how I plan
to fix it ;-)
--
Yoann Vandoorselaere <[EMAIL PROTECTED]>
___
bug-gnulib mailing list
bug-gnulib@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: [bug-gnulib] New GNULIB glob module?
Derek Price <[EMAIL PROTECTED]> writes: >>This seems a bit brittle. Why not simply try to compile this program? >> >> #include >> char a[_GNU_GLOB_INTERFACE_VERSION == 1 ? 1 : -1]; > > Because I like to avoid runtime tests if I can avoid it, since they > cannot be used when cross-compiling. The code that I proposed does not have any runtime tests, since the array is allocated statically. Tests like this are often used during cross-compiling. > it should be pretty safe to grep for "gnu_glob_version = 1", unless you > are worried about some future CPP padding with spaces before the EOL? Yes, that sort of thing. cpp can even insert newlines if it wants to. It's best to avoid cpp tricks if there is an easy substitute, which there is here. > There is one change I know I made that might have an effect. I added > the "!defined _LIBC" to the following block because I was unsure what > __REDIRECT_NTH was supposed to be doing I think that one's OK. > Fixed. It's still listed in srclist.txt as "gpl" however, since the > other glibc modules were. Why is that? That tells the import-from-glibc module to use the GPL in the CVS repository. It's fine. More messages to follow, since I need to look at the code again. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
[bug-gnulib] Re: [PATCH]: fix warning in the hash module
On Tue, 2005-05-17 at 14:21 +0200, Jim Meyering wrote:
> Yoann Vandoorselaere <[EMAIL PROTECTED]> wrote:
> >> > This patch fix constness warning in the GnuLib hash module.
>
> That patch fixes three of the four warnings produced by gcc -Wcast-qual.
> The final one is here:
>
> void *
> hash_insert (Hash_table *table, const void *entry)
> {
> ...
>
> return (void *) entry; <<<===
> }
>
> What's the gain in fixing three of them if one still remains?
This warning can be fixed the same way I fixed the other (using the
union). It was a proof of concept patch.
> That last one looks unavoidable, given my requirement that the
> second parameter retain the `const' attribute.
This requirement is technically wrong since you allow modification of
the input argument through the user provided callback.
Additionally, the last warning is not unavoidable, you can adapt the
union patch to fix up this problem (but I still maintain that you should
not use const at all).
> FYI, there are similar const-removing casts in many other modules.
> Look at all of the functions like memchr, memmem, strdup, etc.,
> that take a const string and return a non-const pointer into that
> same string.
- strdup return a copy of the string. It's expected it won't return a
const.
- memmem, memchr, strstr, and such don't modify the input argument. It
is thus normal they use the const keyword. And they should not return a
const since they don't know if the user used a const argument or not as
the input.
The point is that you modify the input argument through the delete
callback... So you should probably not be using const.
--
Yoann Vandoorselaere <[EMAIL PROTECTED]>
___
bug-gnulib mailing list
bug-gnulib@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gnulib
[bug-gnulib] Re: [PATCH]: fix warning in the hash module
Yoann Vandoorselaere <[EMAIL PROTECTED]> wrote:
> On Mon, 2005-05-16 at 15:20 +0200, Jim Meyering wrote:
>> Yoann Vandoorselaere <[EMAIL PROTECTED]> wrote:
>> > This patch fix constness warning in the GnuLib hash module.
>>
>> I'm all for avoiding warnings, but not when it detracts from what I
>> think of as the correctness of an interface, as it would in this case.
>>
>> Those `const void *entry' parameters constitute a promise
>> by the hash module that it will not try to write through
>> those pointers. Removing `const' here might make users of
>> these functions wonder whether the functions write through
>> the void pointers.
>
> Since the interface let the user provide a function for deleting data
> (which thus does not use const), you might then as well workaround this
> issue using:
>
> union {
> const void *val_ro;
> void *val_rw;
> } user_data;
>
> And use the later in order to provide the deletion callback with a not
> const pointer.
> Proof of concept patch attached.
I'll try to make it clearer this time. The promise is that the
named functions and their callees will not dereference through
the const pointers. Those `const' attributes say nothing about
what other functions (like the user-supplied deletion callback)
will do to the underlying data.
This is similar to using a const pointer to iterate through
non-const data because you're performing a search or doing some
other read-only job. The added (some might say `unnecessary')
const makes the code a little more readable and might also give
the compiler clues to let it generate better code.
Thanks for the patch, but IMHO such a change is not necessary
and would make hash.c (and its interfaces) less readable.
Can't you just turn off that particular warning when compiling hash.c?
___
bug-gnulib mailing list
bug-gnulib@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gnulib
[bug-gnulib] Re: coreutils FTS inclusion
On Mon, 2005-05-16 at 14:42 +0200, Jim Meyering wrote: [...] > Thanks for diagnosing that. > I've confirmed that those modules are not needed, > so I'll remove those lines. Saw that the FTS module was checked in. Are you planing to do something regarding the license problem ? -- Yoann Vandoorselaere | Responsable R&D / CTO | PreludeIDS Technologies Tel: +33 (0)8 70 70 21 58 Fax: +33(0)4 78 42 21 58 http://www.prelude-ids.com ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
[bug-gnulib] new unlinkdir module for gnulib
I installed this, imported from coreutils. It addresses the issue
that `unlink ("directory")' works on some hosts (e.g., Solaris,
OpenBSD) but not others (e.g., GNU/Linux, FreeBSD). Solaris 10 has a
new wrinkle: you can add/remove the privilege of unlinking a
directory.
2005-05-14 Paul Eggert <[EMAIL PROTECTED]>
New unlinkdir module.
* MODULES.html.sh (File system functions): Add unlinkdir.
* lib/unlinkdir.c, lib/unlinkdir.h, m4/unlinkdir.m4, modules/unlinkdir:
New file.
Index: MODULES.html.sh
===
RCS file: /cvsroot/gnulib/gnulib/MODULES.html.sh,v
retrieving revision 1.81
diff -p -u -r1.81 MODULES.html.sh
--- MODULES.html.sh 14 May 2005 06:03:57 - 1.81
+++ MODULES.html.sh 15 May 2005 06:07:19 -
@@ -1826,6 +1826,7 @@ func_all_modules ()
func_module same
func_module save-cwd
func_module savedir
+ func_module unlinkdir
func_module utimecmp
func_module utimens
func_module xgetcwd
--- /dev/null 2003-03-18 13:55:57 -0800
+++ lib/unlinkdir.c 2005-05-14 22:53:47 -0700
@@ -0,0 +1,70 @@
+/* unlinkdir.c - determine (and maybe change) whether we can unlink directories
+
+ Copyright (C) 2005 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, 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 St, Fifth Floor, Boston, MA 02110-1301, USA. */
+
+/* Written by Paul Eggert and Jim Meyering. */
+
+#include
+
+#include "unlinkdir.h"
+
+#if HAVE_PRIV_H
+# include
+#endif
+#if HAVE_UNISTD_H
+# include
+#endif
+
+#if ! UNLINK_CANNOT_UNLINK_DIR
+
+/* Return true if we cannot unlink directories, false if we might be
+ able to unlink directories. If possible, tell the kernel we don't
+ want to be able to unlink directories, so that we can return true. */
+
+bool
+cannot_unlink_dir (void)
+{
+ static bool initialized;
+ static bool cannot;
+
+ if (! initialized)
+{
+# if defined PRIV_EFFECTIVE && defined PRIV_SYS_LINKDIR
+ /* We might be able to unlink directories if we cannot
+determine our privileges, or if we have the
+PRIV_SYS_LINKDIR privilege and cannot delete it. */
+ priv_set_t *pset = priv_allocset ();
+ if (pset)
+ {
+ cannot =
+ (getppriv (PRIV_EFFECTIVE, pset) == 0
+&& (! priv_ismember (pset, PRIV_SYS_LINKDIR)
+|| (priv_delset (pset, PRIV_SYS_LINKDIR) == 0
+&& setppriv (PRIV_SET, PRIV_EFFECTIVE, pset) == 0)));
+ priv_freeset (pset);
+ }
+# else
+ /* In traditional Unix, only root can unlink directories. */
+ cannot = (geteuid () != 0);
+# endif
+ initialized = true;
+}
+
+ return cannot;
+}
+
+#endif
--- /dev/null 2003-03-18 13:55:57 -0800
+++ lib/unlinkdir.h 2005-05-14 21:51:47 -0700
@@ -0,0 +1,27 @@
+/* unlinkdir.h - determine (and maybe change) whether we can unlink directories
+
+ Copyright (C) 2005 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, 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 St, Fifth Floor, Boston, MA 02110-1301, USA. */
+
+/* Written by Paul Eggert and Jim Meyering. */
+
+#include
+
+#if UNLINK_CANNOT_UNLINK_DIR
+static bool cannot_unlink_dir (void) { return true; }
+#else
+bool cannot_unlink_dir (void);
+#endif
--- /dev/null 2003-03-18 13:55:57 -0800
+++ m4/unlinkdir.m4 2005-05-14 21:51:54 -0700
@@ -0,0 +1,34 @@
+#serial 2
+
+# Copyright (C) 2005 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.
+
+# Written by Paul Eggert.
+
+AC_DEFUN([gl_UNLINKDIR],
+[
+ AC_REQUIRE([AC_CANONICAL_HOST])
+ AC_CHECK_HEADERS_ONCE(priv.h unistd.h)
+
+
[bug-gnulib] Re: coreutils FTS inclusion
On Tue, 2005-05-17 at 10:47 +0200, Jim Meyering wrote: > Yoann Vandoorselaere <[EMAIL PROTECTED]> wrote: > ... > > Saw that the FTS module was checked in. Are you planing to do something > > regarding the license problem ? > > Of course :) > As soon as that's resolved, I'll check in modules/fts. Another things is that including the fts module trigger a missing dependency: ../src/.libs/libprelude.so: undefined reference to `xalloc_die' -- Yoann Vandoorselaere <[EMAIL PROTECTED]> ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: [bug-gnulib] gcc -Wall warning for minmax.h
Paul Eggert wrote: >Derek Price <[EMAIL PROTECTED]> writes: > > > >>minmax was so nice and small, but would you be willing to add an m4 file >>which called AC_CHECK_HEADERS([sys/param.h]) and the following lines to >>the beginning of minmax.h? >> >>#ifdef HAVE_SYS_PARAM_H >># include >>#endif >> >> > >Better yet, why not have "configure" check whether >defines MIN and MAX, and include only if that's true? >Similarly for . That way, minmax.h will include these other >files only on broken systems. > > > >>I'll send the complete patch if you approve. >> >> > >Could you please implement something along those lines? Thanks. > > Okay, I've attached the patch you suggested, though I am still using the AC_EGREP_CPP construct you mentioned you disliked in our glob discussion. Since this method works when cross-compiling and I've yet to be convinced it is fragile, I'm going to hold off on a rewrite until we settle the other discussion. 2005-05-15 Derek Price <[EMAIL PROTECTED]> * lib/minmax.h: Only include and when needed. * m4/minmax.m4: New file. * modules/minmax: Add minmax.m4. Cheers, Derek Index: lib/minmax.h === RCS file: /cvsroot/gnulib/gnulib/lib/minmax.h,v retrieving revision 1.4 diff -u -p -r1.4 minmax.h --- lib/minmax.h14 May 2005 06:03:58 - 1.4 +++ lib/minmax.h15 May 2005 18:22:52 - @@ -23,9 +23,14 @@ MIN, MAX macro redefinitions on some systems; the workaround is to #include this file as the last one among the #include list. */ -/* Before we define the following symbols we get the file - since otherwise we get redefinitions on some systems. */ -#include +/* Before we define the following symbols we try to get the + or files since otherwise we get redefinitions on some + systems. */ +#ifdef LIMITS_H_HAS_MINMAX +# include +#elif SYS_PARAM_H_HAS_MINMAX +# include +#endif /* Note: MIN and MAX should be used with two arguments of the same type. They might not return the minimum and maximum of their two Index: m4/minmax.m4 === RCS file: m4/minmax.m4 diff -N m4/minmax.m4 --- /dev/null 1 Jan 1970 00:00:00 - +++ m4/minmax.m415 May 2005 18:22:52 - @@ -0,0 +1,34 @@ +# minmax.m4 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_MINMAX], [gl_PREREQ_MINMAX]) + +# Prerequisites of minmax.h. +AC_DEFUN([gl_PREREQ_MINMAX], +[ dnl This really calls AC_REQUIRE, so just put it first. + AC_CHECK_HEADERS_ONCE([sys/param.h], +[gl_have_sys_param_h=:], [gl_have_sys_param_h=false])dnl + + AC_EGREP_CPP([gl_minmax_test = MIN], +[#include +gl_minmax_test = MIN (xxx, yyy)], +[gl_limits_h_has_minmax=false], [gl_limits_h_has_minmax=:]) + + if $gl_limits_h_has_minmax; then +AC_DEFINE(LIMITS_H_HAS_MINMAX, 1, + [Define to 1 if defines the MIN and MAX macros.]) + elif $gl_have_sys_param_h; then +AC_EGREP_CPP([gl_minmax_test = MIN], +[#include +gl_minmax_test = MIN (xxx, yyy)], + [gl_sys_param_h_has_minmax=false], [gl_sys_param_h_has_minmax=:]) + +if $gl_sys_param_h_has_minmax; then + AC_DEFINE(SYS_PARAM_H_HAS_MINMAX, 1, + [Define to 1 if defines the MIN and MAX macros.]) +fi + fi + :]) Index: modules/minmax === RCS file: /cvsroot/gnulib/gnulib/modules/minmax,v retrieving revision 1.3 diff -u -p -r1.3 minmax --- modules/minmax 28 Sep 2004 20:15:39 - 1.3 +++ modules/minmax 15 May 2005 18:22:52 - @@ -3,10 +3,12 @@ MIN, MAX macros. Files: lib/minmax.h +m4/minmax.m4 Depends-on: configure.ac: +gl_MINMAX Makefile.am: lib_SOURCES += minmax.h ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: [bug-gnulib] New GNULIB glob module?
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Thanks for the latest round. I'm going to be out of the office today, but I should get to it by tomorrow. Regards, Derek -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.0 (Cygwin) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFCiICvLD1OTBfyMaQRAsMbAJ96UgJcf/G+zBfOlXBJM+nBJzff/ACglQte CYnnsSKWV74qlZJbIcVNNWk= =0b5Z -END PGP SIGNATURE- ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
[bug-gnulib] Re: [PATCH]: fix warning in the hash module
Yoann Vandoorselaere <[EMAIL PROTECTED]> wrote:
>> > This patch fix constness warning in the GnuLib hash module.
That patch fixes three of the four warnings produced by gcc -Wcast-qual.
The final one is here:
void *
hash_insert (Hash_table *table, const void *entry)
{
...
return (void *) entry; <<<===
}
What's the gain in fixing three of them if one still remains?
That last one looks unavoidable, given my requirement that the
second parameter retain the `const' attribute.
FYI, there are similar const-removing casts in many other modules.
Look at all of the functions like memchr, memmem, strdup, etc.,
that take a const string and return a non-const pointer into that
same string.
___
bug-gnulib mailing list
bug-gnulib@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gnulib
[bug-gnulib] Re: coreutils FTS inclusion
Yoann Vandoorselaere <[EMAIL PROTECTED]> wrote: ... > Saw that the FTS module was checked in. Are you planing to do something > regarding the license problem ? Of course :) As soon as that's resolved, I'll check in modules/fts. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
[bug-gnulib] Re: coreutils FTS inclusion
Yoann Vandoorselaere <[EMAIL PROTECTED]> wrote: > Another things is that including the fts module trigger a missing > dependency: > ../src/.libs/libprelude.so: undefined reference to `xalloc_die' That's because the hash module uses xalloc. You must tell gnulib-tool that you want to use xalloc-die, or provide your own xalloc_die function. Adding it as an explicit dependency would cause trouble for libraries that must not call exit. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
[bug-gnulib] Re: coreutils FTS inclusion
Yoann Vandoorselaere <[EMAIL PROTECTED]> wrote: ... >> It will be LGPL, since it's based on the glibc one which is LGPL. ... > Well, there seem to be a few license problem with the dependencies: > cycle-check, dirfd, hash, unistd-safer and xalloc are marked as GPL > only, so the inclusion of FTS using --lgpl will fail. Yes, that does complicate things. > Apart from that, it sound okay, but I can't tell much from a module > file :-) The other files are not hidden :) They're part of coreutils. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
[bug-gnulib] Re: coreutils FTS inclusion
On Mon, 2005-05-16 at 10:36 +0200, Jim Meyering wrote: > Yoann Vandoorselaere <[EMAIL PROTECTED]> wrote: > ... > >> It will be LGPL, since it's based on the glibc one which is LGPL. > ... > > Well, there seem to be a few license problem with the dependencies: > > cycle-check, dirfd, hash, unistd-safer and xalloc are marked as GPL > > only, so the inclusion of FTS using --lgpl will fail. > > Yes, that does complicate things. Well, you are the maintainer of most of those :-) > > Apart from that, it sound okay, but I can't tell much from a module > > file :-) > > The other files are not hidden :) > They're part of coreutils. I will try by using these. Note that coreutils contain no fts.m4. I will hack one in the meantime. -- Yoann Vandoorselaere <[EMAIL PROTECTED]> ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
[bug-gnulib] Re: [PATCH]: fix warning in the hash module
Yoann Vandoorselaere <[EMAIL PROTECTED]> wrote: > This patch fix constness warning in the GnuLib hash module. I'm all for avoiding warnings, but not when it detracts from what I think of as the correctness of an interface, as it would in this case. Those `const void *entry' parameters constitute a promise by the hash module that it will not try to write through those pointers. Removing `const' here might make users of these functions wonder whether the functions write through the void pointers. Thanks anyway. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
[bug-gnulib] Re: coreutils FTS inclusion
On Mon, 2005-05-16 at 11:27 +0200, Jim Meyering wrote: > Yoann Vandoorselaere <[EMAIL PROTECTED]> wrote: > ... > >> The other files are not hidden :) > >> They're part of coreutils. > > > > I will try by using these. Note that coreutils contain no fts.m4. I will > > You'll find it in the CVS repository for the project on Savannah: > > savannah.gnu.org/projects/coreutils > > http://savannah.gnu.org/cgi-bin/viewcvs/coreutils/coreutils/m4/fts.m4 Thanks! >From a first look, the fts module file is lacking lstat, realloc and malloc dependencies (which also would need to have their license changed from GPL to LGPL). -- Yoann Vandoorselaere | Responsable R&D / CTO | PreludeIDS Technologies Tel: +33 (0)8 70 70 21 58 Fax: +33(0)4 78 42 21 58 http://www.prelude-ids.com ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
[bug-gnulib] [PATCH]: fix warning in the hash module
Hi,
This patch fix constness warning in the GnuLib hash module.
Regards,
--
Yoann Vandoorselaere | Responsable R&D / CTO | PreludeIDS Technologies
Tel: +33 (0)8 70 70 21 58 Fax: +33(0)4 78 42 21 58
http://www.prelude-ids.com
Index: lib/hash.c
===
RCS file: /cvsroot/gnulib/gnulib/lib/hash.c,v
retrieving revision 1.38
diff -u -r1.38 hash.c
--- lib/hash.c 14 May 2005 06:03:58 - 1.38
+++ lib/hash.c 16 May 2005 12:55:13 -
@@ -905,7 +905,7 @@
Return NULL if the storage required for insertion cannot be allocated. */
void *
-hash_insert (Hash_table *table, const void *entry)
+hash_insert (Hash_table *table, void *entry)
{
void *data;
struct hash_entry *bucket;
@@ -929,11 +929,11 @@
/* Add ENTRY in the overflow of the bucket. */
- new_entry->data = (void *) entry;
+ new_entry->data = entry;
new_entry->next = bucket->next;
bucket->next = new_entry;
table->n_entries++;
- return (void *) entry;
+ return entry;
}
/* Add ENTRY right in the bucket head. */
@@ -980,7 +980,7 @@
table, don't modify the table and return NULL. */
void *
-hash_delete (Hash_table *table, const void *entry)
+hash_delete (Hash_table *table, void *entry)
{
void *data;
struct hash_entry *bucket;
Index: lib/hash.h
===
RCS file: /cvsroot/gnulib/gnulib/lib/hash.h,v
retrieving revision 1.16
diff -u -r1.16 hash.h
--- lib/hash.h 14 May 2005 06:03:58 - 1.16
+++ lib/hash.h 16 May 2005 12:55:13 -
@@ -82,7 +82,7 @@
/* Insertion and deletion. */
bool hash_rehash (Hash_table *, size_t);
-void *hash_insert (Hash_table *, const void *);
-void *hash_delete (Hash_table *, const void *);
+void *hash_insert (Hash_table *, void *);
+void *hash_delete (Hash_table *, void *);
#endif
___
bug-gnulib mailing list
bug-gnulib@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gnulib
[bug-gnulib] Re: coreutils FTS inclusion
On Mon, 2005-05-16 at 12:57 +0200, Jim Meyering wrote: > Yoann Vandoorselaere <[EMAIL PROTECTED]> wrote: > >>From a first look, the fts module file is lacking lstat, realloc and > > malloc dependencies (which also would need to have their license changed > > from GPL to LGPL). > > Thanks. I'll add lstat. > > It doesn't seem to require the malloc module, since there is no potential > for malloc (0). > > It would need realloc only if it might call realloc (p, s) where either > P is NULL or S is zero. Did you see such a case? No, it just seem that "gnulib-tool --test fts" will fail without these modules: lib/Makefile.am:17: required file `lib/realloc.c' not found lib/Makefile.am:17: required file `lib/malloc.c' not found Which is why I originally added these. -- Yoann Vandoorselaere | Responsable R&D / CTO | PreludeIDS Technologies Tel: +33 (0)8 70 70 21 58 Fax: +33(0)4 78 42 21 58 http://www.prelude-ids.com ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
[bug-gnulib] Re: coreutils FTS inclusion
On Mon, 2005-05-16 at 13:04 +0200, Yoann Vandoorselaere wrote: > On Mon, 2005-05-16 at 12:57 +0200, Jim Meyering wrote: > > Yoann Vandoorselaere <[EMAIL PROTECTED]> wrote: > > >>From a first look, the fts module file is lacking lstat, realloc and > > > malloc dependencies (which also would need to have their license changed > > > from GPL to LGPL). > > > > Thanks. I'll add lstat. > > > > It doesn't seem to require the malloc module, since there is no potential > > for malloc (0). > > > > It would need realloc only if it might call realloc (p, s) where either > > P is NULL or S is zero. Did you see such a case? > > No, it just seem that "gnulib-tool --test fts" will fail without these > modules: > lib/Makefile.am:17: required file `lib/realloc.c' not found > lib/Makefile.am:17: required file `lib/malloc.c' not found > > Which is why I originally added these. Btw, this is probably due to fts.m4: AC_REQUIRE([AC_FUNC_MALLOC]) AC_REQUIRE([AC_FUNC_REALLOC]) -- Yoann Vandoorselaere | Responsable R&D / CTO | PreludeIDS Technologies Tel: +33 (0)8 70 70 21 58 Fax: +33(0)4 78 42 21 58 http://www.prelude-ids.com ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
[bug-gnulib] Re: coreutils FTS inclusion
Yoann Vandoorselaere <[EMAIL PROTECTED]> wrote: > On Mon, 2005-05-16 at 13:04 +0200, Yoann Vandoorselaere wrote: >> On Mon, 2005-05-16 at 12:57 +0200, Jim Meyering wrote: >> > Yoann Vandoorselaere <[EMAIL PROTECTED]> wrote: >> > >>From a first look, the fts module file is lacking lstat, realloc and >> > > malloc dependencies (which also would need to have their license changed >> > > from GPL to LGPL). >> > >> > Thanks. I'll add lstat. >> > >> > It doesn't seem to require the malloc module, since there is no potential >> > for malloc (0). >> > >> > It would need realloc only if it might call realloc (p, s) where either >> > P is NULL or S is zero. Did you see such a case? >> >> No, it just seem that "gnulib-tool --test fts" will fail without these >> modules: >> lib/Makefile.am:17: required file `lib/realloc.c' not found >> lib/Makefile.am:17: required file `lib/malloc.c' not found >> >> Which is why I originally added these. > > Btw, this is probably due to fts.m4: > > AC_REQUIRE([AC_FUNC_MALLOC]) > AC_REQUIRE([AC_FUNC_REALLOC]) Thanks for diagnosing that. I've confirmed that those modules are not needed, so I'll remove those lines. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: [bug-gnulib] New GNULIB glob module?
Paul Eggert wrote: >Derek Price <[EMAIL PROTECTED]> writes: > > > >> /* Enable GNU extensions in glob.h. */ >>-#ifndef _GNU_SOURCE >>+#if defined _LIBC && !defined _GNU_SOURCE >> # define _GNU_SOURCE 1 >> #endif >> >> > >I just checked the glibc source file include/libc-symbols.h, and it >defines both _LIBC and _GNU_SOURCE. So this stuff isn't needed at >all; please remove these lines from glob.c instead. > > Done. >>+ HAVE_STRUCT_DIRENT_D_TYPE is defined by GNULIB's glob.m4 when the same >>+ member is found. */ >> >> > >A bit wordy; please rephrase to "HAVE_STRUCT_DIRENT_D_TYPE plays the >same rule in gnulib." > > Rephrased. >>- qsort ((__ptr_t) &pglob->gl_pathv[oldcount], >>+ qsort ((void *) &pglob->gl_pathv[oldcount], >> >> > >You can remove the cast entirely, right? > > So you agreed in an earlier message. I've now removed all typecasts to and from void * which are declared unnecessary by C89. >>- free ((__ptr_t) pglob->gl_pathv[pglob->gl_offs + i]); >>- free ((__ptr_t) pglob->gl_pathv); >>+ free ((void *) pglob->gl_pathv[pglob->gl_offs + i]); >>+ free ((void *) pglob->gl_pathv); >> >> > >Similarly, these casts aren't needed. > >- free ((__ptr_t) array[--i]); >+ free ((void *) array[--i]); > >Nor these. > > > >>- free ((__ptr_t) array[--i]); >>+ free ((void *) array[--i]); >> >> > >Nor this one. > > > >>+ p = mempcpy ((void *) new->name, name, len); >> >> > >Nor this one. > > > >>- free ((__ptr_t) names->name); >>+ free ((void *) names->name); >> >> > >Nor this. > > Removed, removed, removed, removed, and removed, per the above. >>- return (((flags & GLOB_ALTDIRFUNC) >>-? (*pglob->gl_stat) (fullname, &st) >>-: __stat64 (fullname, &st64)) == 0); >>+ return ((flags & GLOB_ALTDIRFUNC) >>+ ? (*pglob->gl_stat) (fullname, &st) == 0 && S_ISDIR (st.st_mode) >>+ : __stat64 (fullname, &st64) == 0 && S_ISDIR (st64.st_mode)); >> >> > >As long as you're reparenthesizing you might as well get rid of >the unnecessary ones around (flags & GLOB_ALTDIRFUNC). > > Are you sure? You asked me to restore similar parens around bit-ands back at several other locations despite other work that changed the lines, in an earlier email. Not that I disagree now. I actually prefer the version without the unnecessary parens around the bit-and. I just think we should be consistent. >>+ new->name = malloc (len + 1 >>+ + ((flags & GLOB_MARK) && isdir)); >> >> > >This line is 80 columns; should probably shrink it to 78, e.g., via. > > new->name = >malloc (...); > > Done. >glob.c is looking pretty good now. I gotta run now, but I'll look at >glob.h later today I hope. > > Thanks. I'm not attaching a patch - I'll include a complete one shortly, after I process your next two emails. Cheers, Derek ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
[bug-gnulib] Re: coreutils FTS inclusion
Yoann Vandoorselaere <[EMAIL PROTECTED]> wrote: ... >> The other files are not hidden :) >> They're part of coreutils. > > I will try by using these. Note that coreutils contain no fts.m4. I will You'll find it in the CVS repository for the project on Savannah: savannah.gnu.org/projects/coreutils http://savannah.gnu.org/cgi-bin/viewcvs/coreutils/coreutils/m4/fts.m4 ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: [bug-gnulib] New GNULIB glob module?
Derek Price <[EMAIL PROTECTED]> writes: >>Why do we need to include here? All we need is size_t, >>right? And stddef.h gives us that. > > If I don't, I get the following error: > > In file included from glob.c:23: > glob.h:107: error: syntax error before "struct" > In file included from /usr/include/errno.h:36, > from glob.c:25: > /usr/include/bits/errno.h:38: error: syntax error before "extern" Sorry, I can't debug that, since I don't have the exact version you do. In the latest version you sent, we have code that looks like this: #ifdef _LIBC # include #else # include # include # undef __size_t # define __size_t size_t #endif __BEGIN_DECLS First, already we have something bogus: that __BEGIN_DECLS. It must be protected by "#ifdef _LIBC", since random C environments don't have it. Similarly for __END_DECLS. Second, that ought to be unnecessary. We need only size_t, and provides that. If we need for some other reason on your platform, let's figure out why that is, and attack its root source rather than the symptoms. (The rest of this message does that, I think.) > Hrm. Tracing this a little farther, it is only , or even > the , OK, here's what's happening, I think. Every glibc header includes (and thus ) first thing, and this defines a whole bunch of stuff. If you attempt to declare libc-related stuff without including first, bad things happen. The simplest fix would be to do something like this: #if defined _LIBC || HAVE_SYS_CDEFS_H # include #endif with the appropriate change to glob.m4. But let's step back a second. Why are we worried about building gnulib glob.c under glibc? It will never happen, right? So perhaps we needn't worry about this problem at all. > I copied and pasted this comment and the #undef block > from the glob.c file. It was previously placed just prior to the > #include of . This looked the better place for it since I > assumed that we wouldn't want applications using the GNULIB glob module > to issue lots of redefinition warnings. OK, but in general this problem cannot be fixed in glob_.h. If the application includes our glob.h and then the buggy system files, it will still lose big-time. I tracked down these #undefs to this patch: http://sourceware.org/cgi-bin/cvsweb.cgi/libc/posix/Attic/glob.c.diff?r1=1.20&r2=1.21&cvsroot=glibc which has this ChangeLog entry in glibc: Thu Jan 21 20:12:25 1993 Roland McGrath ([EMAIL PROTECTED]) * posix/glob.c: Put #includes of and after all system includes, in case one of them has conflicting defns of FNM_* or GLOB_*, so we will redefine. #undef FNM_* and GLOB_* before including our headers. I have heard of this problem occurring with -- indeed, documents that it occurs with HP-UX A.08.07 -- but it's not clear to me that the problem every actually occurred with . Perhaps Roland made the change for simply as a precaution, because it had happened with . Also, the problem, if it occurred with glob.h, was a long time ago, and perhaps we need not worry about it any more. HP-UX 8.07 was released in 1992, and HP hasn't supported it for many years. I doubt whether people are building new GNU software for it any more. With that in mind, how about if we simply remove those #undef's? If the problem recurs we can put them back in again. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
[bug-gnulib] Re: coreutils FTS inclusion
Yoann Vandoorselaere <[EMAIL PROTECTED]> wrote: > Hi Jim, > > I just wanted to know if it is finally planed that the coreutils FTS > module will be included in GnuLIB. Henry originally did his FTS module > for the Prelude project, and we needed this module to be LGPL. > > So my question is whether we can expect the coreutils module to be LGPL > and whether you have any idea of when this module will be included (our > next libprelude release is conditioned by the inclusion of the FTS > module into GnuLIB). Hi Yoann, It will be LGPL, since it's based on the glibc one which is LGPL. Yes, I'll soon add coreutils' fts code to gnulib, maybe this evening. I don't expect the .c .h or .m4 files to change. Here's a draft modules/fts file. Please let know if you find anything wrong with it. Jim - Description: Traverse a file hierarchy. Files: lib/fts_.h lib/fts.c m4/fts.m4 lib/intprops.h m4/inttypes-pri.m4 Depends-on: cycle-check dirfd hash stdbool unistd-safer configure.ac: gl_FUNC_FTS Makefile.am: Include: "fts_.h" # License inherited from the glibc versions of the .c and .h files. License: LGPL # I don't list `glibc' here, since there are so many differences now (2005). Maintainer: Jim Meyering, Paul Eggert ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: [bug-gnulib] New GNULIB glob module?
Derek Price <[EMAIL PROTECTED]> writes: > Are you sure? You asked me to restore similar parens around bit-ands > back at several other locations despite other work that changed the > lines, in an earlier email. Not that I disagree now. I actually prefer > the version without the unnecessary parens around the bit-and. I just > think we should be consistent. It's a minor point, but expressions like (a & b && c) are assumed to be confusing, as they depend on obscure precedence rules, whereas expressions like (a & b ? c : d) are not confusing in the same way: there's only one way to parse them, even if you have forgotten the precedence. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
