Re: #define FOO rpl_FOO in config.h

[John W. Eaton]

On 29-Nov-2011, Eric Blake wrote:

| On 11/29/2011 01:01 AM, John W. Eaton wrote:
| > While trying to cross compile Octave for MinGW, I hit the following
| > errors:
| > 
| >   In file included from /home/jwe/src/octave/liboctave/oct-time.h:26:0,
| >from /home/jwe/src/octave/liboctave/file-stat.h:28,
| >from /home/jwe/src/octave/liboctave/file-ops.cc:43:
| >   /usr/include/c++/4.6/ctime:72:11: error: '::gmtime' has not been declared
| >   /usr/include/c++/4.6/ctime:73:11: error: '::localtime' has not been 
declared
| > 
| > 
| > Would it be possible to handle these definitions in gnulib's time.h
| > instead of inserting them in config.h?
| 
| Yes, we need to modernize this module to get rid of the in-config.h
| replacements, like we have done for other modules.  Would you like to
| try helping in that process?  See commit 60b73b05 for an example of
| where we did the same for mktime().

Hmm, it seems like there is a bit more to it than that...

My first attempt is attached below.  I can try to provide a complete
changeset with commit log info later if this diff is heading in the
right direction.

At best, I have about half a clue about what is actually needed here.
For example, is it necessary to provide declarations for gmtime and
localtime in case the system does not have them?  Should we check for
the functions or for declarations directly?  I couldn't tell what is
appropriate by looking at other modules.  Some appear to check for the
function and provide the declaration based on the existence of the
function (strptime in strptime.m4, for example) while others check for
declarations directly (localtime_r in time_r.m4, for example).

Also, I think the reason that we didn't notice this problem before now
is that it only affects cross compiling because of this code in
gettimeofday.m4:

 dnl When crosscompiling, assume it is broken.
 [gl_cv_func_gettimeofday_clobber=yes])])

I guess MinGW does not actually have this problem, but this path is
taken because of cross compiling.

jwe

diff --git a/lib/time.in.h b/lib/time.in.h
index ca852e8..51bf0d7 100644
--- a/lib/time.in.h
+++ b/lib/time.in.h
@@ -188,6 +188,48 @@ _GL_CXXALIASWARN (gmtime_r);
 #  endif
 # endif
 
+/* Convert TIMER to RESULT, assuming local time and UTC respectively.  See
+    and
+   .  */
+# if @REPLACE_LOCALTIME@
+#  if !(defined __cplusplus && defined GNULIB_NAMESPACE)
+#   undef localtime
+#   define localtime rpl_localtime
+#  endif
+_GL_FUNCDECL_RPL (localtime, struct tm *, (time_t const *restrict __timer,
+   struct tm *restrict __result)
+  _GL_ARG_NONNULL ((1, 2)));
+_GL_CXXALIAS_RPL (localtime, struct tm *, (time_t const *restrict __timer,
+ struct tm *restrict __result));
+# else
+#  if ! @HAVE_LOCALTIME@
+_GL_FUNCDECL_SYS (localtime, struct tm *, (time_t const *restrict __timer,
+   struct tm *restrict __result)
+  _GL_ARG_NONNULL ((1, 2)));
+#  endif
+_GL_CXXALIAS_SYS (localtime, struct tm *, (time_t const *restrict __timer,
+   struct tm *restrict __result));
+# endif
+# if @REPLACE_GMTIME@
+#  if !(defined __cplusplus && defined GNULIB_NAMESPACE)
+#   undef gmtime
+#   define gmtime rpl_gmtime
+#  endif
+_GL_FUNCDECL_RPL (gmtime, struct tm *, (time_t const *restrict __timer,
+struct tm *restrict __result)
+   _GL_ARG_NONNULL ((1, 2)));
+_GL_CXXALIAS_RPL (gmtime, struct tm *, (time_t const *restrict __timer,
+struct tm *restrict __result));
+# else
+#  if ! @HAVE_GMTIME@
+_GL_FUNCDECL_SYS (gmtime, struct tm *, (time_t const *restrict __timer,
+struct tm *restrict __result)
+   _GL_ARG_NONNULL ((1, 2)));
+#  endif
+_GL_CXXALIAS_SYS (gmtime, struct tm *, (time_t const *restrict __timer,
+struct tm *restrict __result));
+# endif
+
 /* Parse BUF as a time stamp, assuming FORMAT specifies its layout, and store
the resulting broken-down time into TM.  See
.  */
diff --git a/m4/gettimeofday.m4 b/m4/gettimeofday.m4
index 47c1e1d..b79ee9a 100644
--- a/m4/gettimeofday.m4
+++ b/m4/gettimeofday.m4
@@ -112,10 +112,15 @@ AC_DEFUN([gl_FUNC_GETTIMEOFDAY_CLOBBER],
 ])
 
 AC_DEFUN([gl_GETTIMEOFDAY_REPLACE_LOCALTIME], [
-  AC_DEFINE([gmtime], [rpl_gmtime],
-[Define to rpl_gmtime if the replacement function should be used.])
-  AC_DEFINE([localtime], [rpl_localtime],
-[Define to rpl_localtime if the replacement function should be used.])
+  AC_CHECK_FUNCS_ONCE([gmtime localti

Re: Removing -Wunsuffixed-float-constants, -Wdouble-promotion, -Wformat-zero-length

[Simon Josefsson]

Paul Eggert  writes:

> On 11/29/11 09:35, Jim Meyering wrote:
>> +  gl_WARN_ADD([-Wno-unsuffixed-float-constants])
>
> How about if we remove -Wunsuffixed-float-constants from
> manywarnings.m4?  In practice it typically causes more
> trouble than it cures (the above-quoted gzip patch is
> one example, but I've run into it elsewhere).
>
> While we're on the subject of manywarnings.m4, can we also
> remove -Wdouble-promotion and -Wformat-zero-length?  They
> also seem to be more suited for special- rather than
> general-purpose code.

I'm opposed to this -- the philosophy behind the manywarnings module is
to enable all possible kind of warnings that you can get from gcc and
then each project has to disable the warnings that they decide are not
worth fixing.  That decision has to be different for each project.  I
believe this is fairly well described in doc/manywarnings.texi.

I'm not opposed to create a somewarnings.m4 or similar that filters out
some warnings, if you would find that useful.

FWIW, I have not had to disable the above flags in any of my projects,
and I would want to be informed if I introduce new code that triggers
them.

/Simon

Re: Removing -Wunsuffixed-float-constants, -Wdouble-promotion, -Wformat-zero-length

[Paul Eggert]

On 11/30/11 02:10, Simon Josefsson wrote:
> the philosophy behind the manywarnings module is
> to enable all possible kind of warnings that you can get from gcc

Hmm, OK, but in that case, shouldn't it be called "allwarnings"
rather than "manywarnings"?  I got the impression from its name
that it doesn't enable every warning in sight

Re: Removing -Wunsuffixed-float-constants, -Wdouble-promotion, -Wformat-zero-length

[Jim Meyering]

Simon Josefsson wrote:

> Paul Eggert  writes:
>
>> On 11/29/11 09:35, Jim Meyering wrote:
>>> +  gl_WARN_ADD([-Wno-unsuffixed-float-constants])
>>
>> How about if we remove -Wunsuffixed-float-constants from
>> manywarnings.m4?  In practice it typically causes more
>> trouble than it cures (the above-quoted gzip patch is
>> one example, but I've run into it elsewhere).
>>
>> While we're on the subject of manywarnings.m4, can we also
>> remove -Wdouble-promotion and -Wformat-zero-length?  They
>> also seem to be more suited for special- rather than
>> general-purpose code.
>
> I'm opposed to this -- the philosophy behind the manywarnings module is
> to enable all possible kind of warnings that you can get from gcc and
> then each project has to disable the warnings that they decide are not
> worth fixing.  That decision has to be different for each project.  I
> believe this is fairly well described in doc/manywarnings.texi.
>
> I'm not opposed to create a somewarnings.m4 or similar that filters out
> some warnings, if you would find that useful.

Even -Wunsuffixed-float-constants has a use.
For example, I've just pushed this:

>From e6ac916bda3887c54cc8668be7a0936b034a28c3 Mon Sep 17 00:00:00 2001
From: Jim Meyering 
Date: Wed, 30 Nov 2011 09:35:00 +0100
Subject: [PATCH] hash: mark a few floating point constants with "f" suffix

* lib/hash.c (DEFAULT_GROWTH_THRESHOLD, DEFAULT_GROWTH_FACTOR)
(DEFAULT_SHRINK_THRESHOLD, DEFAULT_SHRINK_FACTOR): Mark literal
floating point constants with "f", since they're destined to be
saved/used as "float"s.
---
 ChangeLog  |8 
 lib/hash.c |8 
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 272dded..4d1c94c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2011-11-30  Jim Meyering  
+
+   hash: mark a few floating point constants with "f" suffix
+   * lib/hash.c (DEFAULT_GROWTH_THRESHOLD, DEFAULT_GROWTH_FACTOR)
+   (DEFAULT_SHRINK_THRESHOLD, DEFAULT_SHRINK_FACTOR): Mark literal
+   floating point constants with "f", since they're destined to be
+   saved/used as "float"s.
+
 2011-11-29  Paolo Bonzini  

float tests: Correct and re-enable assertion about LDBL_MIN_EXP.
diff --git a/lib/hash.c b/lib/hash.c
index 1dd657a..0ee32d0 100644
--- a/lib/hash.c
+++ b/lib/hash.c
@@ -113,8 +113,8 @@ struct hash_table
1.0).  The growth threshold defaults to 0.8, and the growth factor
defaults to 1.414, meaning that the table will have doubled its size
every second time 80% of the buckets get used.  */
-#define DEFAULT_GROWTH_THRESHOLD 0.8
-#define DEFAULT_GROWTH_FACTOR 1.414
+#define DEFAULT_GROWTH_THRESHOLD 0.8f
+#define DEFAULT_GROWTH_FACTOR 1.414f

 /* If a deletion empties a bucket and causes the ratio of used buckets to
table size to become smaller than the shrink threshold (a number between
@@ -122,8 +122,8 @@ struct hash_table
number greater than the shrink threshold but smaller than 1.0).  The shrink
threshold and factor default to 0.0 and 1.0, meaning that the table never
shrinks.  */
-#define DEFAULT_SHRINK_THRESHOLD 0.0
-#define DEFAULT_SHRINK_FACTOR 1.0
+#define DEFAULT_SHRINK_THRESHOLD 0.0f
+#define DEFAULT_SHRINK_FACTOR 1.0f

 /* Use this to initialize or reset a TUNING structure to
some sensible values. */
--
1.7.8.rc4

Re: Removing -Wunsuffixed-float-constants, -Wdouble-promotion, -Wformat-zero-length

[Simon Josefsson]

Eric Blake  writes:

> On 11/29/2011 02:46 PM, Eric Blake wrote:
>>> Unless there are objections (portability?)
>> 
>> Aargh.  I just reread C99.
>> 
>> F (and f) for float, and L (or l) for long double are required, but D
>> (or d) for double is a GNU extension.
>> 
>> Since we can't silence the warning without adding an explicit 'D', but
>> 'D' is not standardized, I have changed my mind.  Let's nuke the warning.
>
> So for starters, I'm pushing this:

Please revert or change this -- like other warnings, this warning is
useful in some projects and not useful in others, and the idea with
manywarnings.m4 is that all warnings should be enabled and each project
has to disable the warnings they don't like incrementally.  See
doc/manywarnings.texi for background.

The function you added modified was this one:

# gl_MANYWARN_ALL_GCC(VARIABLE)
# -
# Add all documented GCC (currently as per version 4.4) warning
# parameters to variable VARIABLE.

I'm fine with creating another function gl_MANYWARN_ALL_REASONEBLE_GCC
or similar if you prefer that, but let's keep the base function clean
from subjective filtering of warning flags.  It should just generate a
list of all warning flags that can possibly be enabled in gcc.

/Simon

Re: Removing -Wunsuffixed-float-constants, -Wdouble-promotion, -Wformat-zero-length

[Jim Meyering]

Simon Josefsson wrote:

> Eric Blake  writes:
>
>> On 11/29/2011 02:46 PM, Eric Blake wrote:
 Unless there are objections (portability?)
>>>
>>> Aargh.  I just reread C99.
>>>
>>> F (and f) for float, and L (or l) for long double are required, but D
>>> (or d) for double is a GNU extension.
>>>
>>> Since we can't silence the warning without adding an explicit 'D', but
>>> 'D' is not standardized, I have changed my mind.  Let's nuke the warning.
>>
>> So for starters, I'm pushing this:
>
> Please revert or change this -- like other warnings, this warning is
> useful in some projects and not useful in others, and the idea with
> manywarnings.m4 is that all warnings should be enabled and each project
> has to disable the warnings they don't like incrementally.  See
> doc/manywarnings.texi for background.
>
> The function you added modified was this one:
>
> # gl_MANYWARN_ALL_GCC(VARIABLE)
> # -
> # Add all documented GCC (currently as per version 4.4) warning
> # parameters to variable VARIABLE.

Hi Simon,

Isn't that 4.4 out of date, now?
Considering you updated the list recently, I'd expect 4.6 there.

Re: Removing -Wunsuffixed-float-constants, -Wdouble-promotion, -Wformat-zero-length

[Jim Meyering]

Jim Meyering wrote:

> Simon Josefsson wrote:
>
>> Eric Blake  writes:
>>
>>> On 11/29/2011 02:46 PM, Eric Blake wrote:
> Unless there are objections (portability?)

 Aargh.  I just reread C99.

 F (and f) for float, and L (or l) for long double are required, but D
 (or d) for double is a GNU extension.

 Since we can't silence the warning without adding an explicit 'D', but
 'D' is not standardized, I have changed my mind.  Let's nuke the warning.
>>>
>>> So for starters, I'm pushing this:
>>
>> Please revert or change this -- like other warnings, this warning is
>> useful in some projects and not useful in others, and the idea with
>> manywarnings.m4 is that all warnings should be enabled and each project
>> has to disable the warnings they don't like incrementally.  See
>> doc/manywarnings.texi for background.
>>
>> The function you added modified was this one:
>>
>> # gl_MANYWARN_ALL_GCC(VARIABLE)
>> # -
>> # Add all documented GCC (currently as per version 4.4) warning
>> # parameters to variable VARIABLE.
>
> Hi Simon,
>
> Isn't that 4.4 out of date, now?
> Considering you updated the list recently, I'd expect 4.6 there.

It may be better just to remove that parenthetical remark.

Instead, how about automating the extraction of the list of
warnings from gcc sources or from some web page?  Maybe you've
already done something like that?
Then we could add a rule to gnulib's Makefile that would ensure
our list is up to date and run it periodically (albeit manually).

Re: [RFC] sethostname handling patch series

[Bruno Haible]

Hello Ben,

Your patches look pretty good. In parallel to the tweaking of the last
details, it will be useful to assign the copyright for the change to the
FSF. This is needed because Gnulib is central to many GNU packages.

To go ahead with this, please fill in the form in the file
gnulib/doc/Copyright/request-assign.future (if you foresee making more
contributions to Gnulib) or gnulib/doc/Copyright/request-assign.changes
(if this will be your only contribution) and send it to the FSF.

Yeah, it's paperwork. I know.

Bruno
-- 
In memoriam Alfred Herrhausen 

Re: [PATCH 1/3] Split the HOST_NAME_MAX detection into separate m4 macro

[Bruno Haible]

Ben Walton wrote:
> --- a/m4/gethostname.m4
> +++ b/m4/gethostname.m4
> @@ -1,4 +1,4 @@
> -# gethostname.m4 serial 12
> +# gethostname.m4 serial 13
>  dnl Copyright (C) 2002, 2008-2011 Free Software Foundation, Inc.
>  dnl This file is free software; the Free Software Foundation
>  dnl gives unlimited permission to copy and/or distribute it,
> @@ -40,6 +40,10 @@ AC_DEFUN([gl_FUNC_GETHOSTNAME],
>  HAVE_GETHOSTNAME=0
>fi
>  
> +  gl_PREREQ_HOST_NAME_MAX
> +])
> +
> +AC_DEFUN([gl_PREREQ_HOST_NAME_MAX], [
>dnl Also provide HOST_NAME_MAX when  lacks it.
>dnl - On most Unix systems, use MAXHOSTNAMELEN from  instead.
>dnl - On Solaris, Cygwin, BeOS, use MAXHOSTNAMELEN from  instead.
> 

Looks fine. I would move the one-line comment
"Provide HOST_NAME_MAX when  lacks it." outside the new macro,
as a quick overview of what it does. The details documentation can stay
inside the macro.

Bruno
-- 
In memoriam Alfred Herrhausen 

Re: Removing -Wunsuffixed-float-constants, -Wdouble-promotion, -Wformat-zero-length

[Paul Eggert]

On 11/30/11 02:25, Jim Meyering wrote:
> how about automating the extraction of the list of
> warnings from gcc sources or from some web page

Or perhaps extract it from the gcc executable
itself?  Then you'd get all the warnings that the
current GCC supports.

Re: [PATCH 2/3] Add a new sethostname module

[Bruno Haible]

Ben Walton wrote:
> --- a/doc/glibc-functions/sethostname.texi
> +++ b/doc/glibc-functions/sethostname.texi
> @@ -2,10 +2,15 @@
>  @subsection @code{sethostname}
>  @findex sethostname
>  
> -Gnulib module: ---
> +Gnulib module: sethostname

Good.

>  Portability problems fixed by Gnulib:
>  @itemize
> +@item
> +On AIX 7.1, OSF/1 5.1 and Solaris 10 the declaration is missing.

Here you can just move the wording from the "problems not fixed by Gnulib"
to the "problems fixed by Gnulib" section.

> On
> +Minix 3.1.8, Cygwin, mingw, MSVC 9, Interix 3.5, BeOS the function is
> +missing.

Likewise.

> Provide a fallback for all platforms that returns -1 and
> +sets ENOSYS.

Can you reword this as a limitation? "But on some platforms
the Gnulib replacement always fails with ENOSYS."

> Provide a specific implementation for Minix 3.1.8

It's not worth mentioning that the function will work on Minix. Here
in the doc we mention only the problems.

> +@item
> +On Solaris 10, the first argument is @code{char *} instead of
> +@code{const char *} and the second parameter is @code{int} instead of
> +@code{size_t}.

With further adjustments to the patch, this will go to the section
"problems fixed by Gnulib", I guess?

> +/* Set up to LEN chars of NAME as system hostname.
> +   Return 0 if ok, -1 if error.  */

The comments should also say that errno is set upon error.

> +int
> +sethostname (const char *name, size_t len)
> +{
> +  /* glibc seems to allow a zero length hostname: bail on names that
> + are too long or too short */
> +  if (len < 0 || len > HOST_NAME_MAX)
> +{
> +  errno = EINVAL;
> +  return -1;
> +}

Good idea to check the length. But the "len < 0" expression will always
evaluate to false, since size_t is an unsigned type.

> +  /* NAME does not need to be null terminated so leave room to terminate
> + regardless of input. */
> +  char hostname[len + 1];

An array with dynamically computed size, as a local variable in a function,
is an ISO C 99 feature that many compilers don't yet support. The alternative
is a fixed-size array or a malloc(). Since you already verified the bounds on
len, a fixed-size array seems more appropriate to me.

> +  strncpy(hostname, name, len);

GNU coding style prefers to have a space before the opening parenthesis for
function and macro calls:

 strncpy (hostname, name, len);

Oh, and why not use memcpy? It's a simpler function than strncpy.

> +  hostname[len] = '\0';
> +
> +#ifdef __minix /* Minix */
> +  FILE *hostf;

Declaration after statement is also a C99 feature that we cannot assume 
portably.
Workaround: Insert braces around the code that needs the 'hostf' variable.

> +
> +  /* leave errno alone in this case as it will provide a more useful error
> + indication than overriding with ENOSYS */
> +  if ((hostf = fopen("/etc/hostname.file", "w")) == NULL)

Stylistically, we find it better to not have assignments inside 'if' conditions.
Yeah, I know the Linux kernel code style is just the opposite...

> +return -1;
> +  else
> +{
> +  fprintf(hostf, "%s\n", hostname);
> +  fclose(hostf);

Robust programming: Check the return value of fclose(), and check whether there
was an error on the stream before fclose().
> +  return 0;
> +}
> +#else
> +  /* For platforms that we don't have a better option for, simply bail
> + out */
> +  errno = ENOSYS;
> +  return -1;
> +#endif
> +}

OK for the rest.

> diff --git a/m4/sethostname.m4 b/m4/sethostname.m4
> new file mode 100644
> index 000..dbdbb39
> --- /dev/null
> +++ b/m4/sethostname.m4
> @@ -0,0 +1,21 @@
> +# sethostname.m4 serial 1
> +dnl Copyright (C) 2011 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.
> +
> +# Ensure
> +# - the sethostname() function,
> +AC_DEFUN([gl_FUNC_SETHOSTNAME],
> +[
> +  AC_REQUIRE([gl_UNISTD_H_DEFAULTS])
> +
> +  AC_REPLACE_FUNCS([sethostname])
> +  AC_CHECK_DECLS([sethostname])

Good, but you also need to set HAVE_SETHOSTNAME.

> +  if test $ac_cv_func_sethostname = no; then
> +gl_PREREQ_HOST_NAME_MAX
> +  fi

The caller of sethostname() may want to use the HOST_NAME_MAX
unconditionally. I would therefore invoke gl_PREREQ_HOST_NAME_MAX
unconditionally. (We have no module for , otherwise we
would do it there.)

> diff --git a/modules/sethostname b/modules/sethostname
> new file mode 100644
> index 000..a0f36a2
> --- /dev/null
> +++ b/modules/sethostname
> @@ -0,0 +1,32 @@
> +Description:
> +sethostname() function: Set machine's hostname.
> +
> +Files:
> +lib/sethostname.c
> +m4/sethostname.m4
> +
> +Depends-on:
> +unistd
> +gethostname

The sethostname replacement does not need the code (lib/gethostname.c),
nor does it need to invoke gl_FUNC_GETHOSTNAME. All it needs is the
file m4/gethostname.m4. Therefore I would remove the dependency and
inste

Re: Removing -Wunsuffixed-float-constants, -Wdouble-promotion, -Wformat-zero-length

[Simon Josefsson]

Paul Eggert  writes:

> On 11/30/11 02:10, Simon Josefsson wrote:
>> the philosophy behind the manywarnings module is
>> to enable all possible kind of warnings that you can get from gcc
>
> Hmm, OK, but in that case, shouldn't it be called "allwarnings"
> rather than "manywarnings"?  I got the impression from its name
> that it doesn't enable every warning in sight

The intention with the module is that packages get "many" (not all)
warnings enabled, however the gl_MANYWARN_ALL_GCC function (which was
modified here, and is just one part of the module) should list "all"
warnings.  Renaming the module "allwarnings" sounds a bit confusing as
well, since the purpose is not to enable all warnings...  but I don't
care strongly about the name.

/Simon

Re: Removing -Wunsuffixed-float-constants, -Wdouble-promotion, -Wformat-zero-length

[Simon Josefsson]

Jim Meyering  writes:

> Simon Josefsson wrote:
>
>> Eric Blake  writes:
>>
>>> On 11/29/2011 02:46 PM, Eric Blake wrote:
> Unless there are objections (portability?)

 Aargh.  I just reread C99.

 F (and f) for float, and L (or l) for long double are required, but D
 (or d) for double is a GNU extension.

 Since we can't silence the warning without adding an explicit 'D', but
 'D' is not standardized, I have changed my mind.  Let's nuke the warning.
>>>
>>> So for starters, I'm pushing this:
>>
>> Please revert or change this -- like other warnings, this warning is
>> useful in some projects and not useful in others, and the idea with
>> manywarnings.m4 is that all warnings should be enabled and each project
>> has to disable the warnings they don't like incrementally.  See
>> doc/manywarnings.texi for background.
>>
>> The function you added modified was this one:
>>
>> # gl_MANYWARN_ALL_GCC(VARIABLE)
>> # -
>> # Add all documented GCC (currently as per version 4.4) warning
>> # parameters to variable VARIABLE.
>
> Hi Simon,
>
> Isn't that 4.4 out of date, now?
> Considering you updated the list recently, I'd expect 4.6 there.

Oops.  I removed the reference to a version there, it is not useful in
the general function documentation.

/Simon

Re: [PATCH 3/3] Integrate the sethostname module into unistd

[Bruno Haible]

Ben Walton wrote:
> --- a/lib/unistd.in.h
> +++ b/lib/unistd.in.h
> @@ -683,6 +683,30 @@ _GL_WARN_ON_USE (getgroups, "getgroups is unportable - "
>  # endif
>  #endif
>  
> +#if @GNULIB_SETHOSTNAME@
> +/* Set the host name of the machine.
> +   The host name may or may not be fully qualified.
> +
> +   Put LEN bytes of NAME into the host name.
> +   Return 0 if successful, otherwise, set errnon and return -1

Typo: errno.

> +   Platforms with no ability to set the hostname return -1 and set
> +   errno = ENOSYS. */
> +#  if !@HAVE_SETHOSTNAME@ || !@HAVE_DECL_SETHOSTNAME@
> +_GL_FUNCDECL_SYS (sethostname, int, (const char *name, size_t len)
> +_GL_ARG_NONNULL ((1)));
> +#  endif
> +/* Need to cast, because on Solaris 10, the first parameter is not
> +   const and the second parameter is int len.  */

IMO the comment should mention Solaris 11 2010-11, not Solaris 10. On
Solaris 10, HAVE_DECL_SETHOSTNAME will come out as 0, hence the
declaration above will be enabled, and no cast is necessary. Only on
Solaris 11 the cast is necessary.

And btw, the cast is also needed on MacOS X, FreeBSD, IRIX,
because these platforms also have a declaration
  int   sethostname(const char *, int);
So the entire platforms list to mention here is
  MacOS X 10.5, FreeBSD 6.4, IRIX 6.5, Solaris 11 2010-11.

> @@ -132,6 +133,7 @@ AC_DEFUN([gl_UNISTD_H_DEFAULTS],
>HAVE_READLINK=1;AC_SUBST([HAVE_READLINK])
>HAVE_READLINKAT=1;  AC_SUBST([HAVE_READLINKAT])
>HAVE_SLEEP=1;   AC_SUBST([HAVE_SLEEP])
> +  HAVE_SETHOSTNAME=1;  AC_SUBST([HAVE_SETHOSTNAME])
>HAVE_SYMLINK=1; AC_SUBST([HAVE_SYMLINK])
>HAVE_SYMLINKAT=1;   AC_SUBST([HAVE_SYMLINKAT])
>HAVE_UNLINKAT=1;AC_SUBST([HAVE_UNLINKAT])

In Gnulib, no tabs in .m4, .c, .h, .sh files, please.

The rest of the patch is good.

Thanks!!

Bruno
-- 
In memoriam Alfred Herrhausen 

Re: Removing -Wunsuffixed-float-constants, -Wdouble-promotion, -Wformat-zero-length

[Simon Josefsson]

Jim Meyering  writes:

> Instead, how about automating the extraction of the list of
> warnings from gcc sources or from some web page?  Maybe you've
> already done something like that?
> Then we could add a rule to gnulib's Makefile that would ensure
> our list is up to date and run it periodically (albeit manually).

This is unfortunately a big mess and I do it manually.  As a result of
doing it manually, I'm pretty sure some warnings are missing and some
are repeated (due to being implied by other flags).  The problems
include:

1) The gcc manual doesn't list all warning flags.

2) The gcc --help=warnings doesn't list all warning flags, although it
   is a different subset than 1).

3) Several of the warnings mentioned by gcc --help=warnings does not
   apply to C.

4) Some warning flags imply other flags, which complicate things.
   Should the redundant flag be removed or not?  It is made even more
   complicated since this may change between gcc versions.

I think there were other problems as well.  As a result, I gave up
trying to automate it and did it manually initially just to get some
warnings enabled.

It would be nice to automate this.  I'm not sure whether to use the
manual or the source code as the canonical source, although I lean
towards using the source code.  I'm not sure how easy it would be to get
a list though, I recall that warnings were not handled uniformly within
gcc sources.

/Simon

Re: Removing -Wunsuffixed-float-constants, -Wdouble-promotion, -Wformat-zero-length

[Jim Meyering]

Paul Eggert wrote:
> On 11/30/11 02:25, Jim Meyering wrote:
>> how about automating the extraction of the list of
>> warnings from gcc sources or from some web page
>
> Or perhaps extract it from the gcc executable
> itself?  Then you'd get all the warnings that the
> current GCC supports.

Hah!

I didn't know about --help=warnings.
That looks perfect.

For a quick and dirty comparison, I did this:
(using gcc 4.7.0 2024)

diff -u \
  <(sed -n 's/^  *\(-[^ ]*\) .*/\1/p' /gnulib/m4/manywarnings.m4 |sort) \
  <(gcc --help=warnings|sed -n 's/^  \(-[^ ]*\) .*/\1/p' |sort)

which shows at least one false positive (i.e. -Wformat=2 vs -Wformat=)
and many new warnings:

--- /proc/self/fd/112011-11-30 12:07:49.747440364 +0100
+++ /proc/self/fd/122011-11-30 12:07:49.747440364 +0100
@@ -1,79 +1,173 @@
+--all-warnings
+--extra-warnings
 -W
 -Wabi
+-Waddress
 -Waggregate-return
+-Waliasing
+-Walign-commons
 -Wall
+-Wampersand
+-Warray-bounds
+-Warray-temporaries
+-Wassign-intercept
 -Wattributes
 -Wbad-function-cast
 -Wbuiltin-macro-redefined
 -Wc++-compat
+-Wc++0x-compat
+-Wc++11-compat
 -Wcast-align
 -Wcast-qual
+-Wchar-subscripts
+-Wcharacter-truncation
+-Wclobbered
+-Wcomment
+-Wcomments
 -Wconversion
+-Wconversion-extra
+-Wconversion-null
 -Wcoverage-mismatch
 -Wcpp
+-Wctor-dtor-privacy
 -Wdeclaration-after-statement
+-Wdelete-non-virtual-dtor
 -Wdeprecated
 -Wdeprecated-declarations
 -Wdisabled-optimization
 -Wdiv-by-zero
 -Wdouble-promotion
+-Weffc++
+-Wempty-body
 -Wendif-labels
+-Wenum-compare
+-Werror-implicit-function-declaration
 -Wextra
 -Wfloat-equal
+-Wformat
 -Wformat-contains-nul
 -Wformat-extra-args
 -Wformat-nonliteral
 -Wformat-security
 -Wformat-y2k
 -Wformat-zero-length
--Wformat=2
+-Wformat=
+-Wfree-nonheap-object
+-Wfunction-elimination
+-Wignored-qualifiers
+-Wimplicit
+-Wimplicit-function-declaration
+-Wimplicit-int
+-Wimplicit-interface
+-Wimplicit-procedure
 -Winit-self
 -Winline
+-Wint-to-pointer-cast
+-Wintrinsic-shadow
+-Wintrinsics-std
+-Winvalid-memory-model
+-Winvalid-offsetof
 -Winvalid-pch
+-Wjump-misses-init
+-Wlarger-than-
+-Wlarger-than=
+-Wline-truncation
 -Wlogical-op
 -Wlong-long
+-Wmain
+-Wmaybe-uninitialized
+-Wmissing-braces
 -Wmissing-declarations
+-Wmissing-field-initializers
 -Wmissing-format-attribute
 -Wmissing-include-dirs
 -Wmissing-noreturn
+-Wmissing-parameter-type
 -Wmissing-prototypes
 -Wmudflap
 -Wmultichar
--Wmultichar
+-Wnarrowing
 -Wnested-externs
--Wnormalized=nfc
+-Wnoexcept
+-Wnon-template-friend
+-Wnon-virtual-dtor
+-Wnonnull
+-Wnormalized=
+-Wold-style-cast
+-Wold-style-declaration
 -Wold-style-definition
 -Woverflow
 -Woverlength-strings
+-Woverloaded-virtual
+-Woverride-init
 -Wpacked
 -Wpacked-bitfield-compat
 -Wpadded
+-Wparentheses
+-Wpmf-conversions
 -Wpointer-arith
+-Wpointer-sign
 -Wpointer-to-int-cast
 -Wpragmas
+-Wproperty-assign-default
+-Wprotocol
+-Wreal-q-constant
 -Wredundant-decls
+-Wreorder
+-Wreturn-type
+-Wselector
+-Wsequence-point
 -Wshadow
--Wsign-conversion
+-Wsign-compare
+-Wsign-promo
 -Wstack-protector
+-Wstack-usage=
 -Wstrict-aliasing
+-Wstrict-aliasing=
+-Wstrict-null-sentinel
 -Wstrict-overflow
+-Wstrict-overflow=
 -Wstrict-prototypes
+-Wstrict-selector-match
 -Wsuggest-attribute=const
 -Wsuggest-attribute=noreturn
 -Wsuggest-attribute=pure
+-Wsurprising
+-Wswitch
 -Wswitch-default
 -Wswitch-enum
 -Wsync-nand
+-Wsynth
 -Wsystem-headers
+-Wtabs
 -Wtraditional
 -Wtraditional-conversion
 -Wtrampolines
+-Wtrigraphs
+-Wtype-limits
+-Wundeclared-selector
 -Wundef
+-Wunderflow
+-Wuninitialized
 -Wunknown-pragmas
--Wunreachable-code
 -Wunsafe-loop-optimizations
+-Wunsuffixed-float-constants
 -Wunused
+-Wunused-but-set-parameter
+-Wunused-but-set-variable
+-Wunused-dummy-argument
+-Wunused-function
+-Wunused-label
+-Wunused-local-typedefs
 -Wunused-macros
+-Wunused-parameter
+-Wunused-result
+-Wunused-value
+-Wunused-variable
+-Wvariadic-macros
+-Wvector-operation-performance
 -Wvla
 -Wvolatile-register-var
 -Wwrite-strings
+-Wzero-as-null-pointer-constant
+-frequire-return-statement

Re: libidn 1.23

[Simon Josefsson]

Bruno Haible  writes:

> Simon Josefsson wrote:
>> This also came up for Libidn, it is using the latest gnulib.
>> 
>> > * mingw with gcc
>> >
>> > Fails already in the gnulib tests:
>> >
>> > FAIL: test-binary-io.sh
>> 
>> I have not seen this test fail before.
>
> I'm seeing this test failure only in the libidn and gsasl builds, not
> in the gnulib testdir.
>
> Debugging it, it turns out that the failure comes from the libtool wrapper:
>
> $ ./test-binary-io.exe > t-bin-out1.tmp ; echo $?
> 3
>
> $ .libs/test-binary-io.exe > t-bin-out1.tmp ; echo $?
> 0
>
> The exit code 3 means that the child process (.libs/test-binary-io.exe) caught
> a signal (cf. lib/sys_wait.in.h).
>
> The cause is that when the libtool wrapper is present, the wrapper process
> keeps stdin, stdout, stderr open, therefore the fclose(stdout) statement
> in the child process does not have the effect of flushing the file's data
> to disk, because the OS is waiting for the second file handle in the parent
> process to be closed as well.
>
> The details of how the libtool wrapper program works can be seen in
> .libs/lt-test-binary-io.c:
>
>   /* execv doesn't actually work on mingw as expected on unix */
>   newargz = prepare_spawn (newargz);
>   rval = _spawnv (_P_WAIT, lt_argv_zero, (const char * const *) newargz);
>
> I'm applying this fix.

Thank you!  I have re-added the test-binary-io module to libidn for
future releases.

/Simon

Re: Removing -Wunsuffixed-float-constants, -Wdouble-promotion, -Wformat-zero-length

[Jim Meyering]

Simon Josefsson wrote:
> Jim Meyering  writes:
>
>> Instead, how about automating the extraction of the list of
>> warnings from gcc sources or from some web page?  Maybe you've
>> already done something like that?
>> Then we could add a rule to gnulib's Makefile that would ensure
>> our list is up to date and run it periodically (albeit manually).
>
> This is unfortunately a big mess and I do it manually.  As a result of
> doing it manually, I'm pretty sure some warnings are missing and some
> are repeated (due to being implied by other flags).  The problems
> include:
>
> 1) The gcc manual doesn't list all warning flags.

Automating it (even if not perfect) should be better than relying
on a manual and tedious (and thus infrequently done) task whose results
would otherwise tend to be out of date.

> 2) The gcc --help=warnings doesn't list all warning flags, although it
>is a different subset than 1).
>
> 3) Several of the warnings mentioned by gcc --help=warnings does not
>apply to C.

Yes, I noticed a few C++-only warnings.
These can be removed via a hard-coded list that we'd have to
keep up to date.  Shouldn't be much work.

> 4) Some warning flags imply other flags, which complicate things.
>Should the redundant flag be removed or not?  It is made even more
>complicated since this may change between gcc versions.
>
> I think there were other problems as well.  As a result, I gave up
> trying to automate it and did it manually initially just to get some
> warnings enabled.
>
> It would be nice to automate this.  I'm not sure whether to use the
> manual or the source code as the canonical source, although I lean
> towards using the source code.  I'm not sure how easy it would be to get
> a list though, I recall that warnings were not handled uniformly within
> gcc sources.

gcc --help=warnings output is easy to get and parse,
so it seems like a good starting point.

Re: Removing -Wunsuffixed-float-constants, -Wdouble-promotion, -Wformat-zero-length

[Simon Josefsson]

Jim Meyering  writes:

> Paul Eggert wrote:
>> On 11/30/11 02:25, Jim Meyering wrote:
>>> how about automating the extraction of the list of
>>> warnings from gcc sources or from some web page
>>
>> Or perhaps extract it from the gcc executable
>> itself?  Then you'd get all the warnings that the
>> current GCC supports.
>
> Hah!
>
> I didn't know about --help=warnings.
> That looks perfect.
>
> For a quick and dirty comparison, I did this:
> (using gcc 4.7.0 2024)
>
> diff -u \
>   <(sed -n 's/^  *\(-[^ ]*\) .*/\1/p' /gnulib/m4/manywarnings.m4 |sort) \
>   <(gcc --help=warnings|sed -n 's/^  \(-[^ ]*\) .*/\1/p' |sort)
>
> which shows at least one false positive (i.e. -Wformat=2 vs -Wformat=)
> and many new warnings:

Many of the new ones you list are for non-C.  The -Wformat=2 should be
correct, I believe the gcc --help=warnings output is incorrect?
-Wformat is implied by -Wall but -Wformat=2 adds more.

The --all-warnings and --extra-warnings are interesting, they were not
present when manywarnings.m4 was created.  I wonder how they work.

/Simon

Re: [RFC] sethostname handling patch series

[Eric Blake]

On 11/30/2011 03:29 AM, Bruno Haible wrote:
> Hello Ben,
> 
> Your patches look pretty good. In parallel to the tweaking of the last
> details, it will be useful to assign the copyright for the change to the
> FSF. This is needed because Gnulib is central to many GNU packages.
> 
> To go ahead with this, please fill in the form in the file
> gnulib/doc/Copyright/request-assign.future (if you foresee making more
> contributions to Gnulib) or gnulib/doc/Copyright/request-assign.changes
> (if this will be your only contribution) and send it to the FSF.

Your email address implies Canada; but if you can claim US residency,
you can speed up the process by filing copyright assignment electronically.
http://www.gnu.org/prep/maintain/maintain.html#Copyright-Papers

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: Removing -Wunsuffixed-float-constants, -Wdouble-promotion, -Wformat-zero-length

[Eric Blake]

On 11/30/2011 03:20 AM, Simon Josefsson wrote:
> Eric Blake  writes:
> 
>> On 11/29/2011 02:46 PM, Eric Blake wrote:
 Unless there are objections (portability?)
>>>
>>> Aargh.  I just reread C99.
>>>
>>> F (and f) for float, and L (or l) for long double are required, but D
>>> (or d) for double is a GNU extension.
>>>
>>> Since we can't silence the warning without adding an explicit 'D', but
>>> 'D' is not standardized, I have changed my mind.  Let's nuke the warning.
>>
>> So for starters, I'm pushing this:
> 
> Please revert or change this -- like other warnings, this warning is
> useful in some projects and not useful in others, and the idea with
> manywarnings.m4 is that all warnings should be enabled and each project
> has to disable the warnings they don't like incrementally.  See
> doc/manywarnings.texi for background.

We already concluded that the warning is bogus as a reasonable warning -
there is no way to write portable code that both complies with C99 and
silences the warning without either ditching 'double' (using only
'float' and 'long double' constants) or using non-standard extensions
(gcc's '1.0D' for double).

But I can see your counterargument of having a complete list of all
possible gcc warnings, followed by a second list that filters out
warnings that make no sense in a gnulib project, as well as lists of
warnings that make no sense in a C-only, a C++-only, and a dual C/C++
program.  For example, -Wc++-compat makes no sense for a C-only program,
but makes perfect sense for a dual C/C++ program.  Libtool is an example
of a dual program, that strives to use the subset of C and C++ so that
it can be compiled by either language for use in both C and C++ programs.

If we have various filter lists, for all warnings minus those not suited
for this typical usage pattern, as well as a way to easily select which
filters to apply, that would indeed be nicer.
-Wunsuffixed-float-constants should then appear on the list of all
warnings, but also on pretty much every filter list as being worthless
for portable code.  We could also have a filter list for warnings that
will not work with gnulib code directly, but which projects may still
wish to use on their own code (see how coreutils has split warning
levels, for example).

> I'm fine with creating another function gl_MANYWARN_ALL_REASONEBLE_GCC
> or similar if you prefer that, but let's keep the base function clean
> from subjective filtering of warning flags.  It should just generate a
> list of all warning flags that can possibly be enabled in gcc.

Maybe more like gl_MANYWARN_ALL_GCC_EXCEPT(VARIABLE, [EXCLUSION]), where
it would be called like:

gl_MANYWARN_ALL_GCC_EXCEPT([WARN_CFLAGS],
  [gl_MANYWARN_NO_C++_WARNINGS])
gl_MANYWARN_ALL_GCC_EXCEPT([GNULIB_WARN_CFLAGS],
  [gl_MANYWARN_NO_C++_WARNINGS], [gl_MANYWARN_NO_GNULIB_WARNINGS])

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: Removing -Wunsuffixed-float-constants, -Wdouble-promotion, -Wformat-zero-length

[Simon Josefsson]

Eric Blake  writes:

> On 11/30/2011 03:20 AM, Simon Josefsson wrote:
>> Eric Blake  writes:
>> 
>>> On 11/29/2011 02:46 PM, Eric Blake wrote:
> Unless there are objections (portability?)

 Aargh.  I just reread C99.

 F (and f) for float, and L (or l) for long double are required, but D
 (or d) for double is a GNU extension.

 Since we can't silence the warning without adding an explicit 'D', but
 'D' is not standardized, I have changed my mind.  Let's nuke the warning.
>>>
>>> So for starters, I'm pushing this:
>> 
>> Please revert or change this -- like other warnings, this warning is
>> useful in some projects and not useful in others, and the idea with
>> manywarnings.m4 is that all warnings should be enabled and each project
>> has to disable the warnings they don't like incrementally.  See
>> doc/manywarnings.texi for background.
>
> We already concluded that the warning is bogus as a reasonable warning -
> there is no way to write portable code that both complies with C99 and
> silences the warning without either ditching 'double' (using only
> 'float' and 'long double' constants) or using non-standard extensions
> (gcc's '1.0D' for double).

The warning can still be useful -- it would be triggered when I
introduce code that has this property into my projects, and I will be
forced to think about whether the code may be rewritten in another style
or silence the warning.  I much prefer this from not knowing that I
introduced code with that behaviour.

As was indicated earlier in this thread, when reviewing code that
triggered this warning, some code was rewritten to not use floats at all
which is one example of a good consequence of using the warning flag.

> But I can see your counterargument of having a complete list of all
> possible gcc warnings, followed by a second list that filters out
> warnings that make no sense in a gnulib project, as well as lists of
> warnings that make no sense in a C-only, a C++-only, and a dual C/C++
> program.  For example, -Wc++-compat makes no sense for a C-only program,
> but makes perfect sense for a dual C/C++ program.  Libtool is an example
> of a dual program, that strives to use the subset of C and C++ so that
> it can be compiled by either language for use in both C and C++ programs.
>
> If we have various filter lists, for all warnings minus those not suited
> for this typical usage pattern, as well as a way to easily select which
> filters to apply, that would indeed be nicer.
> -Wunsuffixed-float-constants should then appear on the list of all
> warnings, but also on pretty much every filter list as being worthless
> for portable code.  We could also have a filter list for warnings that
> will not work with gnulib code directly, but which projects may still
> wish to use on their own code (see how coreutils has split warning
> levels, for example).

This sounds good to me.  We'd might as well improve the rather ugly code
that people have to put into their configure.ac's in order to use the
manywarnings module, see below...

>> I'm fine with creating another function gl_MANYWARN_ALL_REASONEBLE_GCC
>> or similar if you prefer that, but let's keep the base function clean
>> from subjective filtering of warning flags.  It should just generate a
>> list of all warning flags that can possibly be enabled in gcc.
>
> Maybe more like gl_MANYWARN_ALL_GCC_EXCEPT(VARIABLE, [EXCLUSION]), where
> it would be called like:
>
> gl_MANYWARN_ALL_GCC_EXCEPT([WARN_CFLAGS],
>   [gl_MANYWARN_NO_C++_WARNINGS])
> gl_MANYWARN_ALL_GCC_EXCEPT([GNULIB_WARN_CFLAGS],
>   [gl_MANYWARN_NO_C++_WARNINGS], [gl_MANYWARN_NO_GNULIB_WARNINGS])

Yes.  The current libidn configure.ac code 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
  nw="$nw -Wsuggest-attribute=pure" # Is it worth using attributes?
  nw="$nw -Wsuggest-attribute=const" # Is it worth using attributes?

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

  gl_WARN_ADD([-fdiagnostics-show-option])
fi

Re: unistd.h and getopt.h

[Simon Josefsson]

Bruno Haible  writes:

> Thanks for the analysis. To summarize:
> You have 2 gnulib-tool invocations in the scope of a single configure.ac file.
>   - One gnulib-tool invocation asks for 'unistd'.
>   - A second gnulib-tool invocation asks for 'unistd' and 'getopt'.
> The latter causes the m4 macros to set the GNULIB_UNISTD_H_GETOPT variable
> to 1. So code in the directory for the first gnulib-tool invocation tries
> to include , which does not exist.
>
> This patch fixes it at the gnulib level. (Tested on AIX 5.1.) You can remove
> the workaround that you added yesterday.

Thank you, I did this a few days ago, let's see how the next release
works.  (I thought I acknowledged this earlier, but I cannot find any
response from me in my mailboxes and your e-mail was left unread in one
of my folders...  perhaps I confuse this with some of your other fixes)

/Simon

Re: Removing -Wunsuffixed-float-constants, -Wdouble-promotion, -Wformat-zero-length

[Karl Berry]

1) The gcc manual doesn't list all warning flags.

2) The gcc --help=warnings doesn't list all warning flags, although it
   is a different subset than 1).

How about reporting bugs about these things to gcc?

3) Several of the warnings mentioned by gcc --help=warnings does not
   apply to C.

Could also say that a way to autoparse the warnings by language would
be helpful, and describe what we are trying to accomplish.

k

Re: [RFC] sethostname handling patch series

[Ben Walton]

Excerpts from Bruno Haible's message of Wed Nov 30 05:29:14 -0500 2011:

Hi Bruno,

> Your patches look pretty good. In parallel to the tweaking of the
> last details, it will be useful to assign the copyright for the
> change to the FSF. This is needed because Gnulib is central to many
> GNU packages.

Thanks for the great feedback.  I'm implementing the fixes for issues
you noted and testing them.  I'll submit them shortly (hopefully
tomorrow as I won't get to test on cygwin tonight).

The only point I'm not clear on is the note about using a fixed size
array allocation instead of the dynamic one.  Is it acceptable to use
HOST_NAME_MAX + 1 to achieve a size that is known to the compiler at
compile time or did you have some other non-malloc mechanism in mind?

Each patch should also update Changlog as well, correct?  I think that
the patch to add test-sethostname would be best as a fourth patch
since it can't be used until the unistd integration is done.  The
alternate would be to squash 2 and 3 of this series into a single
patch at which point the test suite could be updated as well.  What is
the preferred method?

> To go ahead with this, please fill in the form in the file
> gnulib/doc/Copyright/request-assign.future (if you foresee making
> more contributions to Gnulib) or
> gnulib/doc/Copyright/request-assign.changes (if this will be your
> only contribution) and send it to the FSF.

I've sent the request for this paperwork.

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