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 address@hidden@ || address@hidden@
> +_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 <http://en.wikipedia.org/wiki/Alfred_Herrhausen>