From 3cc2aff1abff9e34f9fec282d970204dc1eab6f1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 8 Sep 2015 19:14:10 +0200 Subject: [PATCH] tree-wide: don't do assignments within if checks Turn this: if ((r = foo()) < 0) { ... into this: r = foo(); if (r < 0) { ... --- coccinelle/no-if-assignments.cocci | 20 ++++++++++++++++++++ src/ask-password/ask-password.c | 8 ++++++-- src/basic/fdset.c | 6 ++++-- src/core/execute.c | 18 ++++++++++++------ src/core/socket.c | 4 +++- src/core/transaction.c | 6 ++++-- src/initctl/initctl.c | 7 ++----- src/test/test-loopback.c | 3 ++- 8 files changed, 53 insertions(+), 19 deletions(-) create mode 100644 coccinelle/no-if-assignments.cocci diff --git a/coccinelle/no-if-assignments.cocci b/coccinelle/no-if-assignments.cocci new file mode 100644 index 0000000000..9f63e90337 --- /dev/null +++ b/coccinelle/no-if-assignments.cocci @@ -0,0 +1,20 @@ +@@ +expression p, q; +identifier r; +statement s; +@@ +- if ((r = q) < p) +- s ++ r = q; ++ if (r < p) ++ s +@@ +expression p, q; +identifier r; +statement s; +@@ +- if ((r = q) >= p) +- s ++ r = q; ++ if (r >= p) ++ s diff --git a/src/ask-password/ask-password.c b/src/ask-password/ask-password.c index 2cbed293ba..abfd545c79 100644 --- a/src/ask-password/ask-password.c +++ b/src/ask-password/ask-password.c @@ -156,7 +156,9 @@ int main(int argc, char *argv[]) { if (arg_use_tty && isatty(STDIN_FILENO)) { char *password = NULL; - if ((r = ask_password_tty(arg_message, timeout, arg_echo, NULL, &password)) >= 0) { + r = ask_password_tty(arg_message, timeout, arg_echo, NULL, + &password); + if (r >= 0) { puts(password); free(password); } @@ -164,7 +166,9 @@ int main(int argc, char *argv[]) { } else { char **l; - if ((r = ask_password_agent(arg_message, arg_icon, arg_id, timeout, arg_echo, arg_accept_cached, &l)) >= 0) { + r = ask_password_agent(arg_message, arg_icon, arg_id, timeout, + arg_echo, arg_accept_cached, &l); + if (r >= 0) { char **p; STRV_FOREACH(p, l) { diff --git a/src/basic/fdset.c b/src/basic/fdset.c index a4823e6659..d70fe156a2 100644 --- a/src/basic/fdset.c +++ b/src/basic/fdset.c @@ -201,9 +201,11 @@ int fdset_cloexec(FDSet *fds, bool b) { assert(fds); - SET_FOREACH(p, MAKE_SET(fds), i) - if ((r = fd_cloexec(PTR_TO_FD(p), b)) < 0) + SET_FOREACH(p, MAKE_SET(fds), i) { + r = fd_cloexec(PTR_TO_FD(p), b); + if (r < 0) return r; + } return 0; } diff --git a/src/core/execute.c b/src/core/execute.c index de5c16bd76..3e20130f0e 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -122,7 +122,8 @@ static int shift_fds(int fds[], unsigned n_fds) { if (fds[i] == i+3) continue; - if ((nfd = fcntl(fds[i], F_DUPFD, i+3)) < 0) + nfd = fcntl(fds[i], F_DUPFD, i + 3); + if (nfd < 0) return -errno; safe_close(fds[i]); @@ -156,14 +157,16 @@ static int flags_fds(const int fds[], unsigned n_fds, bool nonblock) { for (i = 0; i < n_fds; i++) { - if ((r = fd_nonblock(fds[i], nonblock)) < 0) + r = fd_nonblock(fds[i], nonblock); + if (r < 0) return r; /* We unconditionally drop FD_CLOEXEC from the fds, * since after all we want to pass these fds to our * children */ - if ((r = fd_cloexec(fds[i], false)) < 0) + r = fd_cloexec(fds[i], false); + if (r < 0) return r; } @@ -315,7 +318,8 @@ static int open_terminal_as(const char *path, mode_t mode, int nfd) { assert(path); assert(nfd >= 0); - if ((fd = open_terminal(path, mode | O_NOCTTY)) < 0) + fd = open_terminal(path, mode | O_NOCTTY); + if (fd < 0) return fd; if (fd != nfd) { @@ -629,7 +633,8 @@ static int enforce_groups(const ExecContext *context, const char *username, gid_ if (context->group) { const char *g = context->group; - if ((r = get_group_creds(&g, &gid)) < 0) + r = get_group_creds(&g, &gid); + if (r < 0) return r; } @@ -658,7 +663,8 @@ static int enforce_groups(const ExecContext *context, const char *username, gid_ return -ENOMEM; if (keep_groups) { - if ((k = getgroups(ngroups_max, gids)) < 0) { + k = getgroups(ngroups_max, gids); + if (k < 0) { free(gids); return -errno; } diff --git a/src/core/socket.c b/src/core/socket.c index 7d3d5eb78a..9db42a0333 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -982,7 +982,9 @@ static int fifo_address_create( goto fail; } - if ((fd = open(path, O_RDWR|O_CLOEXEC|O_NOCTTY|O_NONBLOCK|O_NOFOLLOW)) < 0) { + fd = open(path, + O_RDWR | O_CLOEXEC | O_NOCTTY | O_NONBLOCK | O_NOFOLLOW); + if (fd < 0) { r = -errno; goto fail; } diff --git a/src/core/transaction.c b/src/core/transaction.c index 090103fbda..b505297e23 100644 --- a/src/core/transaction.c +++ b/src/core/transaction.c @@ -464,9 +464,11 @@ static int transaction_verify_order(Transaction *tr, unsigned *generation, sd_bu g = (*generation)++; - HASHMAP_FOREACH(j, tr->jobs, i) - if ((r = transaction_verify_order_one(tr, j, NULL, g, e)) < 0) + HASHMAP_FOREACH(j, tr->jobs, i) { + r = transaction_verify_order_one(tr, j, NULL, g, e); + if (r < 0) return r; + } return 0; } diff --git a/src/initctl/initctl.c b/src/initctl/initctl.c index 19d6468fcc..087cc2f7d6 100644 --- a/src/initctl/initctl.c +++ b/src/initctl/initctl.c @@ -399,13 +399,10 @@ int main(int argc, char *argv[]) { struct epoll_event event; int k; - if ((k = epoll_wait(server.epoll_fd, - &event, 1, - TIMEOUT_MSEC)) < 0) { - + k = epoll_wait(server.epoll_fd, &event, 1, TIMEOUT_MSEC); + if (k < 0) { if (errno == EINTR) continue; - log_error_errno(errno, "epoll_wait() failed: %m"); goto fail; } diff --git a/src/test/test-loopback.c b/src/test/test-loopback.c index c03bda4382..e3e5a95add 100644 --- a/src/test/test-loopback.c +++ b/src/test/test-loopback.c @@ -31,7 +31,8 @@ int main(int argc, char* argv[]) { log_open(); log_parse_environment(); - if ((r = loopback_setup()) < 0) + r = loopback_setup(); + if (r < 0) fprintf(stderr, "loopback: %s\n", strerror(-r)); return 0;