Re: [PATCH 2/4] sendfd, recvf pass file descriptors along Unix domain sockets

[Bruno Haible]

Hi Bastien,

>  lib/passfd.c   |  147 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/passfd.h   |   21 ++++++++
>  modules/passfd |   30 +++++++++++

This code looked nearly fine as well (at least superficially), so I've
committed it in your name with this ChangeLog entry:

2011-03-07  Bastien Roucariès  <address@hidden>

        passfd module, part 2.
        * modules/passfd: New file.
        * lib/passfd.h: New file.
        * lib/passfd.c: New file.

and added some tweaks:
- In the module description, section "Include", you should mention only the
  header files the programmer needs to use for your module.
- In the lib/ source files, we use the GPLv3+ copyright header normally.
  The real license terms are in the modules file, however.
- We avoid tabs in gnulib source code. See the README file, section
  "Indent with spaces, not TABs", for a recipe that makes sure not to introduce
  tabs accidentally.
- We use #if, not #ifdef, to test the value of HAVE_... macros defined in
  config.h.
- Specify the functions unambiguously: If a function is expected to set errno
  when it fails (and your unit test in part 4 apparently expects this), then
  the function's specification should say so.
- The specification of recvfd was merely a copy&paste of the specification of
  sendfd.
- Once you depend on module 'sys_socket', you can include <sys/socket.h>
  unconditionally and don't need <winsock2.h>, because the 'sys_socket' module
  deals with all this.
- Reduce the scope of the variables 'fd' and 'mone', so as to avoid gcc warnings
  like "warning: unused variable ‘mone’".
- The dependency to module 'errno' is not needed, since the only errno values
  that you use are ENOSYS and EACCES. (See doc/posix-headers/errno.texi.)


2011-03-13  Bruno Haible  <address@hidden>

        passfd module, part 2, tweaks.
        * modules/passfd (Files): Reorder.
        (Depends-on): Remove errno.
        (Include): Remove <sys/socket.h>, <sys/un.h>.
        * lib/passfd.h: Use a GPLv3+ header. Make C++ safe.
        * lib/passfd.c: Untabify. Use a GPLv3+ header. Really include the
        specification header. Include <sys/socket.h> always. Don't include
        <winsock2.h>. Use "#if HAVE_..." instead of "#ifdef HAVE_...".
        (sendfd): Clarify that it sets errno when it fails.
        (recvfd): Fix specification.

--- modules/passfd.orig Sun Mar 13 15:16:08 2011
+++ modules/passfd      Sun Mar 13 15:09:09 2011
@@ -2,13 +2,12 @@
 Passfile descriptors along Unix (socket/stream) file descriptors
 
 Files:
-m4/sockpfaf.m4
-m4/afunix.m4
-lib/passfd.c
 lib/passfd.h
+lib/passfd.c
+m4/afunix.m4
+m4/sockpfaf.m4
 
 Depends-on:
-errno
 sys_socket
 extensions
 
@@ -19,8 +19,6 @@
 lib_SOURCES += passfd.c
 
 Include:
-<sys/socket.h>
-<sys/un.h>
 "passfd.h"
 
 License:
--- lib/passfd.h.orig   Sun Mar 13 15:16:08 2011
+++ lib/passfd.h        Sun Mar 13 15:12:04 2011
@@ -4,7 +4,7 @@
 
    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 of the License, or
+   the Free Software Foundation; either version 3 of the License, or
    (at your option) any later version.
 
    This program is distributed in the hope that it will be useful,
@@ -14,8 +14,19 @@
 
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
 #ifndef PASSFD_H_
 #define PASSFD_H_ 1
-int sendfd (int sock, int fd);
-int recvfd (int sock);
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+extern int sendfd (int sock, int fd);
+extern int recvfd (int sock);
+
+#ifdef __cplusplus
+}
+#endif
+
 #endif
--- lib/passfd.c.orig   Sun Mar 13 15:16:07 2011
+++ lib/passfd.c        Sun Mar 13 15:15:19 2011
@@ -2,7 +2,7 @@
 
    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 of the License, or
+   the Free Software Foundation; either version 3 of the License, or
    (at your option) any later version.
 
    This program is distributed in the hope that it will be useful,
@@ -16,6 +16,8 @@
 #include <config.h>
 
 /* Specification.  */
+#include "passfd.h"
+
 #include <errno.h>
 #include <stddef.h>
 #include <stdlib.h>
@@ -23,20 +25,15 @@
 #include <sys/types.h>
 #include <unistd.h>
 
-#ifdef HAVE_SYS_SOCKET_H
 #include <sys/socket.h>
-#endif
-#ifdef HAVE_SYS_UN_H
-#include <sys/un.h>
-#endif
-#ifdef HAVE_WINSOCK2_H
-#include <winsock2.h>
+#if HAVE_SYS_UN_H
+# include <sys/un.h>
 #endif
 
-/* Sendfd sends the file descriptor fd along the socket
+/* sendfd sends the file descriptor fd along the socket
    to a process calling recvfd on the other end.
-   
-   return -1 in case of error, 0 on success
+
+   Return 0 on success, or -1 with errno set in case of error.
 */
 int
 sendfd (int sock, int fd)
@@ -54,7 +51,7 @@ sendfd (int sock, int fd)
   msg.msg_namelen = 0;
 
   {
-#ifdef HAVE_UNIXSOCKET_SCM_RIGHTS_BSD44_WAY
+#if HAVE_UNIXSOCKET_SCM_RIGHTS_BSD44_WAY
     struct cmsghdr *cmsg;
     char buf[CMSG_SPACE (sizeof (fd))];
 
@@ -81,17 +78,14 @@ sendfd (int sock, int fd)
   return 0;
 }
 
-/* Sendfd sends the file descriptor fd along the socket 
-   to a process calling recvfd on the other end.
+/* recvfd receives a file descriptor through the socket.
 
-   return -1 in case of error, fd on success
+   Return 0 on success, or -1 with errno set in case of error.
 */
 int
 recvfd (int sock)
 {
   char recv = 0;
-  const int mone = -1;
-  int fd;
   struct iovec iov[1];
   struct msghdr msg;
 
@@ -104,9 +98,11 @@ recvfd (int sock)
   msg.msg_namelen = 0;
 
   {
-#ifdef HAVE_UNIXSOCKET_SCM_RIGHTS_BSD44_WAY
+#if HAVE_UNIXSOCKET_SCM_RIGHTS_BSD44_WAY
+    int fd;
     struct cmsghdr *cmsg;
     char buf[CMSG_SPACE (sizeof (fd))];
+    const int mone = -1;
 
     msg.msg_control = buf;
     msg.msg_controllen = sizeof (buf);
@@ -124,16 +120,18 @@ recvfd (int sock)
     cmsg = CMSG_FIRSTHDR (&msg);
     /* be paranoiac */
     if (cmsg == NULL || cmsg->cmsg_len != CMSG_LEN (sizeof (int))
-       || cmsg->cmsg_level != SOL_SOCKET || cmsg->cmsg_type != SCM_RIGHTS)
+        || cmsg->cmsg_level != SOL_SOCKET || cmsg->cmsg_type != SCM_RIGHTS)
       {
-       /* fake errno: at end the file is not available */
-       errno = EACCES;
-       return -1;
+        /* fake errno: at end the file is not available */
+        errno = EACCES;
+        return -1;
       }
 
     memcpy (&fd, CMSG_DATA (cmsg), sizeof (fd));
     return fd;
 #elif HAVE_UNIXSOCKET_SCM_RIGHTS_BSD43_WAY
+    int fd;
+
     msg.msg_accrights = &fd;
     msg.msg_accrightslen = sizeof (fd);
     if (recvmsg (sock, &msg, 0) < 0)

-- 
In memoriam Odette Sansom <http://en.wikipedia.org/wiki/Odette_Hallowes>