Re: [PATCH] nonblocking: new module

[Bruno Haible]

Eric Blake wrote:
> I've touched up your proposal, and done more testing with this.

Great, thanks!

I've done three more touch-ups:
  - In the module description, prefer a lib_SOURCES augmentation to an
    AC_LIBOBJ invocation when possible. In this case both are equivalent,
    but AC_LIBOBJ has some restrictions that make lib_SOURCES a better
    choice in general.
  - Fix ASSERT expressions: recall that == has higher precedence than ? :.
  - A little more structure in the tests file, to separate independent tests.

Bruno


--- lib/nonblocking.c.orig      Thu Mar 31 13:06:41 2011
+++ lib/nonblocking.c   Thu Mar 31 12:53:36 2011
@@ -48,6 +48,7 @@
         return -1;
     }
   else
+    /* Win32 does not support non-blocking on regular files.  */
     return 0;
 }
 
@@ -125,7 +126,7 @@
   fcntl_flags = fcntl (desc, F_GETFL, 0);
   if (fcntl_flags < 0)
     return -1;
-  if (!!(O_NONBLOCK & fcntl_flags) == value)
+  if (((fcntl_flags & O_NONBLOCK) != 0) == value)
     return 0;
   if (value)
     fcntl_flags |= O_NONBLOCK;
--- lib/nonblocking.h.orig      Thu Mar 31 13:06:41 2011
+++ lib/nonblocking.h   Thu Mar 31 12:51:19 2011
@@ -26,6 +26,9 @@
    - A write() call returns -1 with errno set to EAGAIN when it cannot
      transport the requested amount of data (but at most one pipe buffer)
      without blocking.
+   Non-blocking I/O is most useful for character devices, pipes, and sockets.
+   Whether it also works on regular files and block devices is platform
+   dependent.
 
    There are three modern alternatives to non-blocking I/O:
      - use select() or poll() followed by read() or write() if the descriptor
@@ -48,8 +50,7 @@
    Return 0 upon success, or -1 with errno set upon failure.
    The default depends on the presence of the O_NONBLOCK flag for files
    or pipes opened with open() or on the presence of the SOCK_NONBLOCK
-   flag for sockets.  Regular files and directories cannot be made
-   non-blocking; block and character devices depend on the system.  */
+   flag for sockets.  */
 extern int set_nonblocking_flag (int desc, bool value);
 
 
--- modules/nonblocking.orig    Thu Mar 31 13:06:41 2011
+++ modules/nonblocking Thu Mar 31 12:43:41 2011
@@ -12,9 +12,9 @@
 sys_socket
 
 configure.ac:
-AC_LIBOBJ([nonblocking])
 
 Makefile.am:
+lib_SOURCES += nonblocking.c
 
 Include:
 "nonblocking.h"
--- tests/test-nonblocking.c.orig       Thu Mar 31 13:06:41 2011
+++ tests/test-nonblocking.c    Thu Mar 31 13:02:40 2011
@@ -31,27 +31,29 @@
 main (void)
 {
   const char *file = "test-nonblock.tmp";
-  int fd_file = creat (file, 0600);
+  int fd_file;
   int fd_pipe[2];
   int fd_sock;
   bool sock_works = true;
 
 #if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
-  /* For now, we can't get nonblock status of windows sockets.  */
+  /* For now, we can't get nonblocking status of windows sockets.  */
   sock_works = false;
 #endif
 
+  fd_file = creat (file, 0600);
+
   /* Assume std descriptors were provided by invoker.  */
   ASSERT (STDERR_FILENO < fd_file);
 
-  /* Test regular files; setting nonblock on file is unspecified.  */
+  /* Test regular files; setting nonblocking on file is unspecified.  */
   ASSERT (get_nonblocking_flag (fd_file) == 0);
   ASSERT (set_nonblocking_flag (fd_file, false) == 0);
   ASSERT (get_nonblocking_flag (fd_file) == 0);
   ASSERT (close (fd_file) == 0);
   ASSERT (unlink (file) == 0);
 
-  /* Test directories; setting nonblock is unspecified.  */
+  /* Test directories; setting nonblocking is unspecified.  */
   fd_file = open (".", O_RDONLY);
   ASSERT (STDERR_FILENO < fd_file);
   ASSERT (get_nonblocking_flag (fd_file) == 0);
@@ -84,32 +86,40 @@
 
   /* Test sockets.  */
   fd_sock = socket (AF_INET, SOCK_STREAM, 0);
-  ASSERT (get_nonblocking_flag (fd_sock) == sock_works ? 0 : -1);
+  ASSERT (get_nonblocking_flag (fd_sock) == (sock_works ? 0 : -1));
   ASSERT (set_nonblocking_flag (fd_sock, true) == 0);
-  ASSERT (get_nonblocking_flag (fd_sock) == sock_works ? 1 : -1);
+  ASSERT (get_nonblocking_flag (fd_sock) == (sock_works ? 1 : -1));
   ASSERT (set_nonblocking_flag (fd_sock, false) == 0);
-  ASSERT (get_nonblocking_flag (fd_sock) == sock_works ? 0 : -1);
+  ASSERT (get_nonblocking_flag (fd_sock) == (sock_works ? 0 : -1));
   ASSERT (close (fd_sock) == 0);
 
 #if SOCK_NONBLOCK
   fd_sock = socket (AF_INET, SOCK_STREAM | SOCK_NONBLOCK, 0);
-  ASSERT (get_nonblocking_flag (fd_sock) == sock_works ? 1 : -1);
+  ASSERT (get_nonblocking_flag (fd_sock) == (sock_works ? 1 : -1));
   ASSERT (close (fd_sock) == 0);
 #endif /* SOCK_NONBLOCK */
 
   /* Test error handling.  */
-  errno = 0;
-  ASSERT (get_nonblocking_flag (-1) == -1);
-  ASSERT (errno == EBADF);
-  errno = 0;
-  ASSERT (set_nonblocking_flag (-1, false) == -1);
-  ASSERT (errno == EBADF);
-  errno = 0;
-  ASSERT (set_nonblocking_flag (-1, true) == -1);
-  ASSERT (errno == EBADF);
-  errno = 0;
-  ASSERT (set_nonblocking_flag (10000000, false) == -1);
-  ASSERT (errno == EBADF);
+  {
+    errno = 0;
+    ASSERT (get_nonblocking_flag (-1) == -1);
+    ASSERT (errno == EBADF);
+  }
+  {
+    errno = 0;
+    ASSERT (set_nonblocking_flag (-1, false) == -1);
+    ASSERT (errno == EBADF);
+  }
+  {
+    errno = 0;
+    ASSERT (set_nonblocking_flag (-1, true) == -1);
+    ASSERT (errno == EBADF);
+  }
+  {
+    errno = 0;
+    ASSERT (set_nonblocking_flag (10000000, false) == -1);
+    ASSERT (errno == EBADF);
+  }
 
   return 0;
 }

-- 
In memoriam Selena Quintanilla <http://en.wikipedia.org/wiki/Selena>