stat-time systemology update?
Hello, the top of m4/stat-time.m4 says: # st_atim.tv_nsec - Linux, Solaris, Cygwin # st_atimespec.tv_nsec - FreeBSD, NetBSD, if ! defined _POSIX_SOURCE # st_atimensec - FreeBSD, NetBSD, if defined _POSIX_SOURCE # st_atim.st__tim.tv_nsec - UnixWare (at least 2.1.2 through 7.1) But my GNU libc.info mentiones st_atimensec, but not st_atim. So it seems that platforms using GNU libc (GNU/Linux, Cygwin?) are converging towards POSIX. Yes, this is a minor problem, but if this mail makes one of the portability wizards to do a rough adjustment of the comment, it might helt to the others. Thanks, Stepan
Re: stat-time systemology update?
Hello again, I'm sorry that I was too quick writing my previous mail. On Wed, Oct 17, 2007 at 11:34:54AM +0200, Stepan Kasal wrote: > But my GNU libc.info mentiones st_atimensec, but not st_atim. > > So it seems that platforms using GNU libc (GNU/Linux, Cygwin?) are > converging towards POSIX. I was wrong here. st_atim seems to be the prefferred one, in current GNU libc and POSIX draft. So the only problem is that libc.info is obsolete, I'm going to report that. Sorry, Stepan
Re: stat-time systemology update?
Stepan Kasal wrote: > the top of m4/stat-time.m4 says: > > # st_atim.tv_nsec - Linux, Solaris, Cygwin > # st_atimespec.tv_nsec - FreeBSD, NetBSD, if ! defined _POSIX_SOURCE > # st_atimensec - FreeBSD, NetBSD, if defined _POSIX_SOURCE > # st_atim.st__tim.tv_nsec - UnixWare (at least 2.1.2 through 7.1) > > But my GNU libc.info mentiones st_atimensec, but not st_atim. Please look at the actual header files, not only at the documentation. Among the bits/stat.h files in glibc, - those for Linux have this code: #ifdef __USE_MISC /* Nanosecond resolution timestamps are stored in a format equivalent to 'struct timespec'. This is the type used whenever possible but the Unix namespace rules do not allow the identifier 'timespec' to appear in the header. Therefore we have to handle the use of this header in strictly standard-compliant sources special. */ struct timespec st_atim;/* Time of last access. */ struct timespec st_mtim;/* Time of last modification. */ struct timespec st_ctim;/* Time of last status change. */ # define st_atime st_atim.tv_sec/* Backward compatibility. */ # define st_mtime st_mtim.tv_sec # define st_ctime st_ctim.tv_sec #else __time_t st_atime; /* Time of last access. */ unsigned long int st_atimensec; /* Nscecs of last access. */ __time_t st_mtime; /* Time of last modification. */ unsigned long int st_mtimensec; /* Nsecs of last modification. */ __time_t st_ctime; /* Time of last status change. */ unsigned long int st_ctimensec; /* Nsecs of last status change. */ #endif And __USE_MISC is normally enabled by _GNU_SOURCE. - those for Hurd and BSD have this code: __time_t st_atime; /* Time of last access. */ unsigned long int st_atime_usec; __time_t st_mtime; /* Time of last modification. */ unsigned long int st_mtime_usec; __time_t st_ctime; /* Time of last status change. */ unsigned long int st_ctime_usec; > So it seems that platforms using GNU libc (GNU/Linux, Cygwin?) are > converging towards POSIX. I don't see much convergence here. > Yes, this is a minor problem, but if this mail makes one of the > portability wizards to do a rough adjustment of the comment, it might > helt to the others. Not only the comment. Also, glibc's st_atime_usec is not handled. But I don't know whether it's actually filled. Bruno
More attributes on xmalloc and friends?
If I'm not mistaken, the xalloc family of functions should have the same set of function attributes as the corresponding functions in glibc. glibc gives malloc, calloc, and realloc the 'malloc' and 'warn_unused_result' attributes; similarly, it gives strdup and strndup the 'malloc' and 'nonnull (1)' attributes. 'malloc' requires gcc 2.96, 'nonnull' requires gcc 3.3, and 'warn_unused_result' requires gcc 3.4. Could Gnulib do the same for its x* wrappers? Thanks, -- Colin Watson [EMAIL PROTECTED]
Re: More attributes on xmalloc and friends?
Colin Watson <[EMAIL PROTECTED]> wrote:
> If I'm not mistaken, the xalloc family of functions should have the same
> set of function attributes as the corresponding functions in glibc.
> glibc gives malloc, calloc, and realloc the 'malloc' and
> 'warn_unused_result' attributes; similarly, it gives strdup and strndup
> the 'malloc' and 'nonnull (1)' attributes. 'malloc' requires gcc 2.96,
> 'nonnull' requires gcc 3.3, and 'warn_unused_result' requires gcc 3.4.
> Could Gnulib do the same for its x* wrappers?
Yes, indeed.
I started doing this months ago, but never checked it in.
Here's a preliminary, and probably incomplete, patch:
lib/xalloc.h: Use __warn_unused_result__ and __malloc__ attributes.
---
lib/xalloc.h | 10 +-
1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/lib/xalloc.h b/lib/xalloc.h
index 9c40d72..441410c 100644
--- a/lib/xalloc.h
+++ b/lib/xalloc.h
@@ -107,7 +107,9 @@ char *xstrdup (char const *str);
/* Allocate an array of N objects, each with S bytes of memory,
dynamically, with error checking. S must be nonzero. */
-
+static_inline void *
+xnmalloc (size_t n, size_t s)
+ __attribute__((__warn_unused_result__)) __attribute__((__malloc__));
static_inline void *
xnmalloc (size_t n, size_t s)
{
@@ -121,6 +123,9 @@ xnmalloc (size_t n, size_t s)
static_inline void *
xnrealloc (void *p, size_t n, size_t s)
+ __attribute__((__warn_unused_result__));
+static_inline void *
+xnrealloc (void *p, size_t n, size_t s)
{
if (xalloc_oversized (n, s))
xalloc_die ();
@@ -184,6 +189,9 @@ xnrealloc (void *p, size_t n, size_t s)
static_inline void *
x2nrealloc (void *p, size_t *pn, size_t s)
+ __attribute__((__warn_unused_result__));
+static_inline void *
+x2nrealloc (void *p, size_t *pn, size_t s)
{
size_t n = *pn;
--
1.5.3.4.206.g58ba4
don't dereference NULL
In http://git.sv.gnu.org/gitweb/?p=gnulib.git;a=commitdiff;h=92a8f360080d91e9e,
there are three additions like this:
diff --git a/m4/locale-fr.m4 b/m4/locale-fr.m4
...
+#ifdef __CYGWIN__
...
+ if (strchr (getenv ("LC_ALL"), '.') == NULL) return 1;
That will segfault (dereference NULL) when getenv returns NULL.
Surely that was unintentional...
lib/string.in.h needs
I received the following error after importing gnulib into scponly: cc -DHAVE_CONFIG_H -I. -DDEBUGFILE=\"/tmp/scponly46/etc/debuglevel\" -DPROG_CD=\"cd\" -Ilib -z +O2 +Ofltacc +Olit=all +Oentrysched +Odataprefetch +Onolimit -c scponly.c cc: "lib/string.h", line 298: error 1000: Unexpected symbol: "__stringp". cc: "lib/string.h", line 298: error 1000: Unexpected symbol: "__delim". cc: "lib/string.h", line 298: error 1573: Type of "__stringp" is undefined due to an illegal declaration. cc: "scponly.c", line 494: error 1000: Unexpected symbol: "strbeg". cc: "scponly.c", line 496: error 1533: Illegal function call. cc: "scponly.c", line 494: warning 527: Integral value implicitly converted to pointer in assignment. A similar problem was discovered earlier this year by Jim Meyering: http://lists.gnu.org/archive/html/bug-gnulib/2007-01/msg00374.html The attached patch addresses the problem in the same way. 2007-10-17 Albert Chin-A-Young <[EMAIL PROTECTED]> Jim Meyering <[EMAIL PROTECTED]> * lib/string.in.h: Include before . This fixes a compilation error on HP-UX, due to the system's "restrict"-using strsep prototype. -- albert chin ([EMAIL PROTECTED]) diff --git a/lib/string.in.h b/lib/string.in.h index ba7ca53..32ff5de 100644 --- a/lib/string.in.h +++ b/lib/string.in.h @@ -18,6 +18,8 @@ #ifndef _GL_STRING_H +#include + /* The include_next requires a split double-inclusion guard. */ [EMAIL PROTECTED]@ @NEXT_STRING_H@
Re: lib/string.in.h needs
Hello Albert, * Albert Chin wrote on Wed, Oct 17, 2007 at 05:58:15PM CEST: > > * lib/string.in.h: Include before . > This fixes a compilation error on HP-UX, due to the system's > "restrict"-using strsep prototype. config.h should be included from each .c file as first header. Cheers, Ralf
Re: gplv3 files and updates
On Tue, Oct 16, 2007 at 09:44:37AM -0700, Paul Eggert wrote: > > The confusion comes up because the headers in the individual files > > say something else: they say it's GPLed, period. Given this > > discrepancy, which source of information should users believe? > > They should believe the license information on the files that they > have. If it says GPL, then it's GPL. If it says it's LGPL, then it's > LGPL. The retrieval method for getting files from gnulib generates > files that use either wording, so it's up to the guy who retrieves the > files to get it right. I think I confused this thread by bringing up two issues that were only tangentially related at the beginning. I apologize for that; I didn't realize they were quite so tangential at the time. I agree with you 100% about how users should treat the licensing of gnulib files they get through consumer projects like coreutils: they should rely on the information in the header, and it'd be a lot of trouble to try to advertise the alternative licensing options to them. I have no problem with maintainers of projects that use gnulib sticking with the standard GPL/LGPL notices on the files as appropriate. And I know you can't be responsible for the various ways that different projects use gnulib, so I'm not asking you to police them and make sure they do it the right way, or somehow enforce rules on them. The only thing I'm concerned about at this point is the licensing information within gnulib itself. Right now, if I grab gnulib from git, the header on the argp source will tell me that the code is GPLed, but the documentation in the modules directory will tell me it's LGPLed. I think it would be very helpful for both of those sources of information to say the same thing: that the code is LGPLed. I'm primarily concerned with consistency, so that when people look at the gnulib source (as it exists in its own git repository, not consumer projects), it's unambiguously clear to developers that they can use the code under the LGPL. Helping them find out that the code exists in the first place is a separate problem that can be addressed better through channels other than license headers, and is at most a secondary concern of mine. But, like I said in my last e-mail, I want to see this information made consistent without making your maintainership jobs harder, which is why I've been asking so many pesky questions about how you all use gnulib in your own projects these days. >From what I understand, there's a historical reason why the file headers say the code is GPLed, even though the documentation says it's LGPLed. That's because most projects using gnulib were GPLed, and wanted to use the files under the GPL, and were typically copying them directly into their own source repositories. So having the file headers say "this is GPLed" was a straightforward way to satisfy the common use case. However, I then got the impression that gnulib-tool is now the standard way to get gnulib files into your project's source repository. If that's the case, then I think there's an easy way to make the licensing information consistent without making your jobs harder: simply put the standard LGPL license header notice on the files that are LGPLed, and give gnulib-tool a --gpl switch to convert the files' headers to a GPL license notice on request. Or if you don't want to change gnulib-tool's interface, have it assume that it should change the license headers to GPL by default, and have the existing --lgpl flag keep the LGPL license header that would already be on the files. So, as long as everyone's using gnulib-tool, I think that's the preferable way to go, because then we don't have to write any new text for the license headers. So that's why I'm asking the question: is it true that you all are using gnulib-tool? Again, I understand that there may be other developers out there who aren't, but they can fend for themselves. I'm just concerned about you all as core GNU maintainers. If I've misunderstood something, and not everyone's using gnulib-tool, then we can proceed with the work on some sort of change to the license notice on the LGPLed files. But I'd like to know if there's some reason why the above won't work first. Thanks again, -- Brett Smith Licensing Compliance Engineer, Free Software Foundation
gnulib-tool
While experimenting with a module named foo-testsuite, I noted this
glitch in gnulib-tool, and applied the obvious fix.
Cheers,
Ralf
2007-10-17 Ralf Wildenhues <[EMAIL PROTECTED]>
* gnulib-tool (func_get_dependencies): Fix sed script to
match only tests.
diff --git a/gnulib-tool b/gnulib-tool
index 1003f4c..09059fc 100755
--- a/gnulib-tool
+++ b/gnulib-tool
@@ -1212,7 +1212,7 @@ func_get_filelist ()
func_get_dependencies ()
{
# ${module}-tests always implicitly depends on ${module}.
- echo "$1" | sed -n -e 's/-tests//p'
+ echo "$1" | sed -n -e 's/-tests$//p'
# Then the explicit dependencies listed in the module description.
func_lookup_file "modules/$1"
sed -n -e "/^Depends-on$sed_extract_prog" < "$lookedup_file"
several trim bugs
Firstly, the trim module fails to add its files to lib_SOURCES, so they
don't get built.
Secondly, after fixing that, it tries to include "mbuiter.h" without
depending on that module. On inspection, I found that it claims to be
including it for MB_CUR_MAX, but that's actually in (the only
system header mbuiter.h includes over and above mbiter.h) so including
"mbuiter.h" for it seems odd and redundant. I added a check for
defined(MB_CUR_MAX) because I don't know offhand if C89
guarantees that; if it does, please remove this check again.
Thirdly, trim.c uses isspace unconditionally so it should also include
unconditionally.
Fourthly, gcc complains:
trim.c: In function ‘trim2’:
trim.c:67: warning: ‘r’ may be used uninitialized in this function
... and it may be that the state machine in fact renders it impossible
for this to be used uninitialised but this wasn't obvious to me. Some
more sanity-checking seems to be called for here.
The following patch fixes all of these problems in the obvious ways.
diff --git a/lib/trim.c b/lib/trim.c
index b917549..b7dbd70 100644
--- a/lib/trim.c
+++ b/lib/trim.c
@@ -20,13 +20,13 @@
# include
#endif
+#include /* for MB_CUR_MAX */
+#include
+
#if HAVE_MBRTOWC
# include
# include "mbchar.h"
# include "mbiter.h"
-# include "mbuiter.h" /* FIXME: for MB_CUR_MAX */
-#else
-# include
#endif
#include "xalloc.h"
@@ -42,7 +42,7 @@ trim2(const char *s, int how)
if (!d)
xalloc_die();
-#if HAVE_MBRTOWC
+#if HAVE_MBRTOWC && defined(MB_CUR_MAX)
if (MB_CUR_MAX > 1)
{
mbi_iterator_t i;
@@ -62,7 +62,7 @@ trim2(const char *s, int how)
if (how != TRIM_LEADING)
{
int state = 0;
- char *r;
+ char *r = NULL;
mbi_init (i, d, strlen (d));
@@ -101,7 +101,7 @@ trim2(const char *s, int how)
}
}
- if (state == 2)
+ if (state == 2 && r)
*r = '\0';
}
}
diff --git a/modules/trim b/modules/trim
index 27cdefc..7cd1a7c 100644
--- a/modules/trim
+++ b/modules/trim
@@ -12,6 +12,7 @@ mbiter
configure.ac:
Makefile.am:
+lib_SOURCES += trim.h trim.c
Include:
#include "trim.h"
Thanks,
--
Colin Watson [EMAIL PROTECTED]
Re: don't dereference NULL
Hi Jim,
> diff --git a/m4/locale-fr.m4 b/m4/locale-fr.m4
> ...
> +#ifdef __CYGWIN__
> ...
> + if (strchr (getenv ("LC_ALL"), '.') == NULL) return 1;
>
> That will segfault (dereference NULL) when getenv returns NULL.
> Surely that was unintentional...
It was intentional. getenv cannot return NULL here. I now added a comment
about it.
Bruno
Re: Compiler warning in glob
Eric Blake <[EMAIL PROTECTED]> writes:
> Hmm - do we need to make glob depend on the *at modules?
Good suggestion; thanks. I installed this into gnulib:
2007-10-17 Paul Eggert <[EMAIL PROTECTED]>
Modify glob.c to use fstatat and dirfd, to simplify it.
Suggested by Eric Blake.
* lib/glob.c (__fxstatat64) [!_LIBC]: New macro.
Don't include ; not used.
(link_exists2_p, glob_in_dir) [!_LIBC]: No longer a special case.
(link_exists_p): Simplify implementation, since we can now assume
dirfd and fstatat.
* modules/glob (Depends-on): Add dirfd, openat. Remove stdbool.
diff --git a/lib/glob.c b/lib/glob.c
index 97399c4..1d31d77 100644
--- a/lib/glob.c
+++ b/lib/glob.c
@@ -134,6 +134,7 @@
# define struct_stat64 struct stat64
#else /* !_LIBC */
# define __stat64(fname, buf) stat (fname, buf)
+# define __fxstatat64(_, d, f, st, flag) fstatat (d, f, st, flag)
# define struct_stat64 struct stat
# define __stat(fname, buf)stat (fname, buf)
# define __alloca alloca
@@ -142,9 +143,13 @@
# define __glob_pattern_p glob_pattern_p
#endif /* _LIBC */
-#include
#include
+#ifndef _LIBC
+# include "dirfd.h"
+# include "openat.h"
+#endif
+
#ifdef _SC_GETPW_R_SIZE_MAX
# define GETPW_R_SIZE_MAX()sysconf (_SC_GETPW_R_SIZE_MAX)
#else
@@ -1219,40 +1224,31 @@ weak_alias (__glob_pattern_p, glob_pattern_p)
static int
__attribute_noinline__
link_exists2_p (const char *dir, size_t dirlen, const char *fname,
- glob_t *pglob
-# ifndef _LIBC
- , int flags
-# endif
- )
+ glob_t *pglob)
{
size_t fnamelen = strlen (fname);
char *fullname = __alloca (dirlen + 1 + fnamelen + 1);
struct stat st;
-# ifndef _LIBC
- struct_stat64 st64;
-# endif
mempcpy (mempcpy (mempcpy (fullname, dir, dirlen), "/", 1),
fname, fnamelen + 1);
-# ifdef _LIBC
return (*pglob->gl_stat) (fullname, &st) == 0;
-# else
- return ((__builtin_expect (flags & GLOB_ALTDIRFUNC, 0)
- ? (*pglob->gl_stat) (fullname, &st)
- : __stat64 (fullname, &st64)) == 0);
-# endif
}
-# ifdef _LIBC
-# define link_exists_p(dfd, dirname, dirnamelen, fname, pglob, flags) \
- (__builtin_expect (flags & GLOB_ALTDIRFUNC, 0) \
- ? link_exists2_p (dirname, dirnamelen, fname, pglob)
\
- : ({ struct stat64 st64; \
- __fxstatat64 (_STAT_VER, dfd, fname, &st64, 0) == 0; }))
-# else
-# define link_exists_p(dfd, dirname, dirnamelen, fname, pglob, flags) \
- link_exists2_p (dirname, dirnamelen, fname, pglob, flags)
-# endif
+
+/* Return true if DIR/FNAME exists. */
+static int
+link_exists_p (int dfd, const char *dir, size_t dirlen, const char *fname,
+ glob_t *pglob, int flags)
+{
+ if (__builtin_expect (flags & GLOB_ALTDIRFUNC, 0))
+return link_exists2_p (dir, dirlen, fname, pglob);
+ else
+{
+ struct_stat64 st64;
+ return __fxstatat64 (_STAT_VER, dfd, fname, &st64, 0) == 0;
+}
+}
#endif
@@ -1328,10 +1324,8 @@ glob_in_dir (const char *pattern, const char *directory,
int flags,
}
else
{
-#ifdef _LIBC
int dfd = (__builtin_expect (flags & GLOB_ALTDIRFUNC, 0)
? -1 : dirfd ((DIR *) stream));
-#endif
int fnm_flags = ((!(flags & GLOB_PERIOD) ? FNM_PERIOD : 0)
| ((flags & GLOB_NOESCAPE) ? FNM_NOESCAPE : 0)
#if defined _AMIGA || defined VMS
diff --git a/modules/glob b/modules/glob
index aee245f..dcdf752 100644
--- a/modules/glob
+++ b/modules/glob
@@ -11,11 +11,12 @@ m4/glob.m4
Depends-on:
alloca
d-type
+dirfd
extensions
fnmatch
getlogin_r
mempcpy
-stdbool
+openat
strdup
sys_stat
unistd
