[systemd-devel] [PATCH] tree-wide: Introduce a dup_cloexec inline helper
Cristian RodrÃguez
crrodriguez at opensuse.org
Wed Apr 22 12:29:11 PDT 2015
- Nicer & easier to remember than fcntl(fd, F_DUPFD_CLOEXEC, 3)
- Update CODING_STYLE
- Use it tree-wide
---
CODING_STYLE | 6 +++---
src/import/curl-util.c | 2 +-
src/import/importd.c | 4 ++--
src/journal/cat.c | 2 +-
src/libsystemd-terminal/grdev-drm.c | 4 ++--
src/libsystemd-terminal/idev-evdev.c | 4 ++--
src/libsystemd/sd-bus/bus-message.c | 6 +++---
src/login/inhibit.c | 2 +-
src/login/pam_systemd.c | 2 +-
src/run/run.c | 2 +-
src/shared/copy.c | 2 +-
src/shared/fdset.c | 2 +-
src/shared/install.c | 2 +-
src/shared/util.h | 5 +++++
14 files changed, 25 insertions(+), 20 deletions(-)
diff --git a/CODING_STYLE b/CODING_STYLE
index a295ca7..19891ce 100644
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -233,12 +233,12 @@
fork()ed off a child process, please use _exit() instead of exit(),
so that the exit handlers are not run.
-- Please never use dup(). Use fcntl(fd, F_DUPFD_CLOEXEC, 3)
- instead. For two reason: first, you want O_CLOEXEC set on the new fd
+- Please never use dup(). Use the dup_cloexec() helper instead.
+ For two reasons: first, you want O_CLOEXEC set on the new fd
(see above). Second, dup() will happily duplicate your fd as 0, 1,
2, i.e. stdin, stdout, stderr, should those fds be closed. Given the
special semantics of those fds, it's probably a good idea to avoid
- them. F_DUPFD_CLOEXEC with "3" as parameter avoids them.
+ them.
- When you define a destructor or unref() call for an object, please
accept a NULL object and simply treat this as NOP. This is similar
diff --git a/src/import/curl-util.c b/src/import/curl-util.c
index d390cfb..dfa5acc 100644
--- a/src/import/curl-util.c
+++ b/src/import/curl-util.c
@@ -131,7 +131,7 @@ static int curl_glue_socket_callback(CURLM *curl, curl_socket_t s, int action, v
* anymore. Hence, duplicate the fds here, and keep a
* copy for epoll which we control after use. */
- fd = fcntl(s, F_DUPFD_CLOEXEC, 3);
+ fd = dup_cloexec(s);
if (fd < 0)
return -1;
diff --git a/src/import/importd.c b/src/import/importd.c
index d4da4b2..416a43c 100644
--- a/src/import/importd.c
+++ b/src/import/importd.c
@@ -765,7 +765,7 @@ static int method_import_tar_or_raw(sd_bus *bus, sd_bus_message *msg, void *user
if (!t->local)
return -ENOMEM;
- t->stdin_fd = fcntl(fd, F_DUPFD_CLOEXEC, 3);
+ t->stdin_fd = dup_cloexec(fd);
if (t->stdin_fd < 0)
return -errno;
@@ -826,7 +826,7 @@ static int method_export_tar_or_raw(sd_bus *bus, sd_bus_message *msg, void *user
if (!t->local)
return -ENOMEM;
- t->stdout_fd = fcntl(fd, F_DUPFD_CLOEXEC, 3);
+ t->stdout_fd = dup_cloexec(fd);
if (t->stdout_fd < 0)
return -errno;
diff --git a/src/journal/cat.c b/src/journal/cat.c
index 2e236f0..9e26e7c 100644
--- a/src/journal/cat.c
+++ b/src/journal/cat.c
@@ -138,7 +138,7 @@ int main(int argc, char *argv[]) {
goto finish;
}
- saved_stderr = fcntl(STDERR_FILENO, F_DUPFD_CLOEXEC, 3);
+ saved_stderr = dup_cloexec(STDERR_FILENO);
if (dup3(fd, STDOUT_FILENO, 0) < 0 ||
dup3(fd, STDERR_FILENO, 0) < 0) {
diff --git a/src/libsystemd-terminal/grdev-drm.c b/src/libsystemd-terminal/grdev-drm.c
index 066a4d8..deb7e94 100644
--- a/src/libsystemd-terminal/grdev-drm.c
+++ b/src/libsystemd-terminal/grdev-drm.c
@@ -2782,7 +2782,7 @@ static int managed_card_resume_device_fn(sd_bus *bus,
* TakeDevice(). However, lets be safe and use this FD in case
* we really don't have one. There is no harm in doing this
* and our code works fine this way. */
- fd = fcntl(fd, F_DUPFD_CLOEXEC, 3);
+ fd = dup_cloexec(fd);
if (fd < 0) {
log_debug_errno(errno, "grdrm: %s/%s: cannot duplicate fd: %m",
session->name, cm->card.base.name);
@@ -2874,7 +2874,7 @@ static int managed_card_take_device_fn(sd_bus *bus,
return 0;
}
- fd = fcntl(fd, F_DUPFD_CLOEXEC, 3);
+ fd = dup_cloexec(fd);
if (fd < 0) {
log_debug_errno(errno, "grdrm: %s/%s: cannot duplicate fd: %m",
session->name, cm->card.base.name);
diff --git a/src/libsystemd-terminal/idev-evdev.c b/src/libsystemd-terminal/idev-evdev.c
index 91ae507..c49ba90 100644
--- a/src/libsystemd-terminal/idev-evdev.c
+++ b/src/libsystemd-terminal/idev-evdev.c
@@ -559,7 +559,7 @@ static int managed_evdev_take_device_fn(sd_bus *bus,
if (paused)
return 0;
- fd = fcntl(fd, F_DUPFD_CLOEXEC, 3);
+ fd = dup_cloexec(fd);
if (fd < 0) {
log_debug_errno(errno, "idev-evdev: %s/%s: cannot duplicate evdev fd: %m", s->name, e->name);
return 0;
@@ -692,7 +692,7 @@ static void managed_evdev_resume(idev_element *e, int fd) {
* device.
*/
- fd = fcntl(fd, F_DUPFD_CLOEXEC, 3);
+ fd = dup_cloexec(fd);
if (fd < 0) {
log_debug_errno(errno, "idev-evdev: %s/%s: cannot duplicate evdev fd: %m",
s->name, e->name);
diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c
index 6ee209d..c4ca09d 100644
--- a/src/libsystemd/sd-bus/bus-message.c
+++ b/src/libsystemd/sd-bus/bus-message.c
@@ -1465,7 +1465,7 @@ static int message_push_fd(sd_bus_message *m, int fd) {
if (!m->allow_fds)
return -EOPNOTSUPP;
- copy = fcntl(fd, F_DUPFD_CLOEXEC, 3);
+ copy = dup_cloexec(fd);
if (copy < 0)
return -errno;
@@ -2694,7 +2694,7 @@ _public_ int sd_bus_message_append_array_memfd(
if (r < 0)
return r;
- copy_fd = dup(memfd);
+ copy_fd = dup_cloexec(memfd);
if (copy_fd < 0)
return copy_fd;
@@ -2769,7 +2769,7 @@ _public_ int sd_bus_message_append_string_memfd(
if (r < 0)
return r;
- copy_fd = dup(memfd);
+ copy_fd = dup_cloexec(memfd);
if (copy_fd < 0)
return copy_fd;
diff --git a/src/login/inhibit.c b/src/login/inhibit.c
index 57cfb5d..f273892 100644
--- a/src/login/inhibit.c
+++ b/src/login/inhibit.c
@@ -65,7 +65,7 @@ static int inhibit(sd_bus *bus, sd_bus_error *error) {
if (r < 0)
return r;
- r = fcntl(fd, F_DUPFD_CLOEXEC, 3);
+ r = dup_cloexec(fd);
if (r < 0)
return -errno;
diff --git a/src/login/pam_systemd.c b/src/login/pam_systemd.c
index 1416701..46c9c31 100644
--- a/src/login/pam_systemd.c
+++ b/src/login/pam_systemd.c
@@ -473,7 +473,7 @@ _public_ PAM_EXTERN int pam_sm_open_session(
}
if (session_fd >= 0) {
- session_fd = fcntl(session_fd, F_DUPFD_CLOEXEC, 3);
+ session_fd = dup_cloexec(session_fd);
if (session_fd < 0) {
pam_syslog(handle, LOG_ERR, "Failed to dup session fd: %m");
return PAM_SESSION_ERR;
diff --git a/src/run/run.c b/src/run/run.c
index 156824e..302c736 100644
--- a/src/run/run.c
+++ b/src/run/run.c
@@ -697,7 +697,7 @@ static int start_transient_service(
if (r < 0)
return bus_log_parse_error(r);
- master = fcntl(master, F_DUPFD_CLOEXEC, 3);
+ master = dup_cloexec(master);
if (master < 0)
return log_error_errno(errno, "Failed to duplicate master fd: %m");
diff --git a/src/shared/copy.c b/src/shared/copy.c
index 1282cb8..7faf6c4 100644
--- a/src/shared/copy.c
+++ b/src/shared/copy.c
@@ -225,7 +225,7 @@ static int fd_copy_directory(
if (from)
fdf = openat(df, from, O_RDONLY|O_DIRECTORY|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW);
else
- fdf = fcntl(df, F_DUPFD_CLOEXEC, 3);
+ fdf = dup_cloexec(df);
d = fdopendir(fdf);
if (!d)
diff --git a/src/shared/fdset.c b/src/shared/fdset.c
index 3184927..515c6c6 100644
--- a/src/shared/fdset.c
+++ b/src/shared/fdset.c
@@ -113,7 +113,7 @@ int fdset_put_dup(FDSet *s, int fd) {
assert(s);
assert(fd >= 0);
- copy = fcntl(fd, F_DUPFD_CLOEXEC, 3);
+ copy = dup_cloexec(fd);
if (copy < 0)
return -errno;
diff --git a/src/shared/install.c b/src/shared/install.c
index b121018..1e6854a 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -350,7 +350,7 @@ static int remove_marked_symlinks(
int q, cfd;
deleted = false;
- cfd = fcntl(fd, F_DUPFD_CLOEXEC, 3);
+ cfd = dup_cloexec(fd);
if (cfd < 0) {
r = -errno;
break;
diff --git a/src/shared/util.h b/src/shared/util.h
index 9409ad9..c290410 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -912,3 +912,8 @@ int rename_noreplace(int olddirfd, const char *oldpath, int newdirfd, const char
char *shell_maybe_quote(const char *s);
int parse_mode(const char *s, mode_t *ret);
+
+static inline int dup_cloexec(int oldfd)
+{
+ return fcntl(oldfd, F_DUPFD_CLOEXEC, STDERR_FILENO + 1);
+}
--
2.3.5
More information about the systemd-devel
mailing list