[ovs-discuss] suspicious usage of fchmod
blp at nicira.com
Thu Apr 21 16:23:17 UTC 2011
On Wed, Apr 20, 2011 at 02:55:10PM -0700, Jesse Gross wrote:
> On Wed, Apr 20, 2011 at 8:58 AM, Ben Pfaff <blp at nicira.com> wrote:
> > On Tue, Apr 19, 2011 at 06:50:44PM -0700, Jesse Gross wrote:
> >> On Mon, Apr 18, 2011 at 11:25 AM, Ben Pfaff <blp at nicira.com> wrote:
> >> > With all that taken into account, here is a new version. ?On Linux, I
> >> > prefer the fchmod() based solution, because someday OVS may have to
> >> > add support for threads and the umask is not thread-local.
> >> Doesn't this just introduce another portability problem? ?If we do
> >> introduce threads then it will be less likely that we notice it since
> >> things will work fine on Linux.
> > We'll have to do extensive grepping around at that point anyway and
> > the system-specific #if should stand out well enough.
> But what's the point of introducing this #ifdef in the first place?
> It seems like there are two situations:
> * We don't introduce threading, in which case there is no problem but
> also no benefit from this extra piece of OS dependent code which will
> rarely get exercised.
> * We do implement threading, in which case there is a piece of code
> which we will hopefully remember or find that needs to be fixed.
It sounds like you'd prefer to use the portable solution everywhere.
Fine. Please review this:
From: Ben Pfaff <blp at nicira.com>
Date: Thu, 21 Apr 2011 09:22:39 -0700
Subject: [PATCH] socket-util: Use portable solution for setting Unix socket permissions.
Requested-by: Jesse Gross <jesse at nicira.com>
lib/socket-util.c | 7 -------
1 files changed, 0 insertions(+), 7 deletions(-)
diff --git a/lib/socket-util.c b/lib/socket-util.c
index 7e4b8be..24e8f81 100644
@@ -306,18 +306,11 @@ make_sockaddr_un(const char *name, struct sockaddr_un *un, socklen_t *un_len,
bind_unix_socket(int fd, struct sockaddr *sun, socklen_t sun_len)
- /* On Linux, calling fchmod() *before* bind() sets permissions for the file
- * about to be created. Calling fchmod() *after* bind has no effect on the
- * file that was created.) */
- return fchmod(fd, 0700) || bind(fd, sun, sun_len) ? errno : 0;
/* According to _Unix Network Programming_, umask should affect bind(). */
mode_t old_umask = umask(0077);
int error = bind(fd, sun, sun_len) ? errno : 0;
/* Creates a Unix domain socket in the given 'style' (either SOCK_DGRAM or
More information about the discuss