[PATCH] net_if: give more details about the bug being fixed

[Eric Blake]

Jasper Lievisse Adriaanse reported that NetBSD 5.2 also has
the same bug.  Also, when writing configure tests that use
 (such as for 'struct ifreq'), it is useful to know
what pre-requisite headers are needed within that test.

* doc/posix-headers/net_if.texi: Add clarification.

Signed-off-by: Eric Blake 
---

Pushing this, based on yesterday's thread.

 ChangeLog | 5 +
 doc/posix-headers/net_if.texi | 5 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index d9f6f89..282e060 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2012-09-06  Eric Blake  
+
+   net_if: give more details about the bug being fixed
+   * doc/posix-headers/net_if.texi: Add clarification.
+
 2012-09-05  Eric Blake  

net_if: new module
diff --git a/doc/posix-headers/net_if.texi b/doc/posix-headers/net_if.texi
index fa01b6e..a59669e 100644
--- a/doc/posix-headers/net_if.texi
+++ b/doc/posix-headers/net_if.texi
@@ -8,8 +8,9 @@ net/if.h
 Portability problems fixed by Gnulib:
 @itemize
 @item
-This header file is not self-contained on some platforms:
-FreeBSD 8.2, OpenBSD 5.1.
+This header file is not self-contained on some platforms (needing
+ to be included first):
+FreeBSD 8.2, OpenBSD 5.2.
 @end itemize

 Portability problems not fixed by Gnulib:
-- 
1.7.11.4

[PATCH] manywarnings: cater to more gcc infelicities

[Eric Blake]

Some versions of gcc (at least 4.2.1) noisily warn if
-Wuninitialized is given without -O; in isolation, configure
learns that the warning option is supported, but when later
compiling with -Werror, this makes builds fail.  If the
option will have no effect for the configured choice of
$CFLAGS, then it is not worth including in the set.

* m4/manywarnings.m4 (gl_MANYWARN_ALL_GCC): Add test for
-Wuninitialized without -O.

Signed-off-by: Eric Blake 
---

> Oh... and then there's the issue of the multiple WARN_CFLAGS
> variables.  coreutils has these three:
> 
>   GNULIB_TEST_WARN_CFLAGS
>   GNULIB_WARN_CFLAGS
>   WARN_CFLAGS
> 
> Hmm... with that realization, and knowing that those variable names
> are not standardized, the makefile-based approach looks even less
> interesting.

This patch worked for my particular test case; any objections?

 ChangeLog  |  6 ++
 m4/manywarnings.m4 | 32 +---
 2 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index d9f6f89..ec8bc3a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2012-09-06  Eric Blake  
+
+   manywarnings: cater to more gcc infelicities
+   * m4/manywarnings.m4 (gl_MANYWARN_ALL_GCC): Add test for
+   -Wuninitialized without -O.
+
 2012-09-05  Eric Blake  

net_if: new module
diff --git a/m4/manywarnings.m4 b/m4/manywarnings.m4
index 2760efb..f3cb23b 100644
--- a/m4/manywarnings.m4
+++ b/m4/manywarnings.m4
@@ -1,4 +1,4 @@
-# manywarnings.m4 serial 4
+# manywarnings.m4 serial 5
 dnl Copyright (C) 2008-2012 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -35,14 +35,12 @@ AC_DEFUN([gl_MANYWARN_COMPLEMENT],
 # make sure your gcc understands it.
 AC_DEFUN([gl_MANYWARN_ALL_GCC],
 [
-  dnl First, check if -Wno-missing-field-initializers is needed.
-  dnl -Wmissing-field-initializers is implied by -W, but that issues
-  dnl warnings with GCC version before 4.7, for the common idiom
-  dnl of initializing types on the stack to zero, using { 0, }
+  dnl First, check for some issues that only occur when combining multiple
+  dnl gcc warning categories.
   AC_REQUIRE([AC_PROG_CC])
   if test -n "$GCC"; then

-dnl First, check -W -Werror -Wno-missing-field-initializers is supported
+dnl Check if -W -Werror -Wno-missing-field-initializers is supported
 dnl with the current $CC $CFLAGS $CPPFLAGS.
 AC_MSG_CHECKING([whether -Wno-missing-field-initializers is supported])
 AC_CACHE_VAL([gl_cv_cc_nomfi_supported], [
@@ -77,8 +75,24 @@ AC_DEFUN([gl_MANYWARN_ALL_GCC],
   ])
   AC_MSG_RESULT([$gl_cv_cc_nomfi_needed])
 fi
+
+dnl Next, check if -Werror -Wuninitialized is useful with the
+dnl user's choice of $CFLAGS; some versions of gcc warn that it
+dnl has no effect if -O is not also used
+AC_MSG_CHECKING([whether -Wuninitialized is supported])
+AC_CACHE_VAL([gl_cv_cc_uninitialized_supported], [
+  gl_save_CFLAGS="$CFLAGS"
+  CFLAGS="$CFLAGS -Werror -Wuninitialized"
+  AC_COMPILE_IFELSE(
+[AC_LANG_PROGRAM([[]], [[]])],
+[gl_cv_cc_uninitialized_supported=yes],
+[gl_cv_cc_uninitialized_supported=no])
+  CFLAGS="$gl_save_CFLAGS"])
+AC_MSG_RESULT([$gl_cv_cc_uninitialized_supported])
+
   fi

+  # List all gcc warning categories.
   gl_manywarn_set=
   for gl_manywarn_item in \
 -W \
@@ -197,10 +211,14 @@ AC_DEFUN([gl_MANYWARN_ALL_GCC],
 gl_manywarn_set="$gl_manywarn_set $gl_manywarn_item"
   done

-  # Disable the missing-field-initializers warning if needed
+  # Disable specific options as needed.
   if test "$gl_cv_cc_nomfi_needed" = yes; then
 gl_manywarn_set="$gl_manywarn_set -Wno-missing-field-initializers"
   fi

+  if test "$gl_cv_cc_uninitialized_supported" = no; then
+gl_manywarn_set="$gl_manywarn_set -Wno-uninitialized"
+  fi
+
   $1=$gl_manywarn_set
 ])
-- 
1.7.11.4

Re: broken OpenBSD [was: [libvirt] [PATCH] Include some extra headers needed for OpenBSD.]

[Jasper Lievisse Adriaanse]

On Tue, Sep 04, 2012 at 11:37:30AM -0600, Eric Blake wrote:
> On 09/04/2012 11:23 AM, Jasper Lievisse Adriaanse wrote:
> > On Tue, Sep 04, 2012 at 11:08:30AM -0600, Eric Blake wrote:
> >> [adding gnulib]
> >>
> >> Ouch.  The POSIX definition of  doesn't include any interface
> >> that needs to use struct sockaddr.  Which OpenBSD extension function is
> >> triggering this warning? According to POSIX, this .c file should compile:
> >>
> >> #define _POSIX_C_SOURCE 200809L
> >> #include 
> >> #include 
> >> struct if_nameindex i;
> >>
> 
> > That snippet of example code does not compile on OpenBSD:
> > 
> > In file included from foo.c:2:
> > /usr/include/net/if.h:112: error: expected specifier-qualifier-list before 
> > 'u_int'
> 
> > 
> > Could you please reference where POSIX states it should, so this can 
> > hopefully
> > get fixed in OpenBSD.
> 
> http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/net_if.h.html#tag_13_30
> doesn't mention any prerequisite headers for .  Furthermore,
> looking at each of the functions in that header, none of them call out a
> prerequisite:
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/if_freenameindex.html#
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/if_indextoname.html#
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/if_nameindex.html#
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/if_nametoindex.html#
> 
> If a header cannot be used in isolation, then POSIX would call out
> multiple headers.  For comparison, here's a case where POSIX 2008 _does_
> call out multiple headers:
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/creat.html
> 
> creat() cannot use the S_IRUSR and other constants for its mode_t
> argument unless you also include , since  was not
> required to be self-contained on that front (there's talk underway about
> getting that fixed in the next version of POSIX, but that's another
> story... http://austingroupbugs.net/view.php?id=591)
> 
> -- 
> Eric Blake   ebl...@redhat.com+1-919-301-3266
> Libvirt virtualization library http://libvirt.org

A diff for the OpenBSD header file has been created and is currently being
tested. So this should be fixed pretty soon (and in time for OpenBSD 5.3).

-- 
Cheers,
Jasper

"Stay Hungry. Stay Foolish"

Re: GNU sed version 4.2.1: on OS X, C locale gets aliased to UTF-8

[Paul Eggert]

On 09/06/2012 11:03 AM, Max Horn wrote:

> I proposed the exact same patch you proposed back in January.

Ah, sorry, I was confused.  Again.  Ouch.

Please let me try to summarize the situation to make sure that I
understand it.  We're talking about two different patches, either of
which you say will solve the problem.  The January patch is:

--- a/lib/localcharset.c
+++ b/lib/localcharset.c
@@ -262,6 +262,7 @@ get_charset_aliases (void)
"ISO8859-9" "\0" "ISO-8859-9" "\0"
"ISO8859-13" "\0" "ISO-8859-13" "\0"
"ISO8859-15" "\0" "ISO-8859-15" "\0"
+   "US-ASCII" "\0" "ASCII" "\0"
"KOI8-R" "\0" "KOI8-R" "\0"
"KOI8-U" "\0" "KOI8-U" "\0"
"CP866" "\0" "CP866" "\0"

and the July patch is:

--- a/lib/localcharset.c
+++ b/lib/localcharset.c
@@ -542,5 +542,12 @@ locale_charset (void)
  if (codeset[0] == '\0')
codeset = "ASCII";

+#ifdef DARWIN7
+  /* MacOS X sets MB_CUR_MAX to 1 when LC_ALL=C, and "UTF-8"
+ (the default codeset) does not work when MB_CUR_MAX is 1.  */
+  if (strcmp (codeset, "UTF-8") == 0 && MB_CUR_MAX <= 1)
+codeset = "ASCII";
+#endif
+
  return codeset;
}

If I understand things correctly, you're saying the January patch is
more logical, but that either will work, and so you'd settle for
either.

Bruno has objected to the January patch, but has not commented on the
July patch.  You've responded to Bruno's January objections, and he
hasn't had time to follow up.

> I (and everybody who has installed GNU sed via Fink) has been using
> the proposed fix for months, and I can assure you that i18n is
> working fine.

Thanks for mentioning that.  By "the proposed fix" I assume you mean
the January patch.

I'd greatly appreciate Bruno's comments, if he has the time.  If not,
I'm thinking of pushing the July patch, as a temporary fix until Bruno
does find the time, since he hasn't objected.

Re: GNU sed version 4.2.1: on OS X, C locale gets aliased to UTF-8

[Max Horn]

Hi again!


On 06.09.2012, at 11:08, Paul Eggert wrote:

> Alas, I don't use OS X, or Mac OS X, or whatever it's called
> these days, so I can't easily test any fix, and I'd like some
> discussion if possible.

Sure. 

> 
> In January Bruno wrote that my proposed patch
> 
> would break gettext-based i18n of GNU programs on OS X; see
> .

And I refuted those claims in 



> 
> In July Max Horn proposed a different patch, which he said is more logical
> .

This is incorrect -- indeed, if you read the link you just gave carefully, 
you'll see that I proposed the exact same patch you proposed back in January. 
Moreover, in the email I sent last night, I once more proposed that patch

The different patch you are talking about was proposed by yourself :-), see 
. And I 
later confirmed that this also fixes the issue in my reply to your email. 
However, I don't really see why that patch is better or more logical... Rather, 
I find the original trivial patch much more logical :-).


> Max's patch would set the codeset to be ASCII if
> the codeset is UTF-8 but MB_CUR_MAX is 1.  This seems like
> it should do the trick, and shouldn't break any system
> that isn't already broken.
> 
> Nobody has replied to Max's proposal, as far as I know.
> Are there any objections if I install his July patch?

Well, I still would prefer the original patch. There were some concerns about 
it, but I tried to address and refute every single one. So far replied to that 
either... In particular, I think Bruno's analysys that this would break 
gettext-based i18n of GNU programs on OS X is incorrect, see once more 
.  In 
addition to my explanations there, let me add that I (and everybody who has 
installed GNU sed via Fink) has been using the proposed fix for months, and I 
can assure you that i18n is working fine.

So, I still would very much prefer the original patch, which simply maps 
US-ASCII to ASCII. But in the end, I don't care so much how gnulib fixes it, as 
long as it gets fixed in some way... :)


Cheers,
Max

Re: GNU sed version 4.2.1: on OS X, C locale gets aliased to UTF-8

[Jim Meyering]

Paul Eggert wrote:
> Alas, I don't use OS X, or Mac OS X, or whatever it's called
> these days, so I can't easily test any fix, and I'd like some
> discussion if possible.
>
> In January Bruno wrote that my proposed patch
> 
> would break gettext-based i18n of GNU programs on OS X; see
> .
>
> In July Max Horn proposed a different patch, which he said is more logical
> .
> Max's patch would set the codeset to be ASCII if
> the codeset is UTF-8 but MB_CUR_MAX is 1.  This seems like
> it should do the trick, and shouldn't break any system
> that isn't already broken.
>
> Nobody has replied to Max's proposal, as far as I know.
> Are there any objections if I install his July patch?

Thanks for stepping up, Paul.
I like that solution, too, but we should give Bruno one more chance to respond.

Re: GNU sed version 4.2.1: on OS X, C locale gets aliased to UTF-8

[Paul Eggert]

Alas, I don't use OS X, or Mac OS X, or whatever it's called
these days, so I can't easily test any fix, and I'd like some
discussion if possible.

In January Bruno wrote that my proposed patch

would break gettext-based i18n of GNU programs on OS X; see
.

In July Max Horn proposed a different patch, which he said is more logical
.
Max's patch would set the codeset to be ASCII if
the codeset is UTF-8 but MB_CUR_MAX is 1.  This seems like
it should do the trick, and shouldn't break any system
that isn't already broken.

Nobody has replied to Max's proposal, as far as I know.
Are there any objections if I install his July patch?

Re: poll() emulation in git

[Paolo Bonzini]

Il 06/09/2012 16:44, Joachim Schmitz ha scritto:
>> > Yes, it's an usleep(autocorrect * 10) basically (poll takes
>> > milliseconds, not micro).
> OK, it is _supposed_ to do this usleep(), but is does not, as poll() returns 
> early with EFAULT in this case:
>   /* EFAULT is not necessary to implement, but let's do it in the
>  simplest case. */
>   if (!pfd)
> {
>   errno = EFAULT;
>   return -1;
> }
> 
> poll() is doing this before calling select(), so won't sleep.
> So there's a bug in {gnulib|git}'s poll(), right?
> 

Yes, it should be "if (!pfd && nfd)".

Paolo

Re: another manywarnings failure

[Jim Meyering]

Jim Meyering wrote:
...
> Yes, it looks like if we're using both of those, we'll have to
> add code to reject -Wuninitialized (maybe with a warning?) when
> configuring without -O.
...
> [here's simple PoC, i.e., it doesn't know about -O0,
>  but you get the idea ]
>
> W = -Wuninitialized -Werror
> CFLAGS = -O
>
> ifneq (,$(findstring -Wuninitialized,$(W)))
>   ifneq (,$(findstring -Werror,$(W)))
> ifneq (,$(findstring -O,$(CFLAGS)))
>   W := $(subst -Wuninitialized,,$(W))
> endif
>   endif
> endif
>
> all:
>   @echo $(W)

Not quite.  The innermost test was reversed: s/ifneq/ifeq/:

# With -O in $(CFLAGS) and both -Wuninitialized and -Werror in $(W),
# remove -Wuninitialized from $(W).
ifneq (,$(findstring -Wuninitialized,$(W)))
  ifneq (,$(findstring -Werror,$(W)))
ifeq (,$(findstring -O,$(CFLAGS)))
  W := $(subst -Wuninitialized,,$(W))
endif
  endif
endif

Also, cfg.mk is project-specific.  This is general enough that
a shared file like maint.mk would be a better home for it.
On the other hand, if you want to change the .m4 file, that
would probably be even better, from a locality standpoint.
Oh... and then there's the issue of the multiple WARN_CFLAGS
variables.  coreutils has these three:

  GNULIB_TEST_WARN_CFLAGS
  GNULIB_WARN_CFLAGS
  WARN_CFLAGS

Hmm... with that realization, and knowing that those variable names
are not standardized, the makefile-based approach looks even less
interesting.

RE: poll() emulation in git

[Joachim Schmitz]

> From: Paolo Bonzini [mailto:paolo.bonz...@gmail.com] On Behalf Of Paolo 
> Bonzini
> Sent: Thursday, September 06, 2012 4:32 PM
> To: Joachim Schmitz
> Cc: g...@vger.kernel.org; 'Junio C Hamano'; 'Erik Faye-Lund'; 
> bug-gnulib@gnu.org; rsbec...@nexbridge.com
> Subject: Re: poll() emulation in git
> 
> Il 06/09/2012 16:02, Joachim Schmitz ha scritto:
> >
> > But is there something that could be done to make git work even without 
> > poll()?
> > It is used in 5 places:
> >
> > $ grep -n poll\( *.c */*.c
> > credential-cache--daemon.c:175: if (poll(&pfd, 1, 1000 * wakeup) < 0) {
> > daemon.c:1018:  if (poll(pfd, socklist->nr, -1) < 0) {
> > help.c:361: poll(NULL, 0, autocorrect * 100);
> > upload-pack.c:232:  if (poll(pfd, pollsize, -1) < 0) {
> > builtin/upload-archive.c:125:   if (poll(pfd, 2, -1) < 0) {
> >
> > Don't quite understand why in help.c it has that NULL, which should always 
> > result in an EFAULT and other than that basically is
a
> > NOP (at least in the poll() emulation)? Seems a usleep(autocorrect * 100) 
> > is meant to happen here instead?
> > So I think here a poll() isn't needed at all. But also the 'broken' one 
> > shouldn't harm too much.
> 
> Yes, it's an usleep(autocorrect * 10) basically (poll takes
> milliseconds, not micro).

OK, it is _supposed_ to do this usleep(), but is does not, as poll() returns 
early with EFAULT in this case:
  /* EFAULT is not necessary to implement, but let's do it in the
 simplest case. */
  if (!pfd)
{
  errno = EFAULT;
  return -1;
}

poll() is doing this before calling select(), so won't sleep.
So there's a bug in {gnulib|git}'s poll(), right?

> > ...
> > # else
> >   char data[64];
> >   r = recv (fd, data, sizeof (data), MSG_PEEK);
> >   socket_errno = (r < 0) ? errno : 0;
> > # endif
> >   if (r == 0)
> > happened |= POLLHUP;
> >
> >   /* If the event happened on an unconnected server socket,
> >  that's fine. */
> >   else if (r > 0 || ( /* (r == -1) && */ socket_errno == ENOTCONN))
> > happened |= (POLLIN | POLLRDNORM) & sought;
> >
> >   /* Distinguish hung-up sockets from other errors.  */
> >   else if (socket_errno == ESHUTDOWN || socket_errno == ECONNRESET
> >|| socket_errno == ECONNABORTED || socket_errno == ENETRESET)
> > happened |= POLLHUP;
> >
> > #ifdef __TANDEM /* as we can't recv(...,MSG_PEEK) on a non-socket */
> >   else if (socket_errno == ENOTSOCK)
> > happened |= (POLLIN | POLLRDNORM) & sought;
> > #endif
> >   else
> > happened |= POLLERR;
> > }
> > ...
> >
> > We won't detect POLLHUP that way I think. However it seems to work, we've 
> > been able to clone, push, pull, branch that way with
> > NonStop being the (ssh-)server, something that didn't work at all without 
> > that hack (and yes, I believe it is just that).
> > Someone in for a cleaner way of managing this?
> 
> I suppose it works to always handle ENOTSOCK that way, even on
> non-__TANDEM systems.

So you think this is a clean way of dealing with it?

Bye, Jojo

Re: poll() emulation in git

[Paolo Bonzini]

Il 06/09/2012 16:02, Joachim Schmitz ha scritto:
> 
> But is there something that could be done to make git work even without 
> poll()?
> It is used in 5 places:
> 
> $ grep -n poll\( *.c */*.c
> credential-cache--daemon.c:175: if (poll(&pfd, 1, 1000 * wakeup) < 0) {
> daemon.c:1018:  if (poll(pfd, socklist->nr, -1) < 0) {
> help.c:361: poll(NULL, 0, autocorrect * 100);
> upload-pack.c:232:  if (poll(pfd, pollsize, -1) < 0) {
> builtin/upload-archive.c:125:   if (poll(pfd, 2, -1) < 0) {
> 
> Don't quite understand why in help.c it has that NULL, which should always 
> result in an EFAULT and other than that basically is a
> NOP (at least in the poll() emulation)? Seems a usleep(autocorrect * 100) is 
> meant to happen here instead?
> So I think here a poll() isn't needed at all. But also the 'broken' one 
> shouldn't harm too much.

Yes, it's an usleep(autocorrect * 10) basically (poll takes
milliseconds, not micro).

> ...
> # else
>   char data[64];
>   r = recv (fd, data, sizeof (data), MSG_PEEK);
>   socket_errno = (r < 0) ? errno : 0;
> # endif
>   if (r == 0)
> happened |= POLLHUP;
> 
>   /* If the event happened on an unconnected server socket,
>  that's fine. */
>   else if (r > 0 || ( /* (r == -1) && */ socket_errno == ENOTCONN))
> happened |= (POLLIN | POLLRDNORM) & sought;
> 
>   /* Distinguish hung-up sockets from other errors.  */
>   else if (socket_errno == ESHUTDOWN || socket_errno == ECONNRESET
>|| socket_errno == ECONNABORTED || socket_errno == ENETRESET)
> happened |= POLLHUP;
> 
> #ifdef __TANDEM /* as we can't recv(...,MSG_PEEK) on a non-socket */
>   else if (socket_errno == ENOTSOCK)
> happened |= (POLLIN | POLLRDNORM) & sought;
> #endif
>   else
> happened |= POLLERR;
> }
> ...
> 
> We won't detect POLLHUP that way I think. However it seems to work, we've 
> been able to clone, push, pull, branch that way with
> NonStop being the (ssh-)server, something that didn't work at all without 
> that hack (and yes, I believe it is just that).
> Someone in for a cleaner way of managing this?

I suppose it works to always handle ENOTSOCK that way, even on
non-__TANDEM systems.

Paolo

RE: poll() emulation in git

[Joachim Schmitz]

> From: Paolo Bonzini [mailto:paolo.bonz...@gmail.com] On Behalf Of Paolo 
> Bonzini
> Sent: Wednesday, September 05, 2012 5:26 PM
> To: Joachim Schmitz
> Cc: 'Junio C Hamano'; g...@vger.kernel.org; 'Erik Faye-Lund'; 
> bug-gnulib@gnu.org
> Subject: Re: poll() emulation in git
> 
> Il 05/09/2012 15:36, Joachim Schmitz ha scritto:
> >>> > > Does your system have a working FIONREAD ioctl for pipes?
> >> >
> >> > It does have FIONREAD ioctl. Whether it works properly is to be 
> >> > determined...
> >> > I'll test if you could show me how?
> > Oh, now I see what you aimed at, but no, that Mac OS X method doesn't work 
> > for me, I tried (at least I think I did).
> >
> > And  has
> > /*
> >  * Normal IOCTL's supported by the socket interface
> >  */
> > #define FIONREAD_IOR(0, 8, _ioctl_int)   /* Num of bytes to 
> > read */
> > #define FIONBIO _IOW(0, 9, _ioctl_int)   /* Non-blocking I/O
> >  */
> >
> > So these seem to be supported on sockets only, I guess.
> > And indeed the man pages for ioctl confirms:
> >
> >   Valid values for the request parameter for AF_INET or
> >   AF_INET6 sockets are:
> >
> >
> >   FIONREAD  Gets the number of bytes available for reading and
> > stores it at the int pointed at by arg.
> >
> >
> > So not even AF_UNIX sockets, not to mention pipes...
> 
> So there's no way you can support POLLHUP.  Your system is quite
> crippled. :(

Unfortunatly.

But is there something that could be done to make git work even without poll()?
It is used in 5 places:

$ grep -n poll\( *.c */*.c
credential-cache--daemon.c:175: if (poll(&pfd, 1, 1000 * wakeup) < 0) {
daemon.c:1018:  if (poll(pfd, socklist->nr, -1) < 0) {
help.c:361: poll(NULL, 0, autocorrect * 100);
upload-pack.c:232:  if (poll(pfd, pollsize, -1) < 0) {
builtin/upload-archive.c:125:   if (poll(pfd, 2, -1) < 0) {

Don't quite understand why in help.c it has that NULL, which should always 
result in an EFAULT and other than that basically is a
NOP (at least in the poll() emulation)? Seems a usleep(autocorrect * 100) is 
meant to happen here instead?
So I think here a poll() isn't needed at all. But also the 'broken' one 
shouldn't harm too much.

In daemon.c it seems to be all sockets it polls on, so it should work on 
NonStop.
Same in credential-cache--daemon.c

Remains upload-pack.c and builtin/upload-archive.c
In both start_command() gathers the FDs to poll() on and that indeed works on 
pipes -> problem on NonStop!

Seeing that in those cases xread() takes care of EAGAIN, I've now used 'brute 
force' in poll.c:

...
# else
  char data[64];
  r = recv (fd, data, sizeof (data), MSG_PEEK);
  socket_errno = (r < 0) ? errno : 0;
# endif
  if (r == 0)
happened |= POLLHUP;

  /* If the event happened on an unconnected server socket,
 that's fine. */
  else if (r > 0 || ( /* (r == -1) && */ socket_errno == ENOTCONN))
happened |= (POLLIN | POLLRDNORM) & sought;

  /* Distinguish hung-up sockets from other errors.  */
  else if (socket_errno == ESHUTDOWN || socket_errno == ECONNRESET
   || socket_errno == ECONNABORTED || socket_errno == ENETRESET)
happened |= POLLHUP;

#ifdef __TANDEM /* as we can't recv(...,MSG_PEEK) on a non-socket */
  else if (socket_errno == ENOTSOCK)
happened |= (POLLIN | POLLRDNORM) & sought;
#endif
  else
happened |= POLLERR;
}
...

We won't detect POLLHUP that way I think. However it seems to work, we've been 
able to clone, push, pull, branch that way with
NonStop being the (ssh-)server, something that didn't work at all without that 
hack (and yes, I believe it is just that).
Someone in for a cleaner way of managing this?

Bye, Jojo

Re: another manywarnings failure

[Jim Meyering]

Eric Blake wrote:
> On 09/06/2012 06:48 AM, Jim Meyering wrote:
>>
>> Actually, with GNU make, this is something that we could probably
>> adjust at make-time.
>
> Only if you use Automake-NG, and can assume GNU make.

IMHO, this work-around doesn't have to do anything for those using
non-GNU make, as long as it doesn't break their build.

The suggested snippet could go in a file that is included only when
using GNU make, like cfg.mk.  People who don't use GNU make probably
don't enable all of those warning options, either.

Re: another manywarnings failure

[Eric Blake]

On 09/06/2012 06:48 AM, Jim Meyering wrote:
> 
> Actually, with GNU make, this is something that we could probably
> adjust at make-time.

Only if you use Automake-NG, and can assume GNU make.

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



signature.asc
Description: OpenPGP digital signature

Re: another manywarnings failure

[Jim Meyering]

Eric Blake wrote:

> On 09/06/2012 12:07 AM, Jim Meyering wrote:
>> Eric Blake wrote:
>>
>>> I compiled on FreeBSD 8.2 (gcc 4.2.1), and when I updated to the latest
>>> gnulib, I started to see the following in config.log when running
>>> ./configure CFLAGS=-g:
>>>
>
>> When you configure with one set of warning options and later
>> build with another, as with your "make CFLAGS=-g", you're
>> short-circuiting the build tests.  When I do that, I
>> simply turn off -Werror and ignore the warnings:
>>
>> make CFLAGS=-g WERROR_CFLAGS=
>
> But note that I did:
>
> ./configure CFLAGS=-g

Ah... I didn't read carefully enough.

> at the outset.  Yes, I'm aware that _if_ you override CFLAGS at make
> time, you may have to also disable warnings at that time as well.  But
> here, I'm talking about the case where I configured optimization to be
> off without needing to override CFLAGS at make time; that is, in a
> situation where we CAN run a configure test to see whether the warning
> will even be effective.
>
>>
>> Of course, you could simply rerun configure with CFLAGS=-g,
>
> Which is what I did...
>
>> and it would test each -W option individually...
>
> except that each -W option individually succeeds.  -Wuninitialized only
> warns, not errors out, on this particular gcc 4.2.1 build.  You _have_
> to test the combination of '-Wuninitialized -Werror' but no -O, in order
> to hit the failure.  But I'm not sure how best to modify manywarnings.m4
> to do that testing.  Then again, we've already special-cased
> -Wno-missing-field-initializer to work around gcc infelicities, so I
> guess I could enhance the module by adding another test along those lines.

Yes, it looks like if we're using both of those, we'll have to
add code to reject -Wuninitialized (maybe with a warning?) when
configuring without -O.

Actually, with GNU make, this is something that we could probably
adjust at make-time.  And since we're typically enabling warnings
only when we have reasonable development tools, that might be even
better (simpler).  E.g.,

[here's simple PoC, i.e., it doesn't know about -O0,
 but you get the idea ]

W = -Wuninitialized -Werror
CFLAGS = -O

ifneq (,$(findstring -Wuninitialized,$(W)))
  ifneq (,$(findstring -Werror,$(W)))
ifneq (,$(findstring -O,$(CFLAGS)))
  W := $(subst -Wuninitialized,,$(W))
endif
  endif
endif

all:
@echo $(W)

Re: [PATCH] net_if: new module

[Eric Blake]

On 09/06/2012 01:06 AM, Jasper Lievisse Adriaanse wrote:
> On Wed, Sep 05, 2012 at 03:59:10PM -0600, Eric Blake wrote:
>> OpenBSD's  is not self-contained; this should compile:
>>
>> | #define _POSIX_C_SOURCE 200809L
>> | #include 
>> | struct if_nameindex i;
>>
>> but fails

>> * doc/posix-headers/net_if.texi (net/if.h): Document it.
>> * MODULES.html.sh (lacking POSIX:2008): Likewise.
>> * tests/test-net_if.c: Make function checks conditional.
>> Reported by Jasper Lievisse Adriaanse .
>>
>> Signed-off-by: Eric Blake 
> 
> Although the entry in net_if.texi is correct for OpenBSD, it may be ammended
> to include 5.2 too? As that is the version I'm testing with (OpenBSD 
> -current).

That would indeed be possible, but in that case, it would probably be
nice scrubbing ALL of doc/*/*.texi for each listing of an OpenBSD 5.1
bug, to see which bugs have been carried forward into 5.2 and which have
been fixed in the meantime.  I can push the one-liner for net_if.texi
now, but without instant access to an OpenBSD 5.2 machine and a good
chunk of testing time, it would be easier if you could help with that
effort of updating documentation.  (I am considering installing OpenBSD
in a VM, but that will take me a while...)

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



signature.asc
Description: OpenPGP digital signature

Re: another manywarnings failure

[Eric Blake]

On 09/06/2012 12:07 AM, Jim Meyering wrote:
> Eric Blake wrote:
> 
>> I compiled on FreeBSD 8.2 (gcc 4.2.1), and when I updated to the latest
>> gnulib, I started to see the following in config.log when running
>> ./configure CFLAGS=-g:
>>

> When you configure with one set of warning options and later
> build with another, as with your "make CFLAGS=-g", you're
> short-circuiting the build tests.  When I do that, I
> simply turn off -Werror and ignore the warnings:
> 
> make CFLAGS=-g WERROR_CFLAGS=

But note that I did:

./configure CFLAGS=-g

at the outset.  Yes, I'm aware that _if_ you override CFLAGS at make
time, you may have to also disable warnings at that time as well.  But
here, I'm talking about the case where I configured optimization to be
off without needing to override CFLAGS at make time; that is, in a
situation where we CAN run a configure test to see whether the warning
will even be effective.

> 
> Of course, you could simply rerun configure with CFLAGS=-g,

Which is what I did...

> and it would test each -W option individually...

except that each -W option individually succeeds.  -Wuninitialized only
warns, not errors out, on this particular gcc 4.2.1 build.  You _have_
to test the combination of '-Wuninitialized -Werror' but no -O, in order
to hit the failure.  But I'm not sure how best to modify manywarnings.m4
to do that testing.  Then again, we've already special-cased
-Wno-missing-field-initializer to work around gcc infelicities, so I
guess I could enhance the module by adding another test along those lines.

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



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] net_if: new module

[Jasper Lievisse Adriaanse]

On Wed, Sep 05, 2012 at 03:59:10PM -0600, Eric Blake wrote:
> OpenBSD's  is not self-contained; this should compile:
> 
> | #define _POSIX_C_SOURCE 200809L
> | #include 
> | struct if_nameindex i;
> 
> but fails with:
> 
> In file included from foo.c:2:
> /usr/include/net/if.h:112: error: expected specifier-qualifier-list before 
> 'u_int'
> ...
> /usr/include/net/if.h:674: error: field 'dstaddr' has incomplete type
> In file included from /usr/include/net/if.h:691,
>  from foo.c:2:
> /usr/include/net/if_arp.h:79: error: field 'arp_pa' has incomplete type
> /usr/include/net/if_arp.h:80: error: field 'arp_ha' has incomplete type
> *** Error code 1
> 
> We already had a test program for systems with the if_* functions
> (including OpenBSD); but in adding this module, I had to enhance
> that test to also work on platforms like mingw where we are now
> providing a new header.
> 
> * modules/net_if: New module, borrowing ideas from netinet_in.
> * m4/net_if_h.m4: New file.
> * lib/net_if.in.h: Likewise.
> * doc/posix-headers/net_if.texi (net/if.h): Document it.
> * MODULES.html.sh (lacking POSIX:2008): Likewise.
> * tests/test-net_if.c: Make function checks conditional.
> Reported by Jasper Lievisse Adriaanse .
> 
> Signed-off-by: Eric Blake 

Although the entry in net_if.texi is correct for OpenBSD, it may be ammended
to include 5.2 too? As that is the version I'm testing with (OpenBSD -current).

> ---
>  ChangeLog | 11 +
>  MODULES.html.sh   |  1 +
>  doc/posix-headers/net_if.texi |  5 +++-
>  lib/net_if.in.h   | 53 
> +++
>  m4/net_if_h.m4| 31 +
>  modules/net_if| 49 +++
>  tests/test-net_if.c   | 12 --
>  7 files changed, 159 insertions(+), 3 deletions(-)
>  create mode 100644 lib/net_if.in.h
>  create mode 100644 m4/net_if_h.m4
>  create mode 100644 modules/net_if
> 
> diff --git a/ChangeLog b/ChangeLog
> index dea1f86..d9f6f89 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,14 @@
> +2012-09-05  Eric Blake  
> +
> + net_if: new module
> + * modules/net_if: New module, borrowing ideas from netinet_in.
> + * m4/net_if_h.m4: New file.
> + * lib/net_if.in.h: Likewise.
> + * doc/posix-headers/net_if.texi (net/if.h): Document it.
> + * MODULES.html.sh (lacking POSIX:2008): Likewise.
> + * tests/test-net_if.c: Make function checks conditional.
> + Reported by Jasper Lievisse Adriaanse .
> +
>  2012-09-05  Mats Erik Andersson(tiny change)
> 
>   readutmp: fix non-portable UT_PID use
> diff --git a/MODULES.html.sh b/MODULES.html.sh
> index 8ced611..0e34531 100755
> --- a/MODULES.html.sh
> +++ b/MODULES.html.sh
> @@ -2430,6 +2430,7 @@ func_all_modules ()
>func_module mkfifo
>func_module mknod
>func_module mkstemp
> +  func_module net_if
>func_module netdb
>func_module netinet_in
>func_module nl_langinfo
> diff --git a/doc/posix-headers/net_if.texi b/doc/posix-headers/net_if.texi
> index 38a52f4..fa01b6e 100644
> --- a/doc/posix-headers/net_if.texi
> +++ b/doc/posix-headers/net_if.texi
> @@ -3,10 +3,13 @@ net/if.h
> 
>  POSIX specification:@* 
> @url{http://www.opengroup.org/onlinepubs/9699919799/basedefs/net_if.h.html}
> 
> -Gnulib module: ---
> +Gnulib module: net_if
> 
>  Portability problems fixed by Gnulib:
>  @itemize
> +@item
> +This header file is not self-contained on some platforms:
> +FreeBSD 8.2, OpenBSD 5.1.
>  @end itemize
> 
>  Portability problems not fixed by Gnulib:
> diff --git a/lib/net_if.in.h b/lib/net_if.in.h
> new file mode 100644
> index 000..24f9f83
> --- /dev/null
> +++ b/lib/net_if.in.h
> @@ -0,0 +1,53 @@
> +/* Substitute for .
> +   Copyright (C) 2007-2012 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 2, or (at your option)
> +   any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program; if not, see .  */
> +
> +#ifndef _@GUARD_PREFIX@_NET_IF_H
> +
> +#if __GNUC__ >= 3
> +@PRAGMA_SYSTEM_HEADER@
> +#endif
> +@PRAGMA_COLUMNS@
> +
> +#if @HAVE_NET_IF_H@
> +
> +/* On some platforms,  assumes prior inclusion of
> +   .  */
> +# include 
> +
> +/* The include_next requires a split double-inclusion guard.  */
> +# @INCLUDE_NEXT@ @NEXT_NET_IF_H@
> +
> +#endif
> +
> +#ifndef _@GUARD_PREFIX@_NET_IF_H
> +#define _@GUARD_PREFIX@_NET_IF_H
> +
> +#if !@HAVE_NET_IF_H@
> +
> +/*