On Wed, Apr 20, 2011 at 02:55:10PM -0700, Jesse Gross wrote: > On Wed, Apr 20, 2011 at 8:58 AM, Ben Pfaff <[email protected]> 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 <[email protected]> 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: --8<--------------------------cut here-------------------------->8-- From: Ben Pfaff <[email protected]> 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 <[email protected]> --- 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 --- a/lib/socket-util.c +++ b/lib/socket-util.c @@ -306,18 +306,11 @@ make_sockaddr_un(const char *name, struct sockaddr_un *un, socklen_t *un_len, static int bind_unix_socket(int fd, struct sockaddr *sun, socklen_t sun_len) { -#ifdef __linux__ - /* 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; -#else /* According to _Unix Network Programming_, umask should affect bind(). */ mode_t old_umask = umask(0077); int error = bind(fd, sun, sun_len) ? errno : 0; umask(old_umask); return error; -#endif } /* Creates a Unix domain socket in the given 'style' (either SOCK_DGRAM or -- 1.7.1 _______________________________________________ discuss mailing list [email protected] http://openvswitch.org/mailman/listinfo/discuss
