Re: unistr use of undefined macros

[Ludovic Courtès]

Hi Eric,

Eric Blake  writes:

> On 12/07/2010 02:24 PM, Ludovic Courtès wrote:
>> Hello,
>> 
>> Commit c98cb289 changed #ifdefs in unistr.in.h to #ifs.  As a
>> side-effect, this prevents compilation of users of this file, such as
>> Guile, with ‘-Wundef’:
>
> Unfortunately, that points to a bug in guile.
>
> Gnulib is explicitly documented as being incompatible with -Wundef; and
> while you may choose to use it for the rest of your project, you should
> not use it for gnulib files.

That’s what Guile is doing already.

The error here is when *using*  from libguile, which is
compiled with -Wundef.

> See for example how coreutils sets up two distinct CFLAGS, a stricter
> one for coreutils files in src/ [1] and a looser one for gnulib files in
> lib/ [2]:

I just changed  so
that Coreutils is built with ‘--enable-gcc-warnings’, and it builds
fine.

However:

--8<---cut here---start->8---
$ grep -r 'unistr.[ch]' . ; echo $?
1
--8<---cut here---end--->8---

(This is in a pristine checkout, without Gnulib in here.)

So no wonder it doesn’t hit that problem.

Thanks,
Ludo’.

[PATCH] pipe-posix: new module

[Eric Blake]

* modules/pipe-posix: New file.
* m4/unistd_h.m4 (gl_UNISTD_H_DEFAULTS): Set default.
(gl_UNISTD_H): Check for declaration.
* modules/unistd (Makefile.am): Substitute it.
* lib/unistd.in.h (pipe): Provide it for mingw.
* doc/posix-functions/pipe.texi (pipe): Update documentation.
* MODULES.html.sh (File descriptor based Input/Output): Likewise.

Signed-off-by: Eric Blake 
---

I'm pushing this.  The module could perhaps be improved (for example,
since it is just a #define rather than a replacement function, it
fails the POSIX aspect of being able to pass a function pointer to
pipe around), and testing I/O through the pipe is impossible without
either threading or spawning a child process, all of which seem rather
complex given how simple the module itself is.  The module name 'pipe'
is already claimed for an unrelated application-level wrapper around
both pipe(2) and the notion of a child process.

 ChangeLog |   11 +++
 MODULES.html.sh   |1 +
 doc/posix-functions/pipe.texi |8 
 lib/unistd.in.h   |   24 ++--
 m4/unistd_h.m4|3 ++-
 modules/pipe-posix|   22 ++
 modules/unistd|1 +
 7 files changed, 63 insertions(+), 7 deletions(-)
 create mode 100644 modules/pipe-posix

diff --git a/ChangeLog b/ChangeLog
index b1ad470..29849ba 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2010-12-10  Eric Blake  
+
+   pipe-posix: new module
+   * modules/pipe-posix: New file.
+   * m4/unistd_h.m4 (gl_UNISTD_H_DEFAULTS): Set default.
+   (gl_UNISTD_H): Check for declaration.
+   * modules/unistd (Makefile.am): Substitute it.
+   * lib/unistd.in.h (pipe): Provide it for mingw.
+   * doc/posix-functions/pipe.texi (pipe): Update documentation.
+   * MODULES.html.sh (File descriptor based Input/Output): Likewise.
+
 2010-12-07  Bruno Haible  

unistr/u8-strcmp: Avoid collision with libc function on Solaris 11.
diff --git a/MODULES.html.sh b/MODULES.html.sh
index 983953c..456497f 100755
--- a/MODULES.html.sh
+++ b/MODULES.html.sh
@@ -2649,6 +2649,7 @@ func_all_modules ()
   func_module full-write
   func_module binary-io
   func_module isapipe
+  func_module pipe-posix
   func_module pipe2
   func_module pipe2-safer
   func_end_table
diff --git a/doc/posix-functions/pipe.texi b/doc/posix-functions/pipe.texi
index b8cc508..420e400 100644
--- a/doc/posix-functions/pipe.texi
+++ b/doc/posix-functions/pipe.texi
@@ -4,15 +4,15 @@ pipe

 POSIX specification:@* 
@url{http://www.opengroup.org/onlinepubs/9699919799/functions/pipe.html}

-Gnulib module: ---
+Gnulib module: pipe-posix

 Portability problems fixed by Gnulib:
 @itemize
+...@item
+This function is missing on some platforms:
+mingw.
 @end itemize

 Portability problems not fixed by Gnulib:
 @itemize
-...@item
-This function is missing on some platforms:
-mingw.
 @end itemize
diff --git a/lib/unistd.in.h b/lib/unistd.in.h
index 4834e37..571afa1 100644
--- a/lib/unistd.in.h
+++ b/lib/unistd.in.h
@@ -82,10 +82,14 @@
 # include 
 #endif

-/* mingw declares getcwd in , not in .  */
-#if ((@GNULIB_GETCWD@ || defined GNULIB_POSIXCHECK) \
+/* mingw declares getcwd in , not in .  It also provides
+   _pipe in , but that requires _O_BINARY from .  */
+#if ((@GNULIB_GETCWD@ || @GNULIB_PIPE@ || defined GNULIB_POSIXCHECK) \
  && ((defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__))
 # include 
+# if @GNULIB_PIPE@
+#  include 
+# endif
 #endif

 /* AIX and OSF/1 5.1 declare getdomainname in , not in .  */
@@ -972,6 +976,22 @@ _GL_WARN_ON_USE (lseek, "lseek does not fail with ESPIPE 
on pipes on some "
 #endif


+#if @GNULIB_PIPE@
+/* Create a pipe, defaulting to O_BINARY mode.
+   Store the read-end as fd[0] and the write-end as fd[1].
+   Return 0 upon success, or -1 with errno set upon failure.  */
+# if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
+#  define pipe(fd) _pipe (fd, 4096, _O_BINARY)
+# endif
+#elif defined GNULIB_POSIXCHECK
+# undef pipe
+# if HAVE_RAW_DECL_PIPE
+_GL_WARN_ON_USE (pipe, "pipe is unportable - "
+ "use gnulib module pipe for portability");
+# endif
+#endif
+
+
 #if @GNULIB_PIPE2@
 /* Create a pipe, applying the given flags when opening the read-end of the
pipe and the write-end of the pipe.
diff --git a/m4/unistd_h.m4 b/m4/unistd_h.m4
index e2f7f2b..0690951 100644
--- a/m4/unistd_h.m4
+++ b/m4/unistd_h.m4
@@ -38,7 +38,7 @@ AC_DEFUN([gl_UNISTD_H],
 ]], [chown dup2 dup3 environ euidaccess faccessat fchdir fchownat
 fsync ftruncate getcwd getdomainname getdtablesize getgroups
 gethostname getlogin getlogin_r getpagesize getusershell setusershell
-endusershell lchown link linkat lseek pipe2 pread pwrite readlink
+endusershell lchown link linkat lseek pipe pipe2 pread pwrite readlink
 readlinkat rmdir sleep symlink symlinkat ttyname_r unlink unlinkat
 usleep])
 ])
@@ -78,6 +78,7 @@ A

relax license of cloexec, fcntl

[Eric Blake]

Jim, Bruno, and Paolo,

I want to eventually get to the point where open(O_CLOEXEC) can be
supported in gnulib for use in libraries like LGPLv2+ libvirt.  Along
the way, I will want to make open (LGPLv2+) depend on cloexec (currently
LGPLv3+), which in turn is a thin wrapper around fcntl (currently
LGPLv3+).  Meanwhile, I can immediately start using cloexec within
libvirt before O_CLOEXEC support is complete in gnulib.

Are there any objections to relaxing the license?

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: relax license of cloexec, fcntl

[Paul Eggert]

On 12/10/2010 03:36 PM, Eric Blake wrote:

> Are there any objections to relaxing the license?

None here.

By the way, it bugs me a bit that in the normal
(e.g., Linux or Solaris) case, set_cloexec_flag
issues two system calls when one would do.  Could
this be fixed at some point?  One advantage of a
single system call is that it's nicer in the presence
of threads and forking (a topic that has been taking
up far too much of my limited spare time lately :-).

pipe: rename to spawn-pipe

[Bruno Haible]

Eric Blake wrote:
> The module name 'pipe'
> is already claimed for an unrelated application-level wrapper around
> both pipe(2) and the notion of a child process.

Since at this point we are not considering moving either all POSIX
related modules or all application-level modules to a new naming
convention, I think it's best to rename the application-level module 'pipe'
at this point.

In a second step, in a couple of months, we'll be able to remove the
module 'pipe' without much damage.

In a third step, later again, you can then rename 'pipe-posix' to 'pipe'.


2010-12-10  Bruno Haible  

Rename module 'pipe' to 'spawn-pipe'.
* modules/spawn-pipe: New file, renamed from modules/pipe.
(Files, configure.ac, Makefile.am): Update.
(Include): Mention "spawn-pipe.h" instead of "pipe.h".
* modules/pipe: Reduce to an obsolete indirection to 'spawn-pipe'.
* lib/spawn-pipe.h: New file, renamed from lib/pipe.h.
* lib/spawn-pipe.c: New file, renamed from lib/pipe.c. Include
"spawn-pipe.h" instead of "pipe.h".
* m4/spawn-pipe.m4: New file, renamed from m4/pipe.m4. Rename gl_PIPE
to gl_SPAWN_PIPE.
* modules/spawn-pipe-tests: New file, renamed from modules/pipe-tests.
(Files, Makefile.am): Update.
* tests/test-spawn-pipe.sh: New file, renamed from tests/test-pipe.sh.
Update.
* tests/test-spawn-pipe.c: New file, renamed from tests/test-pipe.c.
Include "spawn-pipe.h" instead of "pipe.h".
* lib/csharpcomp.c: Include "spawn-pipe.h" instead of "pipe.h".
* lib/javacomp.c: Likewise.
* lib/javaversion.c: Likewise.
* lib/pipe-filter-gi.c: Likewise.
* lib/pipe-filter-ii.c: Likewise.
* modules/csharpcomp (Depends-on): Add 'spawn-pipe', remove 'pipe'.
* modules/javacomp (Depends-on): Likewise.
* modules/javaversion (Depends-on): Likewise.
* modules/pipe-filter-gi (Depends-on): Likewise.
* modules/pipe-filter-ii (Depends-on): Likewise.
* MODULES.html.sh (Executing programs): Update.
* NEWS: Mention the change.

Re: [PATCH] pipe-posix: new module

[Bruno Haible]

Hi Eric,

> since it is just a #define rather than a replacement function, it
> fails the POSIX aspect of being able to pass a function pointer to
> pipe around

And it also fails the signature test in C++ mode: When I add these lines

*** tests/test-unistd-c++.cc.orig   Sat Dec 11 02:47:25 2010
--- tests/test-unistd-c++.ccSat Dec 11 02:34:46 2010
***
*** 129,134 
--- 129,138 
  SIGNATURE_CHECK (GNULIB_NAMESPACE::lseek, off_t, (int, off_t, int));
  #endif
  
+ #if GNULIB_TEST_PIPE
+ SIGNATURE_CHECK (GNULIB_NAMESPACE::pipe, int, (int[2]));
+ #endif
+ 
  #if GNULIB_TEST_PIPE2
  SIGNATURE_CHECK (GNULIB_NAMESPACE::pipe2, int, (int[2], int));
  #endif

then create a testdir like this:

$ ./gnulib-tool --create-testdir --dir=/home/bruno/data/tmp/testdir4 
--with-tests --with-c++-tests pipe-posix

then on mingw I get a build failure:

test-unistd-c++.cc:133: error: `pipe' is not a member of `gnulib'
test-unistd-c++.cc:133: warning: 'signature_check133' defined but not used
make[4]: *** [test-unistd-c++.o] Error 1

It's the same situation as with waitpid() in September.

This fixes it.


2010-12-10  Bruno Haible  

pipe-posix: Make it work in C++ mode.
* lib/unistd.in.h: Don't include ,  for pipe.
(pipe): Use common idiom, not a macro definition.
* lib/pipe.c: New file.
* m4/pipe.m4: New file.
* modules/pipe-posix (Description): Enhance.
(Files): Add lib/pipe.c, m4/pipe.m4.
(configure.ac): Invoke gl_FUNC_PIPE.
* m4/unistd_h.m4 (gl_UNISTD_H_DEFAULTS): Initialize HAVE_PIPE.
* modules/unistd (Makefile.am): Substitute HAVE_PIPE.
* tests/test-unistd-c++.cc: Check the signature of pipe.

= lib/pipe.c =
/* Create a pipe.
   Copyright (C) 2009-2010 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, write to the Free Software Foundation,
   Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.  */

#include 

/* Specification.  */
#include 

#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
/* Native Woe32 API.  */

/* Get _pipe().  */
# include 

/* Get _O_BINARY.  */
# include 

int
pipe (int fd[2])
{
  return _pipe (fd, 4096, _O_BINARY);
}

#else

# error "This platform lacks a pipe function, and Gnulib doesn't provide a 
replacement. This is a bug in Gnulib."

#endif
= m4/pipe.m4 =
# pipe.m4 serial 1
dnl Copyright (C) 2010 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.

AC_DEFUN([gl_FUNC_PIPE],
[
  AC_REQUIRE([gl_UNISTD_H_DEFAULTS])

  AC_CHECK_FUNCS_ONCE([pipe])
  if test $ac_cv_func_pipe != yes; then
HAVE_PIPE=0
AC_LIBOBJ([pipe])
  fi
])
==
--- lib/unistd.in.h.origSat Dec 11 02:55:30 2010
+++ lib/unistd.in.h Sat Dec 11 02:19:32 2010
@@ -82,14 +82,10 @@
 # include 
 #endif
 
-/* mingw declares getcwd in , not in .  It also provides
-   _pipe in , but that requires _O_BINARY from .  */
-#if ((@GNULIB_GETCWD@ || @GNULIB_PIPE@ || defined GNULIB_POSIXCHECK) \
+/* mingw declares getcwd in , not in .  */
+#if ((@GNULIB_GETCWD@ || defined GNULIB_POSIXCHECK) \
  && ((defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__))
 # include 
-# if @GNULIB_PIPE@
-#  include 
-# endif
 #endif
 
 /* AIX and OSF/1 5.1 declare getdomainname in , not in .  */
@@ -980,14 +976,22 @@
 /* Create a pipe, defaulting to O_BINARY mode.
Store the read-end as fd[0] and the write-end as fd[1].
Return 0 upon success, or -1 with errno set upon failure.  */
-# if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
-#  define pipe(fd) _pipe (fd, 4096, _O_BINARY)
+# if @HAVE_PIPE@
+#  if !(defined __cplusplus && defined GNULIB_NAMESPACE)
+#   define pipe rpl_pipe
+#  endif
+_GL_FUNCDECL_RPL (pipe, int, (int fd[2]) _GL_ARG_NONNULL ((1)));
+_GL_CXXALIAS_RPL (pipe, int, (int fd[2]));
+# else
+_GL_FUNCDECL_SYS (pipe, int, (int fd[2]) _GL_ARG_NONNULL ((1)));
+_GL_CXXALIAS_SYS (pipe, int, (int fd[2]));
 # endif
+_GL_CXXALIASWARN (pipe);
 #elif defined GNULIB_POSIXCHECK
 # undef pipe
 # if HAVE_RAW_DECL_PIPE
 _GL_WARN_ON_USE (pipe, "pipe

Re: [PATCH] pipe-posix: new module

[Bruno Haible]

Eric Blake wrote:
> testing I/O through the pipe is impossible without
> either threading or spawning a child process, all of which seem rather
> complex given how simple the module itself is.

A partial test is better than none. In fact, we already have a test for the
pipe2() function. It's easy to adapt it to the pipe() function.

In fact, this test uncovers a link error of my previous commit:
symbol rpl_pipe not found on a glibc system. I had copied the wrong idiom
into unistd.in.h. Lesson learned: Even a simple link test is better than no
test at all.


2010-12-10  Bruno Haible  

Tests for module 'pipe-posix'.
* modules/pipe-posix-tests: New file.
* tests/test-pipe.c: New file, based on tests/test-pipe2.c.

== modules/pipe-posix-tests ==
Files:
tests/test-pipe.c
tests/signature.h
tests/macros.h

Depends-on:
stdbool
binary-io

configure.ac:

Makefile.am:
TESTS += test-pipe
check_PROGRAMS += test-pipe
== tests/test-pipe.c ==
/* Test of pipe.
   Copyright (C) 2009-2010 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 3, 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, write to the Free Software Foundation,
   Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.  */

#include 

#include 

#include "signature.h"
SIGNATURE_CHECK (pipe, int, (int[2]));

#include 
#include 

#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
/* Get declarations of the Win32 API functions.  */
# define WIN32_LEAN_AND_MEAN
# include 
#endif

#include "binary-io.h"
#include "macros.h"

/* Return true if FD is open.  */
static bool
is_open (int fd)
{
#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
  /* On Win32, the initial state of unassigned standard file
 descriptors is that they are open but point to an
 INVALID_HANDLE_VALUE, and there is no fcntl.  */
  return (HANDLE) _get_osfhandle (fd) != INVALID_HANDLE_VALUE;
#else
# ifndef F_GETFL
#  error Please port fcntl to your platform
# endif
  return 0 <= fcntl (fd, F_GETFL);
#endif
}

/* Return true if FD is not inherited to child processes.  */
static bool
is_cloexec (int fd)
{
#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
  HANDLE h = (HANDLE) _get_osfhandle (fd);
  DWORD flags;
  ASSERT (GetHandleInformation (h, &flags));
  return (flags & HANDLE_FLAG_INHERIT) == 0;
#else
  int flags;
  ASSERT ((flags = fcntl (fd, F_GETFD)) >= 0);
  return (flags & FD_CLOEXEC) != 0;
#endif
}

/* Return true if FD is in non-blocking mode.  */
static bool
is_nonblocking (int fd)
{
#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
  /* We don't use the non-blocking mode for sockets here.  */
  return 0;
#else
  int flags;
  ASSERT ((flags = fcntl (fd, F_GETFL)) >= 0);
  return (flags & O_NONBLOCK) != 0;
#endif
}

int
main ()
{
  int fd[2];

  fd[0] = -1;
  fd[1] = -1;
  ASSERT (pipe (fd) >= 0);
  ASSERT (fd[0] >= 0);
  ASSERT (fd[1] >= 0);
  ASSERT (fd[0] != fd[1]);
  ASSERT (is_open (fd[0]));
  ASSERT (is_open (fd[1]));
  ASSERT (!is_cloexec (fd[0]));
  ASSERT (!is_cloexec (fd[1]));
  ASSERT (!is_nonblocking (fd[0]));
  ASSERT (!is_nonblocking (fd[1]));

  return 0;
}

Re: relax license of cloexec, fcntl

[Bruno Haible]

Hi Eric,

> cloexec (currently LGPLv3+),  fcntl (currently LGPLv3+).
> Are there any objections to relaxing the license?

Fine with me. lib/fcntl.c contains some originally GPLed code from me
(the DuplicateHandle stuff: originally in lib/w32spawn.h under GPL,
moved to lib/cloexec.c on 2009-12-05, became LGPLv3+ on 2009-12-10,
moved to lib/fcntl.c on 2009-12-16), but that is not worth considering
as very valuable.

Eric Blake wrote:
> I will want to make open (LGPLv2+) depend on cloexec

We carefully avoided to have such a dependency. I guess that at least
some people would consider it "creepy dependencies": The 'open' module fixes
a number of problems that people usually want to avoid. But 'cloexec'
is useful only in programs that spawn subprocesses. Programs that simply
manipulate files without having subprocesses will not need this. How
about having two modules 'open' and 'open-cloexec' (or maybe 'open-posix')?

Bruno