[v2] Remove __ASSUME_O_CLOEXEC / O_CLOEXEC conditionals in sysdeps/unix/sysv/linux/
Commit Message
(Differences from the previous version
<https://sourceware.org/ml/libc-alpha/2014-06/msg00520.html>: this one
includes the removal of a <kernel-features.h> include from shm_open.c,
while I've committed all the rest of
<https://sourceware.org/ml/libc-alpha/2014-06/msg00636.html> that
didn't depend on this patch.)
This patch removes conditionals on __ASSUME_O_CLOEXEC, and on
O_CLOEXEC being defined, in sysdeps/unix/sysv/linux/, now that
O_CLOEXEC support can be unconditionally assumed.
The patch is conservative in what it changes and further followup
cleanups may be possible. It may be possible to remove dl-opendir.c,
but the patch does not do so, just removing a redundant undefine and
redefine of __ASSUME_O_CLOEXEC. Also, __ASSUME_O_CLOEXEC is defined
unconditionally for Hurd as well as Linux. Thus, if we decide that
O_CLOEXEC support is a required feature of any glibc port, we could
remove __ASSUME_O_CLOEXEC and all conditionals on it throughout glibc,
rather than just cleaning up sysdeps/unix/sysv/linux/.
Tested x86_64 that the disassembly of installed shared libraries is
unchanged by this patch.
2014-06-25 Joseph Myers <joseph@codesourcery.com>
* sysdeps/unix/sysv/linux/dl-opendir.c (__ASSUME_O_CLOEXEC): Do
not undefine and redefine.
* sysdeps/unix/sysv/linux/getsysstats.c (__get_nprocs)
[O_CLOEXEC]: Make code unconditional.
(__get_nprocs) [!O_CLOEXEC]: Remove conditional code.
* sysdeps/unix/sysv/linux/shm_open.c: Do not include
<kernel-features.h>.
[O_CLOEXEC && !__ASSUME_O_CLOEXEC] (have_o_cloexec): Remove
conditional variable definition.
(shm_open) [O_CLOEXEC]: Make code unconditional.
(shm_open) [!O_CLOEXEC || !__ASSUME_O_CLOEXEC]: Remove conditional
code.
Comments
Currently I'm inclined against requiring O_CLOEXEC for all configurations.
> @@ -164,9 +157,7 @@ shm_open (const char *name, int oflag, mode_t mode)
> __mempcpy (__mempcpy (fname, mountpoint.dir, mountpoint.dirlen),
> name, namelen + 1);
>
> -#ifdef O_CLOEXEC
> oflag |= O_CLOEXEC;
> -#endif
Just move | O_CLOEXEC into the open call along with | O_NOFOLLOW.
Otherwise looks fine.
Thanks,
Roland
@@ -1,6 +1 @@
-/* In this implementation we do not really care whether the opened
- file descriptor has the CLOEXEC bit set. The only call happens
- long before there is a call to fork or exec. */
-#undef __ASSUME_O_CLOEXEC
-#define __ASSUME_O_CLOEXEC 1
#include <opendir.c>
@@ -142,11 +142,7 @@ __get_nprocs (void)
char *cp = buffer_end;
char *re = buffer_end;
-#ifdef O_CLOEXEC
const int flags = O_RDONLY | O_CLOEXEC;
-#else
- const int flags = O_RDONLY;
-#endif
int fd = open_not_cancel_2 ("/sys/devices/system/cpu/online", flags);
char *l;
int result = 0;
@@ -28,8 +28,6 @@
#include <bits/libc-lock.h>
#include "linux_fsinfo.h"
-#include <kernel-features.h>
-
/* Mount point of the shared memory filesystem. */
static struct
@@ -45,11 +43,6 @@ static const char defaultdir[] = "/dev/shm/";
__libc_once_define (static, once);
-#if defined O_CLOEXEC && !defined __ASSUME_O_CLOEXEC
-static bool have_o_cloexec;
-#endif
-
-
/* Determine where the shmfs is mounted (if at all). */
static void
where_is_shmfs (void)
@@ -164,9 +157,7 @@ shm_open (const char *name, int oflag, mode_t mode)
__mempcpy (__mempcpy (fname, mountpoint.dir, mountpoint.dirlen),
name, namelen + 1);
-#ifdef O_CLOEXEC
oflag |= O_CLOEXEC;
-#endif
/* And get the file descriptor.
XXX Maybe we should test each descriptor whether it really is for a
@@ -174,41 +165,7 @@ shm_open (const char *name, int oflag, mode_t mode)
should be revamped since we can determine whether shmfs is available
while trying to open the file, all in one turn. */
fd = open (fname, oflag | O_NOFOLLOW, mode);
- if (fd != -1)
- {
-#if !defined O_CLOEXEC || !defined __ASSUME_O_CLOEXEC
-# ifdef O_CLOEXEC
- if (have_o_cloexec <= 0)
-# endif
- {
- /* We got a descriptor. Now set the FD_CLOEXEC bit. */
- int flags = fcntl (fd, F_GETFD, 0);
-
- if (__builtin_expect (flags, 0) >= 0)
- {
-# ifdef O_CLOEXEC
- if (have_o_cloexec == 0)
- have_o_cloexec = (flags & FD_CLOEXEC) == 0 ? -1 : 1;
- if (have_o_cloexec < 0)
-# endif
- {
- flags |= FD_CLOEXEC;
- flags = fcntl (fd, F_SETFD, flags);
- }
- }
-
- if (flags == -1)
- {
- /* Something went wrong. We cannot return the descriptor. */
- int save_errno = errno;
- close (fd);
- fd = -1;
- __set_errno (save_errno);
- }
- }
-#endif
- }
- else if (__glibc_unlikely (errno == EISDIR))
+ if (fd == -1 && __glibc_unlikely (errno == EISDIR))
/* It might be better to fold this error with EINVAL since
directory names are just another example for unsuitable shared
object names and the standard does not mention EISDIR. */