Re: declare sethostname if unistd.h doesn't

2011-11-20 Thread Bruno Haible
Hi Ben,

> When testing the pending release of inetutils on solaris 9/10, I
> discovered that sethostname isn't declared in unistd.h on Solaris.
> The following patch adds handling for this although I'm not positive
> that it's the best way to do it.  At the very least it's a starting
> point for discussing the best way to add the required support.
> 
> Based on the 'not corrected by' section in
> glibc-functions/sethostname.texi, I was tempted to make this a
> separate module but decided not to go that far yet.  If that's the
> better or more proper solution, I don't mind resubmitting.

Yes, the declaration is missing in the Solaris 10 header files, but also
on other platforms that have the function: AIX and OSF/1.

The usual approach in gnulib is to have the header file module ('unistd'
in this case) provide the types and C macros that ought to be present in
that header file, while functions declarations and function definitions
go into a module per function.

And when we provide the function module, we try to fix as many problems
as reasonably possible. This would mean, provide code for the function
on Minix 3.1.8, AIX 5.1, Cygwin, mingw, MSVC 9, Interix 3.5, BeOS.
Even if, in the worst case, the function just sets errno to ENOSYS.

> --- a/doc/glibc-functions/sethostname.texi
> +++ b/doc/glibc-functions/sethostname.texi
> @@ -6,6 +6,8 @@ Gnulib module: ---
>  
>  Portability problems fixed by Gnulib:
>  @itemize
> +Some platforms provide the function but not the prototype:
> +Solaris 9 and 10.
>  @end itemize
>  
>  Portability problems not fixed by Gnulib:

It would not make sense to say that we fix a problem in Gnulib but
don't say in which module.

Also, mentioning Solaris 9 is redundant when we mention Solaris 10.
In most cases, from the fact that a bug exists in Solaris 10 you can
also conclude that it exists in older Solaris versions. In other words,
here we mention the newest OS release that is known to have the bug.
Since Solaris 11 2011-10 _does_ have the declaration, we need to mention
Solaris 10.

While you can ponder how to define a sethostname() replacement, I'm
already adding this to the doc:


2011-11-20  Bruno Haible  

sethostname: Mention more portability problems.
* doc/glibc-functions/sethostname.texi: Mention the missing declaration
problem.
Reported by Ben Walton .

--- doc/glibc-functions/sethostname.texi.orig   Sun Nov 20 14:19:27 2011
+++ doc/glibc-functions/sethostname.texiSun Nov 20 14:19:16 2011
@@ -13,4 +13,7 @@
 @item
 This function is missing on some platforms:
 Minix 3.1.8, AIX 5.1, Cygwin, mingw, MSVC 9, Interix 3.5, BeOS.
+@item
+This function is not declared on some platforms:
+AIX 7.1, OSF/1 5.1, Solaris 10.
 @end itemize

> diff --git a/lib/unistd.in.h b/lib/unistd.in.h
> index f53f34b..057eaab 100644
> --- a/lib/unistd.in.h
> +++ b/lib/unistd.in.h
> @@ -683,6 +683,10 @@ _GL_WARN_ON_USE (getgroups, "getgroups is unportable - "
>  # endif
>  #endif
>  
> +#if HAVE_SETHOSTNAME && !HAVE_DECL_SETHOSTNAME
> +_GL_FUNCDECL_SYS (sethostname, int, (const char *name, size_t len)
> +   _GL_ARG_NONNULL ((1)));
> +#endif
>  
>  #if @GNULIB_GETHOSTNAME@
>  /* Return the standard host name of the machine.

Here, please use one of the templates from build-aux/snippet/c++defs.h.
These templates make sure that C++ programs will see the right declarations
too.

Also, please, Gnulib uses spaces for indentation in most files. Exceptions
are ChangeLog, Makefile snippets in module descriptions, and Makefiles.

> diff --git a/m4/unistd_h.m4 b/m4/unistd_h.m4
> index 57c8094..8b5cf6f 100644
> --- a/m4/unistd_h.m4
> +++ b/m4/unistd_h.m4
> @@ -21,6 +21,12 @@ AC_DEFUN([gl_UNISTD_H],
>fi
>AC_SUBST([HAVE_UNISTD_H])
>  
> +  dnl Ensure there is a prototype for sethostname
> +  AC_CHECK_DECLS([sethostname])
> +  dnl Detect the function too as we don't need the prototype if the function
> +  dnl isn't available.
> +  AC_CHECK_FUNCS([sethostname])
> +
>dnl Ensure the type pid_t gets defined.
>AC_REQUIRE([AC_TYPE_PID_T])

This would belong in a file sethostname.m4.


Now, how to implement sethostname() on platforms that don't have it?

- On Cygwin and native Windows, SetComputerNameEx [1] seems to be the
  best suited primitive.

- On AIX 5.1, sethostname() actually exists; only this can't be seen by using
  "nm /lib/libc.a" as I had done.

- On Minix 3.1.8, sethostname() should write the hostname into 
/etc/hostname.file,
  followed by a newline.

- On other systems, just use errno = ENOSYS; as fallback.

Can you implement this?

Bruno

[1] 
http://msdn.microsoft.com/en-us/library/windows/desktop/ms724931%28v=vs.85%29.aspx
-- 
In memoriam Kerem Yılmazer 



utimens test failures

2011-11-20 Thread Bruno Haible
Hi Eric, Jim,

In a testdir created through

  ./gnulib-tool --create-testdir --dir=/tmp/testdir --with-tests \
--single-configure futimens

I can see these test failures:

* AIX 5.1, HP-UX 11.00, IRIX 6.5, Solaris 7..9

  test-utimens.h:83: assertion failed
  FAIL: test-utimens

  {
struct timespec ts[2] = { { Y2K, 0 }, { Y2K, 0 } };
errno = 0;
ASSERT (func (BASE "file/", ts) == -1); /* <== HERE */
ASSERT (errno == ENOTDIR || errno == EINVAL);
  }

* FreeBSD 6.4

  test-utimens.h:127: assertion failed
  FAIL: test-utimens

ASSERT (st3.st_mtime == st2.st_mtime); /* <== HERE */

* MacOS X 10.5

  test-utimens.h:145: assertion failed
  FAIL: test-utimens

  errno = 0;
  ASSERT (func (BASE "link/", NULL) == -1); /* <== HERE */
  ASSERT (errno == ENOTDIR);

Bruno
-- 
In memoriam Kerem Yılmazer 



Re: warning options

2011-11-20 Thread Bruno Haible
I wrote:
> Thanks. I've updated my build script to include these for glibc/x86 builds
> (except -Werror, which causes a configuration error already in
> "checking whether the compiler works...").

But now I see the warnings coming out.

I won't use -Wold-style-definition and -Wstrict-prototypes because it leads
to pointless warnings in lib/:

glob.c:258:1: warning: old-style function definition [-Wold-style-definition]
glob.c:1096:1: warning: old-style function definition [-Wold-style-definition]
glob.c:1193:1: warning: old-style function definition [-Wold-style-definition]
glob.c:1232:1: warning: old-style function definition [-Wold-style-definition]
fatal-signal.c:135:1: warning: function declaration isn’t a prototype 
[-Wstrict-prototypes]
fatal-signal.c:135:1: warning: old-style function definition 
[-Wold-style-definition]
fatal-signal.c:180:1: warning: function declaration isn’t a prototype 
[-Wstrict-prototypes]
fatal-signal.c:180:1: warning: old-style function definition 
[-Wold-style-definition]
fatal-signal.c:254:1: warning: function declaration isn’t a prototype 
[-Wstrict-prototypes]
fatal-signal.c:254:1: warning: old-style function definition 
[-Wold-style-definition]
fatal-signal.c:274:1: warning: old-style function definition 
[-Wold-style-definition]
fatal-signal.c:282:1: warning: old-style function definition 
[-Wold-style-definition]

and hundreds of warnings in tests/. Most of them pointless, because an empty
function argument list (in a function definition!) is equivalent to (void).
Nothing to warn about.

I also won't use -Wunused-macros since it produces warnings that are pointless
to fix:

printf-frexp.c:63:0: warning: macro "L_" is not used
tempname.c:70:0: warning: macro "__open64" is not used
tempname.c:72:0: warning: macro "__xstat64" is not used
fnmatch.c:171:0: warning: macro "STRCOLL" is not used
fnmatch.c:199:0: warning: macro "STRCOLL" is not used
fnmatch.c:91:0: warning: macro "STREQ" is not used
getcwd.c:86:0: warning: macro "__opendir" is not used
glob.c:148:0: warning: macro "__stat" is not used
glob.c:151:0: warning: macro "__readdir64" is not used
glob.c:73:0: warning: macro "DIRENT_MUST_BE" is not used
strerror_r.c:23:0: warning: macro "_NETBSD_SOURCE" is not used
vasnprintf.c:145:0: warning: macro "DCHAR_SET" is not used
vasnprintf.c:210:0: warning: macro "remainder" is not used
vasnprintf.c:208:0: warning: macro "exp" is not used
vasnprintf.c:273:0: warning: macro "decimal_point_char_defined" is not used

-Wstrict-overflow give spurious warnings with gcc 4.5:

utimecmp.c:279:14: warning: assuming signed overflow does not occur when 
simplifying conditional to constant [-Wstrict-overflow]

Even -Wshadow fires back unreasonably:

fma.c:393:26: warning: declaration of ‘yn’ shadows a global declaration 
[-Wshadow]

And -Wtype-limits warnings in dead code are not useful either:

fma.c:172:7: warning: comparison of unsigned expression < 0 is always false 
[-Wtype-limits]
fma.c:210:7: warning: comparison of unsigned expression < 0 is always false 
[-Wtype-limits]
fma.c:248:7: warning: comparison of unsigned expression < 0 is always false 
[-Wtype-limits]


The fallout is small:


2011-11-20  Bruno Haible  

fma: Remove unused code.
* lib/fma.c (DECL_ROUNDING, BEGIN_ROUNDING, END_ROUNDING): Remove
unused macros.

--- lib/fma.c.orig  Sun Nov 20 15:51:09 2011
+++ lib/fma.c   Sun Nov 20 15:44:00 2011
@@ -41,9 +41,6 @@
 # define LDEXP ldexpl
 # define MIN_EXP LDBL_MIN_EXP
 # define MANT_BIT LDBL_MANT_BIT
-# define DECL_ROUNDING DECL_LONG_DOUBLE_ROUNDING
-# define BEGIN_ROUNDING() BEGIN_LONG_DOUBLE_ROUNDING ()
-# define END_ROUNDING() END_LONG_DOUBLE_ROUNDING ()
 # define L_(literal) literal##L
 #elif ! defined USE_FLOAT
 # define FUNC fma
@@ -52,9 +49,6 @@
 # define LDEXP ldexp
 # define MIN_EXP DBL_MIN_EXP
 # define MANT_BIT DBL_MANT_BIT
-# define DECL_ROUNDING
-# define BEGIN_ROUNDING()
-# define END_ROUNDING()
 # define L_(literal) literal
 #else /* defined USE_FLOAT */
 # define FUNC fmaf
@@ -63,9 +57,6 @@
 # define LDEXP ldexpf
 # define MIN_EXP FLT_MIN_EXP
 # define MANT_BIT FLT_MANT_BIT
-# define DECL_ROUNDING
-# define BEGIN_ROUNDING()
-# define END_ROUNDING()
 # define L_(literal) literal##f
 #endif
 

-- 
In memoriam Kerem Yılmazer 



Re: [PATCH] tests: factor st_ctime-comparison out of two headers

2011-11-20 Thread Simon Josefsson
Ben Pfaff  writes:

> Simon Josefsson  writes:
>
>> Bruno Haible  writes:
>>
>>> Jim Meyering wrote:
 -W
>> ...
 -Wwrite-strings
 -fdiagnostics-show-option
>>>
>>> Thanks. I've updated my build script to include these for glibc/x86 builds
>>> (except -Werror, which causes a configuration error already in
>>> "checking whether the compiler works...").
>>
>> I recommend putting warning flags in a separate variable,
>> e.g. WARN_CFLAGS, which is not used during ./configure checks but only
>> when building (parts of) the project code.  That way, -Werror can be
>> used, which I find helpful since I can mentally ignore the compilation
>> output since I know I will be interrupted if there is anything real to
>> pay attention to.
>
> Another approach is to add -Werror last (this is what Eric Blake
> suggested on the autoconf mailing list a long time ago), e.g.:
>
> AC_DEFUN([OVS_ENABLE_WERROR],
>   [AC_ARG_ENABLE(
>  [Werror],
>  [AC_HELP_STRING([--enable-Werror], [Add -Werror to CFLAGS])],
>  [], [enable_Werror=no])
>AC_CONFIG_COMMANDS_PRE(
>  [if test "X$enable_Werror" = Xyes; then
> CFLAGS="$CFLAGS -Werror"
>   fi])])

But does that really work?  Don't you risk affecting ./configure checks
that often are written in a way that triggers warnings, and with the
-Werror flag, leads to compilation errors.  If the only reason for the
compilation to fail was the -Werror flag, then that ./configure check
has the wrong result.  I guess it may depend on where in the ./configure
run the statement above is placed, but that seems fragile to me.

A combination of the two approaches that I've used is this:

if test "$gl_gcc_warnings" = yes; then
  gl_WARN_ADD([-Werror], [WERROR_CFLAGS])
  gl_WARN_ADD([-Wframe-larger-than=112], [WSTACK_CFLAGS])

  nw="$nw -Wsystem-headers" # Don't let system headers trigger warnings
  nw="$nw -Wpadded" # Struct in src/idn_cmd.h is not padded
  nw="$nw -Wformat" # Self tests and examples print size_t as %d
  nw="$nw -Wc++-compat" # We don't care strongly about C++ compilers
  nw="$nw -Woverlength-strings" # Some of our strings are too large
  nw="$nw -Wsign-conversion"# Too many warnings for now
  nw="$nw -Wconversion" # Too many warnings for now
  nw="$nw -Wtraditional"# Warns on #elif which we use often
  nw="$nw -Wtraditional-conversion" # Too many warnings for now
  nw="$nw -Wmissing-noreturn"   # Too many warnings for now
  nw="$nw -Wunreachable-code"   # Too many false positives
  nw="$nw -Wlogical-op" # Too many false positives

  gl_MANYWARN_ALL_GCC([ws])
  gl_MANYWARN_COMPLEMENT(ws, [$ws], [$nw])
  for w in $ws; do
gl_WARN_ADD([$w])
  done

  gl_WARN_ADD([-Wno-format])
  gl_WARN_ADD([-Wno-missing-field-initializers])
  gl_WARN_ADD([-fdiagnostics-show-option])
fi

note the different variables, WERROR_CFLAGS, WSTACK_CFLAGS and
WARN_CFLAGS.  Then I can use $(WERROR_CFLAGS) $(WARN_CFLAGS)
$(WSTACK_CFLAGS) on my own code, and typically $(WARN_CFLAGS) on gnulib
code or similar.

/Simon



Re: [PATCH] tests: factor st_ctime-comparison out of two headers

2011-11-20 Thread Peter Johansson

On 11/20/11 1:49 PM, Simon Josefsson wrote:

>  Another approach is to add -Werror last (this is what Eric Blake
>  suggested on the autoconf mailing list a long time ago), e.g.:
>
>  AC_DEFUN([OVS_ENABLE_WERROR],
> [AC_ARG_ENABLE(
>[Werror],
>[AC_HELP_STRING([--enable-Werror], [Add -Werror to CFLAGS])],
>[], [enable_Werror=no])
>  AC_CONFIG_COMMANDS_PRE(
>[if test "X$enable_Werror" = Xyes; then
>   CFLAGS="$CFLAGS -Werror"
> fi])])

But does that really work?  Don't you risk affecting ./configure checks
that often are written in a way that triggers warnings, and with the
-Werror flag, leads to compilation errors.  If the only reason for the
compilation to fail was the -Werror flag, then that ./configure check
has the wrong result.  I guess it may depend on where in the ./configure
run the statement above is placed, but that seems fragile to me.

Hi Simon,

The key is that -Werror is added to CFLAGS within 
AC_CONFIG_COMMANDS_PRE, which means it occurs in configure just before 
config.status is created, in other words, all checks should already have 
been executed.


Peter



Re: [PATCH] tests: factor st_ctime-comparison out of two headers

2011-11-20 Thread Jim Meyering
Simon Josefsson wrote:
...
> A combination of the two approaches that I've used is this:
>
> if test "$gl_gcc_warnings" = yes; then
>   gl_WARN_ADD([-Werror], [WERROR_CFLAGS])
>   gl_WARN_ADD([-Wframe-larger-than=112], [WSTACK_CFLAGS])
>
>   nw="$nw -Wsystem-headers" # Don't let system headers trigger 
> warnings
>   nw="$nw -Wpadded" # Struct in src/idn_cmd.h is not padded
>   nw="$nw -Wformat" # Self tests and examples print size_t as 
> %d
>   nw="$nw -Wc++-compat" # We don't care strongly about C++ 
> compilers
>   nw="$nw -Woverlength-strings" # Some of our strings are too large
>   nw="$nw -Wsign-conversion"# Too many warnings for now
>   nw="$nw -Wconversion" # Too many warnings for now
>   nw="$nw -Wtraditional"# Warns on #elif which we use often
>   nw="$nw -Wtraditional-conversion" # Too many warnings for now
>   nw="$nw -Wmissing-noreturn"   # Too many warnings for now
>   nw="$nw -Wunreachable-code"   # Too many false positives
>   nw="$nw -Wlogical-op" # Too many false positives
>
>   gl_MANYWARN_ALL_GCC([ws])
>   gl_MANYWARN_COMPLEMENT(ws, [$ws], [$nw])
>   for w in $ws; do
> gl_WARN_ADD([$w])
>   done
>
>   gl_WARN_ADD([-Wno-format])
>   gl_WARN_ADD([-Wno-missing-field-initializers])
>   gl_WARN_ADD([-fdiagnostics-show-option])
> fi
>
> note the different variables, WERROR_CFLAGS, WSTACK_CFLAGS and
> WARN_CFLAGS.  Then I can use $(WERROR_CFLAGS) $(WARN_CFLAGS)
> $(WSTACK_CFLAGS) on my own code, and typically $(WARN_CFLAGS) on gnulib
> code or similar.

I use the same idea (probably got it from you ;-):

I've found that handy when some condition triggers a new warning,
yet I want to link in spite of that.  In that case, it's trivial to
retain the warnings without -Werror:
Just add WERROR_CFLAGS= to your "make" invocation line.

If you simply append -Werror onto the variable that has
all of the other warning options, it's not as convenient.



Re: declare sethostname if unistd.h doesn't

2011-11-20 Thread Simon Josefsson
Ben Walton  writes:

> Hi All,
>
> When testing the pending release of inetutils on solaris 9/10, I
> discovered that sethostname isn't declared in unistd.h on Solaris.
> The following patch adds handling for this although I'm not positive
> that it's the best way to do it.  At the very least it's a starting
> point for discussing the best way to add the required support.
>
> Based on the 'not corrected by' section in
> glibc-functions/sethostname.texi, I was tempted to make this a
> separate module but decided not to go that far yet.  If that's the
> better or more proper solution, I don't mind resubmitting.
>
> Comments and criticisms are welcomed.

Have you tested that it resolves the portability issue?  If so that is
great.  My man page says Solaris uses this prototype:

 int sethostname(char *name, int namelen);

however possibly int and size_t have the same size on all Solaris 8 and
9 architectures?

In any case, I support installing this if it does something useful on at
least some (Solaris) machine, since the current situation does not
appear to work on any (Solaris) machine.

/Simon

> Thanks
> -Ben
>
> From 0dd943363bbd4d2884b8851ccbd703fee5eb6353 Mon Sep 17 00:00:00 2001
> From: Ben Walton 
> Date: Sat, 19 Nov 2011 23:13:08 +0100
> Subject: [PATCH] unistd: declare sethostname when required
>
> * m4/unistd_h.m4 (gl_UNISTD_H): Check for missing sethostname
>   declaration
> * lib/unistd.in.h: Define sethostname if the function is available but
>   the prototype isn't.
>
> Signed-off-by: Ben Walton 
> ---
>  ChangeLog|8 
>  doc/glibc-functions/sethostname.texi |2 ++
>  lib/unistd.in.h  |4 
>  m4/unistd_h.m4   |6 ++
>  4 files changed, 20 insertions(+), 0 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index 8725c92..aeaeb00 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,11 @@
> +2011-11-19  Ben Walton   
> +
> + unistd: declare sethostname when required
> + * m4/unistd_h.m4 (gl_UNISTD_H): Check for missing sethostname
> + declaration
> + * lib/unistd.in.h: Define sethostname if the function is
> + available but the prototype isn't.
> +
>  2011-11-19  Bruno Haible  
>  
>   Depend on module fcntl-h when AT_FDCWD is used.
> diff --git a/doc/glibc-functions/sethostname.texi 
> b/doc/glibc-functions/sethostname.texi
> index 64c4548..88d2ed8 100644
> --- a/doc/glibc-functions/sethostname.texi
> +++ b/doc/glibc-functions/sethostname.texi
> @@ -6,6 +6,8 @@ Gnulib module: ---
>  
>  Portability problems fixed by Gnulib:
>  @itemize
> +Some platforms provide the function but not the prototype:
> +Solaris 9 and 10.
>  @end itemize
>  
>  Portability problems not fixed by Gnulib:
> diff --git a/lib/unistd.in.h b/lib/unistd.in.h
> index f53f34b..057eaab 100644
> --- a/lib/unistd.in.h
> +++ b/lib/unistd.in.h
> @@ -683,6 +683,10 @@ _GL_WARN_ON_USE (getgroups, "getgroups is unportable - "
>  # endif
>  #endif
>  
> +#if HAVE_SETHOSTNAME && !HAVE_DECL_SETHOSTNAME
> +_GL_FUNCDECL_SYS (sethostname, int, (const char *name, size_t len)
> +   _GL_ARG_NONNULL ((1)));
> +#endif
>  
>  #if @GNULIB_GETHOSTNAME@
>  /* Return the standard host name of the machine.
> diff --git a/m4/unistd_h.m4 b/m4/unistd_h.m4
> index 57c8094..8b5cf6f 100644
> --- a/m4/unistd_h.m4
> +++ b/m4/unistd_h.m4
> @@ -21,6 +21,12 @@ AC_DEFUN([gl_UNISTD_H],
>fi
>AC_SUBST([HAVE_UNISTD_H])
>  
> +  dnl Ensure there is a prototype for sethostname
> +  AC_CHECK_DECLS([sethostname])
> +  dnl Detect the function too as we don't need the prototype if the function
> +  dnl isn't available.
> +  AC_CHECK_FUNCS([sethostname])
> +
>dnl Ensure the type pid_t gets defined.
>AC_REQUIRE([AC_TYPE_PID_T])



getcwd on AIX

2011-11-20 Thread Bruno Haible
Hi,

In a testdir for module getcwd, I get this test failure on AIX 5.1, 5.2, 5.3,
6.1, 7.1:

  FAIL: test-getcwd.sh

On this platform, configure says:

  checking whether getcwd (NULL, 0) allocates memory for result... no
  checking for getcwd with POSIX signature... yes
  checking whether getcwd is declared... yes
  checking whether getcwd is declared without a macro... yes

The test program test-getcwd fails with exit code 4.

What's happening? After creating a sufficiently large number of subdirectories,
the statement

  c = getcwd (buf, PATH_MAX);

produces c = buf, strlen (c) = 1021 (whereas PATH_MAX = 1023), and
the result in buf is

/haible/multibuild-1511/aix51-cc/testdir1/gltests/confdir3/{...}/confdir3

which is wrong: The result should be

/home/haible/multibuild-1511/aix51-cc/testdir1/gltests/confdir3/{...}/confdir3

That is, the system's getcwd function and with it also the rpl_getcwd
function has returned a file name with a missing *first* component,
and errno is still 0, giving no indication to the failure.


Either of the two following patches fixes it. Which one do you prefer?


2011-11-20  Bruno Haible  

getcwd: Work around getcwd bug on AIX 5..7.
* lib/getcwd.c (__getcwd): Don't use the system's getcwd on AIX.
* doc/posix-functions/getcwd.texi: Mention list of platforms where
getcwd does not handle long file names.

--- lib/getcwd.c.orig   Sun Nov 20 19:19:55 2011
+++ lib/getcwd.cSun Nov 20 19:03:32 2011
@@ -135,7 +135,7 @@
   size_t allocated = size;
   size_t used;
 
-#if HAVE_RAW_DECL_GETCWD
+#if HAVE_RAW_DECL_GETCWD && ! defined _AIX
   /* If AT_FDCWD is not defined, the algorithm below is O(N**2) and
  this is much slower than the system getcwd (at least on
  GNU/Linux).  So trust the system getcwd's results unless they
@@ -143,7 +143,11 @@
 
  Use the system getcwd even if we have openat support, since the
  system getcwd works even when a parent is unreadable, while the
- openat-based approach does not.  */
+ openat-based approach does not.
+
+ But on AIX 5.1..7.1, the system getcwd is not usable: If the current
+ directory name is slightly longer than PATH_MAX, it omits the first
+ directory component and returns this wrong result with errno = 0.  */
 
 # undef getcwd
   dir = getcwd (buf, size);
--- doc/posix-functions/getcwd.texi.origSun Nov 20 19:19:55 2011
+++ doc/posix-functions/getcwd.texi Sun Nov 20 19:12:51 2011
@@ -33,7 +33,8 @@
 This function is missing on some older platforms.
 @item
 This function does not handle long file names (greater than @code{PATH_MAX})
-correctly on some platforms.
+correctly on some platforms:
+glibc on Linux 2.4.20, MacOS X 10.5, FreeBSD 6.4, OpenBSD 4.9, AIX 7.1.
 @end itemize
 
 Portability problems not fixed by Gnulib:



2011-11-20  Bruno Haible  

getcwd: Work around getcwd bug on AIX 5..7.
* lib/getcwd.c (__getcwd): On AIX, verify the system's getcwd result
before returning it.
* doc/posix-functions/getcwd.texi: Mention list of platforms where
getcwd does not handle long file names.

*** lib/getcwd.c.orig   Sun Nov 20 19:40:14 2011
--- lib/getcwd.cSun Nov 20 19:40:09 2011
***
*** 147,173 
  
  # undef getcwd
dir = getcwd (buf, size);
!   if (dir || (size && errno == ERANGE))
! return dir;
! 
!   /* Solaris getcwd (NULL, 0) fails with errno == EINVAL, but it has
!  internal magic that lets it work even if an ancestor directory is
!  inaccessible, which is better in many cases.  So in this case try
!  again with a buffer that's almost always big enough.  */
!   if (errno == EINVAL && buf == NULL && size == 0)
  {
!   char big_buffer[BIG_FILE_NAME_LENGTH + 1];
!   dir = getcwd (big_buffer, sizeof big_buffer);
!   if (dir)
! return strdup (dir);
  }
  
  # if HAVE_PARTLY_WORKING_GETCWD
!   /* The system getcwd works, except it sometimes fails when it
!  shouldn't, setting errno to ERANGE, ENAMETOOLONG, or ENOENT.*/
!   if (errno != ERANGE && errno != ENAMETOOLONG && errno != ENOENT)
! return NULL;
  # endif
  #endif
  
if (size == 0)
--- 147,194 
  
  # undef getcwd
dir = getcwd (buf, size);
!   if (dir != NULL)
  {
! # ifdef _AIX
!   /* On AIX 5.1..7.1, the system getcwd can succeed and produce a
!  wrong result: If the current directory name is slightly longer
!  than PATH_MAX, it omits the first directory component and
!  returns this wrong result with errno = 0.  */
!   struct stat st2;
! 
!   if (__lstat (".", &st) >= 0
!   && __lstat (dir, &st2) >= 0
!   && st.st_dev == st2.st_dev && st.st_ino == st2.st_ino)
! return dir;
! # else
!   return dir;
! # endif
  }
+   else
+ {
+   /* dir is NULL.  Look at errno.  */
+   if (size && errno == ERANGE)
+ return NULL;
+ 
+   /* Solaris getcwd (NULL, 0) fails with errno == EINVA

Re: [PATCH] tests: factor st_ctime-comparison out of two headers

2011-11-20 Thread Ben Pfaff
Simon Josefsson  writes:

> Ben Pfaff  writes:
>
>> Simon Josefsson  writes:
>>
>>> Bruno Haible  writes:
>>>
 Jim Meyering wrote:
> -W
>>> ...
> -Wwrite-strings
> -fdiagnostics-show-option

 Thanks. I've updated my build script to include these for glibc/x86 builds
 (except -Werror, which causes a configuration error already in
 "checking whether the compiler works...").
>>>
>>> I recommend putting warning flags in a separate variable,
>>> e.g. WARN_CFLAGS, which is not used during ./configure checks but only
>>> when building (parts of) the project code.  That way, -Werror can be
>>> used, which I find helpful since I can mentally ignore the compilation
>>> output since I know I will be interrupted if there is anything real to
>>> pay attention to.
>>
>> Another approach is to add -Werror last (this is what Eric Blake
>> suggested on the autoconf mailing list a long time ago), e.g.:
>>
>> AC_DEFUN([OVS_ENABLE_WERROR],
>>   [AC_ARG_ENABLE(
>>  [Werror],
>>  [AC_HELP_STRING([--enable-Werror], [Add -Werror to CFLAGS])],
>>  [], [enable_Werror=no])
>>AC_CONFIG_COMMANDS_PRE(
>>  [if test "X$enable_Werror" = Xyes; then
>> CFLAGS="$CFLAGS -Werror"
>>   fi])])
>
> But does that really work?  Don't you risk affecting ./configure checks
> that often are written in a way that triggers warnings, and with the
> -Werror flag, leads to compilation errors.   [...]

The argument to AC_CONFIG_COMMANDS_PRE is run just before
creating config.status, regardless of when the
AC_CONFIG_COMMANDS_PRE command itself runs, so it doesn't risk
affecting any configure checks.

Here's the documentation for AC_CONFIG_COMMANDS_PRE from the
Autoconf manual.

 -- Macro: AC_CONFIG_COMMANDS_PRE (CMDS)
 Execute the CMDS right before creating `config.status'.

 This macro presents the last opportunity to call `AC_SUBST',
 `AC_DEFINE', or `AC_CONFIG_ITEMS' macros.

-- 
Ben Pfaff 
http://benpfaff.org



Re: declare sethostname if unistd.h doesn't

2011-11-20 Thread Bruno Haible
Simon Josefsson wrote:
> I support installing this if it does something useful on at
> least some (Solaris) machine, since the current situation does not
> appear to work on any (Solaris) machine.

I'm in favour of doing it using the normal Gnulib style - one module
per POSIX or glibc function. And I would like to give Ben the occasion
to implement a full replacement also for those platforms where the function
is missing.

Bruno
-- 
In memoriam Kerem Yılmazer 



Re: getcwd on AIX

2011-11-20 Thread Jim Meyering
Bruno Haible wrote:
> In a testdir for module getcwd, I get this test failure on AIX 5.1, 5.2, 5.3,
> 6.1, 7.1:
>
>   FAIL: test-getcwd.sh
>
> On this platform, configure says:
>
>   checking whether getcwd (NULL, 0) allocates memory for result... no
>   checking for getcwd with POSIX signature... yes
>   checking whether getcwd is declared... yes
>   checking whether getcwd is declared without a macro... yes
>
> The test program test-getcwd fails with exit code 4.
>
> What's happening? After creating a sufficiently large number of 
> subdirectories,
> the statement
>
>   c = getcwd (buf, PATH_MAX);
>
> produces c = buf, strlen (c) = 1021 (whereas PATH_MAX = 1023), and
> the result in buf is
>
> /haible/multibuild-1511/aix51-cc/testdir1/gltests/confdir3/{...}/confdir3
>
> which is wrong: The result should be
>
> /home/haible/multibuild-1511/aix51-cc/testdir1/gltests/confdir3/{...}/confdir3
>
> That is, the system's getcwd function and with it also the rpl_getcwd
> function has returned a file name with a missing *first* component,
> and errno is still 0, giving no indication to the failure.
>
>
> Either of the two following patches fixes it. Which one do you prefer?

Is there an advantage to using the system getcwd for names
shorter than PATH_MAX, as there is on Solaris and systems with
a linux kernel? (i.e., ability to function in spite of restricted
permissions on a parent directory)

If so, I think we must prefer the latter, even though it incurs
the additional overhead (albeit only on AIX) of two lstat calls per
component.

> 2011-11-20  Bruno Haible  
>
>   getcwd: Work around getcwd bug on AIX 5..7.
>   * lib/getcwd.c (__getcwd): Don't use the system's getcwd on AIX.
>   * doc/posix-functions/getcwd.texi: Mention list of platforms where
>   getcwd does not handle long file names.
>
...
> 2011-11-20  Bruno Haible  
>
>   getcwd: Work around getcwd bug on AIX 5..7.
>   * lib/getcwd.c (__getcwd): On AIX, verify the system's getcwd result
>   before returning it.
>   * doc/posix-functions/getcwd.texi: Mention list of platforms where
>   getcwd does not handle long file names.
>
...
> ! # ifdef _AIX
> !   /* On AIX 5.1..7.1, the system getcwd can succeed and produce a
> !  wrong result: If the current directory name is slightly longer
> !  than PATH_MAX, it omits the first directory component and
> !  returns this wrong result with errno = 0.  */
> !   struct stat st2;
> !
> !   if (__lstat (".", &st) >= 0
> !   && __lstat (dir, &st2) >= 0
> !   && st.st_dev == st2.st_dev && st.st_ino == st2.st_ino)
> ! return dir;
> ! # else
> !   return dir;
> ! # endif
...

Thanks for writing the test program (I haven't really looked at it, though).



Re: Accessing the environment's locale encoding settings

2011-11-20 Thread Bruno Haible
[CCing bug-gnulib. This is a reply to
 ].

Hi Ludovic,

> I’m now convinced that an implicit setlocale(LC_ALL, "") is the right
> thing for ‘master’.

Good, glad that I could help with my opinion :)

> For 2.0, though, this brings us back to the hack I proposed at the
> beginning of this thread, namely trying to honor LC_CTYPE without
> actually calling setlocale, so that command-line arguments are suitably
> converted.
> 
> Could Gnulib’s get_charset_aliases be exported?

As documented in [1], not every possible code modification is suitable for
gnulib proper. In this case, in particular, I don't think that code
that returns the locale encoding _if_ setlocale (LC_CTYPE, "") had been
called, without really calling it, is a frequent enough use-case.

If I were you, I would start using the gnulib-tool option --local-dir
with a local modification of the 'localcharset' module, as documented in [1].
This means:

  1) Hack your local copy of localcharset.c so that it not only defines
 the locale_charset() function, but also an additional function
 environ_locale_charset() that looks only at the environment variables.

  2) Store this file in guile:

   $ cp ludo-localcharset.c guile/gnulib-local/lib/localcharset.c

 or if there are few changes just the differences:

   $ diff -u gnulib/lib/localcharset.c ludo-localcharset.c \
 > guile/gnulib-local/lib/localcharset.c.diff

  3) Pass the option
--local-dir gnulib-local 
 to the gnulib-tool invocation in autogen.sh.

Bruno

[1] http://www.gnu.org/s/hello/manual/gnulib/Extending-Gnulib.html
-- 
In memoriam Kerem Yılmazer 



Re: declare sethostname if unistd.h doesn't

2011-11-20 Thread Paul Eggert
On 11/20/11 11:04, Simon Josefsson wrote:
> however possibly int and size_t have the same size on all Solaris 8 and
> 9 architectures?

No, on 64-bit Solaris hosts, int is 32, size_t is 64;
this has been true since full 64-bit support was added
(Solaris 7 in the late 1990s, if I recall correctly).



Re: warning options

2011-11-20 Thread Paul Eggert
On 11/20/11 06:58, Bruno Haible wrote:

> And -Wtype-limits warnings in dead code are not useful either:

I agree with you about this one -- it's been a longrunning problem
with GCC.  I use -Wno-type-limits when compiling Emacs.  I have
the problem in non-dead code as well.

> an empty
> function argument list (in a function definition!) is equivalent to (void).

It's not equivalent in C, since (void) gives us extra compile-time type
checking that an empty arglist does not.  For example, there is no
type error (and no GCC diagnostic) in this C code:

  int f () { return 0; }
  int main (void) { return f (1); }

whereas there would be a type error if the () were replaced by (void).
In C, the argument for (void) in particular is similar to the argument
for new-style definitions in general.

> I also won't use -Wunused-macros since it produces warnings that are pointless
> to fix:
> 
> printf-frexp.c:63:0: warning: macro "L_" is not used
> tempname.c:70:0: warning: macro "__open64" is not used
> tempname.c:72:0: warning: macro "__xstat64" is not used
> fnmatch.c:171:0: warning: macro "STRCOLL" is not used
> fnmatch.c:199:0: warning: macro "STRCOLL" is not used
> ...

The first warning I'll grant you (though surely it is easy to work around)
but I don't see why the next four warnings are pointless to fix
(I stopped looking after the fifth warning).
Those macro definitions are never used, so why clutter up the code
with them?

> -Wstrict-overflow give spurious warnings with gcc 4.5:

Yes, -Wstrict-overflow has several problems with older GCCs.
I've been trying to get this fixed by filing bug reports
(e.g., ).
I don't advocate rewriting the code to pacify older GCC bugs
in this area.  I think Jim works around this problem by using
GCC 4.7 beta; I use 4.6.2.

> Even -Wshadow fires back unreasonably:

This may be taste, but I don't find that warning unreasonable,
as a local (yn) is shadowing a global (yn).



Re: getcwd on AIX

2011-11-20 Thread Paul Eggert
On 11/20/11 11:27, Bruno Haible wrote:

> Either of the two following patches fixes it. Which one do you prefer?

The first patch is simpler and easier to maintain, so I'd go with it
unless there's a performance or correctness reason to go with the
second patch.

In Solaris, the system getcwd has some internal magic that
makes it better than our replacement when it works; if the
same is true for AIX that'd argue for the second patch.



manywarnings update

2011-11-20 Thread Simon Josefsson
I have pushed the patch below.  For Libtasn1, the only new warning flag
I had to manually disable for that project was -Wsuggest-attribute=pure
because there were many hits and I didn't feel like adding many function
attributes.  Unfortunately, several of the parameters aren't documented
in the manual so it is unclear whether they are implied by -Wall or not.

/Simon

>From 3e07d5d60bee7d0e6fc0d79e974ce5a8dc7dea61 Mon Sep 17 00:00:00 2001
From: Simon Josefsson 
Date: Sun, 20 Nov 2011 23:08:31 +0100
Subject: [PATCH] manywarnings: More warnings.

* m4/manywarnings.m4: Add more warnings from gcc 4.6.2.
---
 ChangeLog  |4 
 m4/manywarnings.m4 |   27 +++
 2 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index e447786..8173a2e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2011-11-20  Simon Josefsson  
+
+   * m4/manywarnings.m4: Add more warnings from gcc 4.6.2.
+
 2011-11-20  Bruno Haible  
 
fma tests: Avoid shadowing local variables.
diff --git a/m4/manywarnings.m4 b/m4/manywarnings.m4
index 67db064..6e78c07 100644
--- a/m4/manywarnings.m4
+++ b/m4/manywarnings.m4
@@ -148,6 +148,33 @@ AC_DEFUN([gl_MANYWARN_ALL_GCC],
 ; do
 gl_manywarn_set="$gl_manywarn_set $gl_manywarn_item"
   done
+  # More warnings from gcc 4.6.2 --help=warnings.
+  for gl_manywarn_item in \
+-Wabi \
+-Wcpp \
+-Wdeprecated \
+-Wdeprecated-declarations \
+-Wdiv-by-zero \
+-Wdouble-promotion \
+-Wendif-labels \
+-Wextra \
+-Wformat-contains-nul \
+-Wformat-extra-args \
+-Wformat-zero-length \
+-Wformat=2 \
+-Wmultichar \
+-Wnormalized=nfc \
+-Woverflow \
+-Wpointer-to-int-cast \
+-Wpragmas \
+-Wsuggest-attribute=const \
+-Wsuggest-attribute=noreturn \
+-Wsuggest-attribute=pure \
+-Wtrampolines \
+-Wunsuffixed-float-constants \
+; do
+gl_manywarn_set="$gl_manywarn_set $gl_manywarn_item"
+  done
 
   # Disable the missing-field-initializers warning if needed
   if test "$gl_cv_cc_nomfi_needed" = yes; then
-- 
1.7.2.5




Re: git-version-gen

2011-11-20 Thread Simon Josefsson
Eric Blake  writes:

> On 11/13/2011 03:18 AM, Simon Josefsson wrote:
>>> Yes, since that allows you the freedom to have more tags than just
>>> official releases.  (I frequently have local tags to intermediate
>>> points, but only push official v1.0 style tags upstream, but since the
>>> command runs locally, git describe tries to use my local tags unless I
>>> use --match).
>> 
>> Ok.  But what is the harm if the --match is not done?  You would get a
>> version string based on your own tags, but how does that matter?
>
> If you are using incremental version strings (such as coreutils), by
> using git-version-gen as part of configure.ac, then every build you do
> locally would pick up on your local tags instead of the latest release
> tag, if you omit the --match.  Yes, at release time, the latest tag is a
> release tag; but it is during the incremental development (such as
> posting candidate snapshots to the platform-testers list) where --match
> saves the day.

Ok thanks for explaining.

/Simon



Re: manywarnings update

2011-11-20 Thread Jim Meyering
Simon Josefsson wrote:

> I have pushed the patch below.  For Libtasn1, the only new warning flag
> I had to manually disable for that project was -Wsuggest-attribute=pure
> because there were many hits and I didn't feel like adding many function
> attributes.  Unfortunately, several of the parameters aren't documented
> in the manual so it is unclear whether they are implied by -Wall or not.
>
> /Simon
>
>>From 3e07d5d60bee7d0e6fc0d79e974ce5a8dc7dea61 Mon Sep 17 00:00:00 2001
> From: Simon Josefsson 
> Date: Sun, 20 Nov 2011 23:08:31 +0100
> Subject: [PATCH] manywarnings: More warnings.
>
> * m4/manywarnings.m4: Add more warnings from gcc 4.6.2.
> ---
>  ChangeLog  |4 
>  m4/manywarnings.m4 |   27 +++
>  2 files changed, 31 insertions(+), 0 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index e447786..8173a2e 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,7 @@
> +2011-11-20  Simon Josefsson  
> +
> + * m4/manywarnings.m4: Add more warnings from gcc 4.6.2.
> +

Thanks.
I took that for a spin in coreutils (not yet pushed, of course).
Lots of new "error"s, doubtless soon to be dealt with or ignored:

168 format not a string literal, argument types not checked 
[-Werror=format-nonliteral]
135 unsuffixed float constant [-Werror=unsuffixed-float-constants]
  4 format not a string literal, format string not checked 
[-Werror=format-nonliteral]
  1 implicit conversion from 'float' to 'double' when passing argument to 
function [-Werror=double-promotion]



getcwd: an old bug fix

2011-11-20 Thread Bruno Haible
Since 2009-09-10, there is a small bug in getcwd.m4: it treats
gl_cv_func_getcwd_null="guessing yes" like a "no" in one place.

This fixes it.


2011-11-20  Bruno Haible  

getcwd: Fix bug from 2009-09-10.
* m4/getcwd.m4 (gl_FUNC_GETCWD): Treat "guessing yes" like "yes", not
like "no".

--- m4/getcwd.m4.orig   Sun Nov 20 23:27:12 2011
+++ m4/getcwd.m4Sun Nov 20 23:25:20 2011
@@ -6,7 +6,7 @@
 # with or without modifications, as long as this notice is preserved.
 
 # Written by Paul Eggert.
-# serial 9
+# serial 10
 
 AC_DEFUN([gl_FUNC_GETCWD_NULL],
   [
@@ -108,12 +108,18 @@
   AC_REQUIRE([AC_CANONICAL_HOST]) dnl for cross-compiles
 
   gl_abort_bug=no
-  case $gl_cv_func_getcwd_null,$host_os in
-  *,mingw*)
-gl_cv_func_getcwd_path_max=yes;;
-  yes,*)
-gl_FUNC_GETCWD_PATH_MAX
-gl_FUNC_GETCWD_ABORT_BUG([gl_abort_bug=yes]);;
+  case "$host_os" in
+mingw*)
+  gl_cv_func_getcwd_path_max=yes
+  ;;
+*)
+  case "$gl_cv_func_getcwd_null" in
+*yes)
+  gl_FUNC_GETCWD_PATH_MAX
+  gl_FUNC_GETCWD_ABORT_BUG([gl_abort_bug=yes])
+  ;;
+  esac
+  ;;
   esac
 
   case 
$gl_cv_func_getcwd_null,$gl_cv_func_getcwd_posix_signature$gl_cv_func_getcwd_path_max,$gl_abort_bug
 in


-- 
In memoriam Kerem Yılmazer 



Re: [PATCH] getcwd-lgpl: fix m4 to match relaxed test for BSD

2011-11-20 Thread Bruno Haible
Eric Blake wrote on 2011-08-17:
>  gl_FUNC_GETCWD_ABORT_BUG([gl_abort_bug=yes]);;
>esac
> 
> -  case $gl_cv_func_getcwd_null,$gl_cv_func_getcwd_path_max,$gl_abort_bug in
> -  *yes,yes,no) ;;
> +  case 
> $gl_cv_func_getcwd_null,$gl_cv_func_getcwd_posix_signature$gl_cv_func_getcwd_path_max,$gl_abort_bug
>  in
> +  *yes,yes,yes,no) ;;
>*)
>  dnl Full replacement lib/getcwd.c, overrides LGPL replacement.
>  REPLACE_GETCWD=1;;

This change has the effect of setting REPLACE_GETCWD to 1 on *all*
platforms. A string with only two commas can never math the pattern
"*yes,yes,yes,no".

Such as on glibc 2.11/Linux or MacOS X or FreeBSD 6.4, OpenBSD 4.9, NetBSD 5.1:
  checking whether getcwd (NULL, 0) allocates memory for result... yes
  checking for getcwd with POSIX signature... yes
  checking whether getcwd is declared... yes
  S["REPLACE_GETCWD"]="1"

I'd like to rectify this. The fix is not just to fix the 'case' statement,
because on OpenBSD 4.9, NetBSD 5.1 that leads to REPLACE_GETCWD=0 (as intended)
but then the unit test fails with error code 32, i.e. test_abort_bug() returned 
4.

On these two platforms,
  - The test program from m4/getcwd-abort-bug.m4 fails with exit code 4,
  - but the macrology around it considers an error code 4 to be OK
(this is also what the comment says: "This is ok, and expected".
  - Then, if getcwd does not get overridden by gnulib, in tests/test-getcwd.c
the function test_abort_bug() returns 4 and the test fails.
  - Finally, the documentation says that the 'getcwd' module fixes the fact
that "This function does not handle long file names (greater than PATH_MAX)"

So, what should we do?
  - Change m4/getcwd-abort-bug.m4 to consider an exit code 4 also to be a
failure?
  - Or change tests/test-getcwd.c to consider test_abort_bug() = 4 a success
and change the documentation to state that getcwd(NULL,0) may not work
with long file names, even when module 'getcwd' is enabled?

The first one seems more right to me. Jim, Paul?

Bruno
-- 
In memoriam Kerem Yılmazer 



Re: getcwd: an old bug fix

2011-11-20 Thread Paul Eggert
Thanks!  Please apply.



Re: getcwd on AIX

2011-11-20 Thread Bruno Haible
Jim Meyering wrote:
> Is there an advantage to using the system getcwd for names
> shorter than PATH_MAX, as there is on Solaris and systems with
> a linux kernel? (i.e., ability to function in spite of restricted
> permissions on a parent directory)

Paul Eggert wrote:
> The first patch is simpler and easier to maintain, so I'd go with it
> unless there's a performance or correctness reason to go with the
> second patch.
> 
> In Solaris, the system getcwd has some internal magic that
> makes it better than our replacement when it works; if the
> same is true for AIX that'd argue for the second patch.

On both AIX 5.1 and 7.1, I can see with a "truss" run of my test program
that getcwd() does the usual search through ../, ../../, ../../../, etc.,
like gnulib/lib/getcwd.c also does:

statx("./../../../../../../../../../../../../", 0x2FF20E50, 76, 0) = 0
open("./../../../../../../../../../../../../", O_RDONLY) = 3
getdirent(3, 0x2998, 4096)  = 124
lseek(3, 0, 0)  = 0
kfcntl(3, F_GETFD, 0x2FF22FFC)  = 0
kfcntl(3, F_SETFD, 0x0001)  = 0
fstatx(3, 0x2FF20FB8, 76, 0)= 0
getdirent(3, 0x2998, 4096)  = 124
close(3)= 0

When run in a directory with an unreadable parent dir, the test program
shows that getcwd() fails, and /bin/pwd as well:
  $ /bin/pwd
  pwd: The file access permissions do not allow the specified action.

So, I'm going with the first approach.

I'm not writing 'defined _AIX' but instead using an autoconf test. My
new autoconf test is not needed; Jim's old one in m4/getcwd-path-max.m4
also recognizes the bug. Only two modifications are needed: to actually
run the test on this platform (previously it was only run on platforms
where getcwd(NULL,0) works), and a new exit code.


2011-11-20  Bruno Haible  

getcwd: Work around getcwd bug on AIX 5..7.
* m4/getcwd-path-max.m4 (gl_FUNC_GETCWD_PATH_MAX): Require
AC_CANONICAL_HOST. Assign exit code 31 to the bug seen on AIX 5.1..7.1.
Use a different value for gl_cv_func_getcwd_path_max. Move the
definition of HAVE_PARTLY_WORKING_GETCWD from here...
* m4/getcwd.m4 (gl_FUNC_GETCWD): ... to here. Invoke
gl_FUNC_GETCWD_PATH_MAX also when $gl_cv_func_getcwd_null is 'no'.
Define HAVE_MINIMALLY_WORKING_GETCWD.
* lib/getcwd.c (__getcwd): Don't use the system's getcwd on platforms
where it is not even minimally working, that is, on AIX.
* tests/test-getcwd.c (test_long_name): Distinguish the same cases as
m4/getcwd-path-max.m4.
(main): Update exit code computation.
* doc/posix-functions/getcwd.texi: Mention list of platforms where
getcwd does not handle long file names.

--- doc/posix-functions/getcwd.texi.origMon Nov 21 00:44:38 2011
+++ doc/posix-functions/getcwd.texi Mon Nov 21 00:35:22 2011
@@ -33,7 +33,8 @@
 This function is missing on some older platforms.
 @item
 This function does not handle long file names (greater than @code{PATH_MAX})
-correctly on some platforms.
+correctly on some platforms:
+glibc on Linux 2.4.20, MacOS X 10.5, FreeBSD 6.4, NetBSD 5.1, OpenBSD 4.9, AIX 
7.1.
 @end itemize
 
 Portability problems not fixed by Gnulib:
--- lib/getcwd.c.orig   Mon Nov 21 00:44:38 2011
+++ lib/getcwd.cMon Nov 21 00:24:42 2011
@@ -135,7 +135,7 @@
   size_t allocated = size;
   size_t used;
 
-#if HAVE_RAW_DECL_GETCWD
+#if HAVE_RAW_DECL_GETCWD && HAVE_MINIMALLY_WORKING_GETCWD
   /* If AT_FDCWD is not defined, the algorithm below is O(N**2) and
  this is much slower than the system getcwd (at least on
  GNU/Linux).  So trust the system getcwd's results unless they
@@ -143,7 +143,12 @@
 
  Use the system getcwd even if we have openat support, since the
  system getcwd works even when a parent is unreadable, while the
- openat-based approach does not.  */
+ openat-based approach does not.
+
+ But on AIX 5.1..7.1, the system getcwd is not even minimally
+ working: If the current directory name is slightly longer than
+ PATH_MAX, it omits the first directory component and returns
+ this wrong result with errno = 0.  */
 
 # undef getcwd
   dir = getcwd (buf, size);
--- m4/getcwd-path-max.m4.orig  Mon Nov 21 00:44:38 2011
+++ m4/getcwd-path-max.m4   Mon Nov 21 00:29:53 2011
@@ -1,4 +1,4 @@
-# serial 18
+# serial 19
 # Check for several getcwd bugs with long file names.
 # If so, arrange to compile the wrapper function.
 
@@ -16,6 +16,7 @@
 AC_DEFUN([gl_FUNC_GETCWD_PATH_MAX],
 [
   AC_CHECK_DECLS_ONCE([getcwd])
+  AC_REQUIRE([AC_CANONICAL_HOST]) dnl for cross-compiles
   AC_REQUIRE([gl_USE_SYSTEM_EXTENSIONS])
   AC_CHECK_HEADERS_ONCE([unistd.h])
   AC_REQUIRE([gl_PATHMAX_SNIPPET_PREREQ])
@@ -124,7 +125,12 @@
   fail = 11;
   break;
 }
-  if (c || ! (errno == ERANGE || is_ENAMETOOLO

Re: warning options

2011-11-20 Thread Bruno Haible
Hi Paul,

Thanks for your feedback.

> > an empty
> > function argument list (in a function definition!) is equivalent to (void).
> 
> It's not equivalent in C, since (void) gives us extra compile-time type
> checking that an empty arglist does not.  For example, there is no
> type error (and no GCC diagnostic) in this C code:
> 
>   int f () { return 0; }
>   int main (void) { return f (1); }
> 
> whereas there would be a type error if the () were replaced by (void).

It is true that there is an error if we write (void) instead of ().
But "gcc -Wall" produces a warning for

  int f () { return 0; }
  int main (void) { return f (1); }

namely:
f.c: In function 'main':
f.c:2:3: warning: call to function 'f' without a real prototype
f.c:1:7: note: 'f' was declared here

whereas it produces no warning for

  int f () { return 0; }
  int main (void) { return f (); }

Since I use -Wall for all compilations anyway, there is no benefit for
me in -Wstrict-prototypes or -Wold-style-definition: -Wall warns about
precisely the dangerous cases, whereas -Wstrict-prototypes and
-Wold-style-definition also warn for harmless code.

Additionally, by your explanation of how () differs from (void), the
'main' function is better declared as () not (void) - since crt0.o calls it
with two arguments. But -Wstrict-prototypes and -Wold-style-definition
give me a warning each time I write

  int main () { ... }

> > I also won't use -Wunused-macros since it produces warnings that are 
> > pointless
> > to fix:
> > 
> > printf-frexp.c:63:0: warning: macro "L_" is not used
> > tempname.c:70:0: warning: macro "__open64" is not used
> > tempname.c:72:0: warning: macro "__xstat64" is not used
> > fnmatch.c:171:0: warning: macro "STRCOLL" is not used
> > fnmatch.c:199:0: warning: macro "STRCOLL" is not used
> > ...
> 
> The first warning I'll grant you (though surely it is easy to work around)

Here I would have to conditionalize a macro because it happens to be unused
if a particular #if cascade is taken.

> but I don't see why the next four warnings are pointless to fix

These are in code that we borrow from glibc. We want to minimize the
differences to the original glibc code. Removing these macro definitions
would hamper future merges.

> > Even -Wshadow fires back unreasonably:
> 
> This may be taste, but I don't find that warning unreasonable,
> as a local (yn) is shadowing a global (yn).

I would find it useful to have different warning options, one for
the shadowing of local variables or parameters, and a different one
for global entities. Getting a warning for each variable called
'y0', 'y1', 'exp', or 'index' is so annoying.

Bruno
-- 
In memoriam Kerem Yılmazer 



Re: declare sethostname if unistd.h doesn't

2011-11-20 Thread Ben Walton
Excerpts from Simon Josefsson's message of Sun Nov 20 14:04:42 -0500 2011:

Hi Simon,

> Have you tested that it resolves the portability issue?  If so that is
> great.  My man page says Solaris uses this prototype:
> 
>  int sethostname(char *name, int namelen);

I used a modified version of the patch to compile inetutils and got
to the logprio.h issue, which was past the sethostname issue.

> however possibly int and size_t have the same size on all Solaris 8 and
> 9 architectures?

You're right that this should be int though.  That's what the solaris
man page indicates...I grabbed the prototype from the wrong terminal
it would seem.

> In any case, I support installing this if it does something useful
> on at least some (Solaris) machine, since the current situation does
> not appear to work on any (Solaris) machine.

It could be a local modification of the inetutils gnulib code until
the proper version is implemented?

Thanks
-Ben
--
Ben Walton
Systems Programmer - CHASS
University of Toronto
C:416.407.5610 | W:416.978.4302




Re: declare sethostname if unistd.h doesn't

2011-11-20 Thread Ben Walton
Excerpts from Bruno Haible's message of Sun Nov 20 09:12:55 -0500 2011:

Hi Bruno,

Thanks for the detailed rundown on this patch.  It's nice to get such
good feedback.

> Yes, the declaration is missing in the Solaris 10 header files, but
> also on other platforms that have the function: AIX and OSF/1.
> 
> The usual approach in gnulib is to have the header file module
> ('unistd' in this case) provide the types and C macros that ought to
> be present in that header file, while functions declarations and
> function definitions go into a module per function.

Ok, thanks.  I did read the docs but as I wasn't (at the time) doing a
separate module, I thought the declarations could go directly in the
header.

> And when we provide the function module, we try to fix as many
> problems as reasonably possible. This would mean, provide code for
> the function on Minix 3.1.8, AIX 5.1, Cygwin, mingw, MSVC 9, Interix
> 3.5, BeOS.  Even if, in the worst case, the function just sets errno
> to ENOSYS.

Fair enough.  I was scratching a particular itch but I agree that
solving the problem entirely is much better.

> Here, please use one of the templates from
> build-aux/snippet/c++defs.h.  These templates make sure that C++
> programs will see the right declarations too.

Again, this is something that I wasn't clear on, so I appreciate the
pointers.

> Also, please, Gnulib uses spaces for indentation in most
> files. Exceptions are ChangeLog, Makefile snippets in module
> descriptions, and Makefiles.

Sorry about that.  I'll drop some directory local emacs config in
place.

> Now, how to implement sethostname() on platforms that don't have it?
> 
> - On Cygwin and native Windows, SetComputerNameEx [1] seems to be
>   the best suited primitive.
> 
> - On AIX 5.1, sethostname() actually exists; only this can't be seen
>   by using "nm /lib/libc.a" as I had done.
> 
> - On Minix 3.1.8, sethostname() should write the hostname into
>   /etc/hostname.file, followed by a newline.
> 
> - On other systems, just use errno = ENOSYS; as fallback.
> 
> Can you implement this?

I'm going to give it a shot.  As long as you're not expecting it
'tomorrow', I'll work on it over the next few days.

Thanks
-Ben
--
Ben Walton
Systems Programmer - CHASS
University of Toronto
C:416.407.5610 | W:416.978.4302




Re: warning options

2011-11-20 Thread Paul Eggert
On 11/20/11 16:25, Bruno Haible wrote:
> "gcc -Wall" produces a warning for
> 
>   int f () { return 0; }
>   int main (void) { return f (1); }

That's weird.  It doesn't produce a warning for me, for either
GCC 4.6.1 (bundled with Fedora 15) or GCC 4.6.2 (which I built):

$ cat t.c
int f () { return 0; }
int main (void) { return f (1); }
$ gcc -Wall t.c
$ 

> the 'main' function is better declared as () not (void) - since crt0.o calls 
> it
> with two arguments.

I don't quite follow that argument.  On platforms where the ABI
is OK if a function is called with extra arguments, crt0.o can
call main with a dozen arguments if it wants to.
The worry is about what happens on platforms where it's
not OK to call a function with extra arguments.
On such platforms, the system presumably links differently
depending on which form of 'main' the program defines.  But
since the C standard doesn't say what to do when 'main' is
defined without a prototype, the system need not do the
right thing for that case.

> We want to minimize the
> differences to the original glibc code.

Sure, but those macros defns aren't needed anywhere.  They're in glibc
only because of gnulib.  If gnulib doesn't need them, they should be
removed from both glibc and gnulib.