On 03/28/2011 05:41 AM, Bastien ROUCARIES wrote: > Fix a typo in passfd code > --- > lib/passfd.c | 1 - > 1 files changed, 0 insertions(+), 1 deletions(-) > > diff --git a/lib/passfd.c b/lib/passfd.c > index 573b80e..ae716a6 100644 > --- a/lib/passfd.c > +++ b/lib/passfd.c > @@ -67,7 +67,6 @@ sendfd (int sock, int fd) > cmsg->cmsg_len = CMSG_LEN (sizeof (int)); > /* Initialize the payload: */ > memcpy (CMSG_DATA (cmsg), &fd, sizeof (fd)); > - msg.msg_controllen = cmsg->cmsg_len; > #elif HAVE_UNIXSOCKET_SCM_RIGHTS_BSD43_WAY > msg.msg_accrights = &fd; > msg.msg_accrightslen = sizeof (fd);
Hmm, both before and after this patch of yours, I get things to compile
on NetBSD 5.0.2, but the test hangs in both cases, after printing:
sendfd: Invalid argument
Hanging a unit test is bad; so I'm also going to check in a followup
that does an alarm(5) to the self-test (we've done it elsewhere, so I
have precedence). However, I also concur that your patch is reasonable,
as you already assigned msg_controllen earlier in the function. I
guessed that the reason for the failure is that you forgot to assign
msg_flags, and NetBSD was rejecting uninitialized data as invalid flags.
However, after pushing this patch, things still fail, so I'm still
investigating.
From cc0745fc8e17997b71a1e021ca15c0b95ba779b9 Mon Sep 17 00:00:00 2001
From: Eric Blake <ebl...@redhat.com>
Date: Wed, 30 Mar 2011 14:46:02 -0600
Subject: [PATCH] passfd: fix incorrect sendmsg arguments
The unit test hung on NetBSD, which pointed out a couple of bugs.
* lib/passfd.c (sendfd): Avoid uninitialized msg_flags field, and
incorrect msg_controllen value.
* modules/passfd-tests (Depends-on): Check for alarm.
* tests/test-passfd.c (main) [HAVE_DECL_ALARM]: Avoid hanging test.
Reported by Bastien ROUCARIES.
---
ChangeLog | 9 +++++++++
lib/passfd.c | 2 +-
modules/passfd-tests | 1 +
tests/test-passfd.c | 7 +++++++
4 files changed, 18 insertions(+), 1 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 09a5810..ec324a0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2011-03-30 Eric Blake <ebl...@redhat.com>
+
+ passfd: fix incorrect sendmsg arguments
+ * lib/passfd.c (sendfd): Avoid uninitialized msg_flags field, and
+ incorrect msg_controllen value.
+ * modules/passfd-tests (Depends-on): Check for alarm.
+ * tests/test-passfd.c (main) [HAVE_DECL_ALARM]: Avoid hanging test.
+ Reported by Bastien ROUCARIES.
+
2011-03-30 Jim Meyering <meyer...@redhat.com>
tests: readlink* ("",... fails with EINVAL on newer kernels
diff --git a/lib/passfd.c b/lib/passfd.c
index 573b80e..5bf400d 100644
--- a/lib/passfd.c
+++ b/lib/passfd.c
@@ -47,6 +47,7 @@ sendfd (int sock, int fd)
struct msghdr msg;
/* send at least one char */
+ memset (&msg, 0, sizeof msg);
iov[0].iov_base = &send;
iov[0].iov_len = 1;
msg.msg_iov = iov;
@@ -67,7 +68,6 @@ sendfd (int sock, int fd)
cmsg->cmsg_len = CMSG_LEN (sizeof (int));
/* Initialize the payload: */
memcpy (CMSG_DATA (cmsg), &fd, sizeof (fd));
- msg.msg_controllen = cmsg->cmsg_len;
#elif HAVE_UNIXSOCKET_SCM_RIGHTS_BSD43_WAY
msg.msg_accrights = &fd;
msg.msg_accrightslen = sizeof (fd);
diff --git a/modules/passfd-tests b/modules/passfd-tests
index 132679c..477754b 100644
--- a/modules/passfd-tests
+++ b/modules/passfd-tests
@@ -5,6 +5,7 @@ tests/macros.h
Depends-on:
configure.ac:
+AC_CHECK_DECLS_ONCE([alarm])
Makefile.am:
TESTS += test-passfd
diff --git a/tests/test-passfd.c b/tests/test-passfd.c
index 2048934..d657ad9 100644
--- a/tests/test-passfd.c
+++ b/tests/test-passfd.c
@@ -19,6 +19,7 @@
#include "passfd.h"
#include <fcntl.h>
+#include <signal.h>
#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
@@ -40,6 +41,12 @@ main ()
int fd;
struct stat st;
+#if HAVE_DECL_ALARM
+ /* Avoid hanging on failure. */
+ signal (SIGALRM, SIG_DFL);
+ alarm (5);
+#endif
+
fdnull = open ("/dev/null", O_RDWR);
if (fdnull < 0)
{
--
1.7.4
I'm also pushing this followup, which reduces the code size a bit (we
prefer sizeof variable over sizeof (type), NULL over 0 for null
pointers, there's no need to make a one-element array, and we prefer to
avoid in-function #ifdefs when possible).
From aeea9482713470cd1e4cd78b39c8d87841c62b2f Mon Sep 17 00:00:00 2001
From: Eric Blake <ebl...@redhat.com>
Date: Wed, 30 Mar 2011 15:30:13 -0600
Subject: [PATCH] passfd: standardize coding conventions
* m4/afunix.m4 (gl_SOCKET_AFUNIX): Drop check for something that
can be learned at compile time.
* lib/passfd.c (MSG_CMSG_CLOEXEC): Reduce number of in-function
ifdefs.
(passfd, recvfd): Follow gnulib code conventions.
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
ChangeLog | 7 +++++++
lib/passfd.c | 48 +++++++++++++++++++++++-------------------------
m4/afunix.m4 | 29 +----------------------------
3 files changed, 31 insertions(+), 53 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index ec324a0..5d4dc84 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
2011-03-30 Eric Blake <ebl...@redhat.com>
+ passfd: standardize coding conventions
+ * m4/afunix.m4 (gl_SOCKET_AFUNIX): Drop check for something that
+ can be learned at compile time.
+ * lib/passfd.c (MSG_CMSG_CLOEXEC): Reduce number of in-function
+ ifdefs.
+ (passfd, recvfd): Follow gnulib code conventions.
+
passfd: fix incorrect sendmsg arguments
* lib/passfd.c (sendfd): Avoid uninitialized msg_flags field, and
incorrect msg_controllen value.
diff --git a/lib/passfd.c b/lib/passfd.c
index 5bf400d..f507dde 100644
--- a/lib/passfd.c
+++ b/lib/passfd.c
@@ -34,6 +34,10 @@
#include "cloexec.h"
+#ifndef MSG_CMSG_CLOEXEC
+# define MSG_CMSG_CLOEXEC 0
+#endif
+
/* sendfd sends the file descriptor fd along the socket
to a process calling recvfd on the other end.
@@ -43,41 +47,41 @@ int
sendfd (int sock, int fd)
{
char send = 0;
- struct iovec iov[1];
+ struct iovec iov;
struct msghdr msg;
/* send at least one char */
memset (&msg, 0, sizeof msg);
- iov[0].iov_base = &send;
- iov[0].iov_len = 1;
- msg.msg_iov = iov;
+ iov.iov_base = &send;
+ iov.iov_len = 1;
+ msg.msg_iov = &iov;
msg.msg_iovlen = 1;
- msg.msg_name = 0;
+ msg.msg_name = NULL;
msg.msg_namelen = 0;
{
#if HAVE_UNIXSOCKET_SCM_RIGHTS_BSD44_WAY
struct cmsghdr *cmsg;
- char buf[CMSG_SPACE (sizeof (fd))];
+ char buf[CMSG_SPACE (sizeof fd)];
msg.msg_control = buf;
- msg.msg_controllen = sizeof (buf);
+ msg.msg_controllen = sizeof buf;
cmsg = CMSG_FIRSTHDR (&msg);
cmsg->cmsg_level = SOL_SOCKET;
cmsg->cmsg_type = SCM_RIGHTS;
- cmsg->cmsg_len = CMSG_LEN (sizeof (int));
+ cmsg->cmsg_len = CMSG_LEN (sizeof fd);
/* Initialize the payload: */
- memcpy (CMSG_DATA (cmsg), &fd, sizeof (fd));
+ memcpy (CMSG_DATA (cmsg), &fd, sizeof fd);
#elif HAVE_UNIXSOCKET_SCM_RIGHTS_BSD43_WAY
msg.msg_accrights = &fd;
- msg.msg_accrightslen = sizeof (fd);
+ msg.msg_accrightslen = sizeof fd;
#else
errno = ENOSYS;
return -1;
#endif
}
- if (sendmsg (sock, &msg, 0) != iov[0].iov_len)
+ if (sendmsg (sock, &msg, 0) != iov.iov_len)
return -1;
return 0;
}
@@ -91,7 +95,7 @@ int
recvfd (int sock, int flags)
{
char recv = 0;
- struct iovec iov[1];
+ struct iovec iov;
struct msghdr msg;
if ((flags & ~O_CLOEXEC) != 0)
@@ -101,24 +105,20 @@ recvfd (int sock, int flags)
}
/* send at least one char */
- iov[0].iov_base = &recv;
- iov[0].iov_len = 1;
- msg.msg_iov = iov;
+ iov.iov_base = &recv;
+ iov.iov_len = 1;
+ msg.msg_iov = &iov;
msg.msg_iovlen = 1;
- msg.msg_name = 0;
+ msg.msg_name = NULL;
msg.msg_namelen = 0;
{
#if HAVE_UNIXSOCKET_SCM_RIGHTS_BSD44_WAY
int fd;
struct cmsghdr *cmsg;
- char buf[CMSG_SPACE (sizeof (fd))];
+ char buf[CMSG_SPACE (sizeof fd)];
const int mone = -1;
-# if HAVE_MSG_CMSG_CLOEXEC
- int flags_recvmsg = (flags & O_CLOEXEC ? MSG_CMSG_CLOEXEC : 0);
-# else
- int flags_recvmsg = 0;
-# endif
+ int flags_recvmsg = flags & O_CLOEXEC ? MSG_CMSG_CLOEXEC : 0;
msg.msg_control = buf;
msg.msg_controllen = sizeof (buf);
@@ -145,9 +145,8 @@ recvfd (int sock, int flags)
memcpy (&fd, CMSG_DATA (cmsg), sizeof (fd));
-# if !HAVE_MSG_CMSG_CLOEXEC
/* set close-on-exec flag */
- if (flags & O_CLOEXEC)
+ if (!MSG_CMSG_CLOEXEC && (flags & O_CLOEXEC))
{
if (set_cloexec_flag (fd, true) < 0)
{
@@ -157,7 +156,6 @@ recvfd (int sock, int flags)
return -1;
}
}
-# endif
return fd;
diff --git a/m4/afunix.m4 b/m4/afunix.m4
index 46b3bf3..13f7583 100644
--- a/m4/afunix.m4
+++ b/m4/afunix.m4
@@ -1,4 +1,4 @@
-# afunix.m4 serial 5
+# afunix.m4 serial 6
dnl Copyright (C) 2011 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
dnl gives unlimited permission to copy and/or distribute it,
@@ -113,31 +113,4 @@ AC_DEFUN([gl_SOCKET_AFUNIX],
AC_DEFINE([HAVE_UNIXSOCKET_SCM_RIGHTS_BSD43_WAY], [1],
[Define to 1 if fd can be sent/received in the BSD4.3 way.])
fi
-
- AC_MSG_CHECKING([for UNIX domain sockets recvmsg() MSG_CMSG_CLOEXEC
flag])
- AC_CACHE_VAL([gl_cv_socket_unix_msg_cmsg_cloexec],
- [AC_COMPILE_IFELSE(
- [AC_LANG_PROGRAM(
- [[#include <sys/types.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>
- #endif
- ]],
- [[int flags = MSG_CMSG_CLOEXEC;
- if (&flags) return 0;
- ]])],
- [gl_cv_socket_unix_msg_cmsg_cloexec=yes],
- [gl_cv_socket_unix_msg_cmsg_cloexec=no])
- ])
- AC_MSG_RESULT([$gl_cv_socket_unix_msg_cmsg_cloexec])
- if test $gl_cv_socket_unix_msg_cmsg_cloexec = yes; then
- AC_DEFINE([HAVE_MSG_CMSG_CLOEXEC], [1],
- [Define to 1 if recvmsg could be specified with MSG_CMSG_CLOEXEC.])
- fi
])
--
1.7.4
--
Eric Blake ebl...@redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
