From 31d31f20218aabe63cd714da7f5f2fca94c98ef4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 20 Apr 2018 15:07:42 +0200 Subject: [PATCH 1/4] coccinelle: run spatch just on version-controlled files Also, allow run-cocinnelle.sh to be started from any directory. Unfortunately set -x does not work nicely anymore, because the list is too verbose. Replace it by an echo line. --- coccinelle/run-coccinelle.sh | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/coccinelle/run-coccinelle.sh b/coccinelle/run-coccinelle.sh index f463f5eed5..8c4ed07dd8 100755 --- a/coccinelle/run-coccinelle.sh +++ b/coccinelle/run-coccinelle.sh @@ -1,9 +1,12 @@ #!/bin/bash -e -for SCRIPT in ${@-*.cocci} ; do +files="$(git ls-files ':/*.[ch]')" + +for SCRIPT in ${@-coccinelle/*.cocci} ; do echo "--x-- Processing $SCRIPT --x--" TMPFILE=`mktemp` - ( set -x ; spatch --sp-file $SCRIPT --dir $PWD/.. 2> "$TMPFILE" || cat "$TMPFILE" ) + echo "+ spatch --sp-file $SCRIPT ..." + spatch --sp-file $SCRIPT $files 2>"$TMPFILE" || cat "$TMPFILE" rm "$TMPFILE" echo -e "--x-- Processed $SCRIPT --x--\n" done From 00bfe67f6b178056bffcfa9fbfb04ca3fa809989 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 20 Apr 2018 15:21:06 +0200 Subject: [PATCH 2/4] coccinelle: add option to make changes in place --- coccinelle/run-coccinelle.sh | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/coccinelle/run-coccinelle.sh b/coccinelle/run-coccinelle.sh index 8c4ed07dd8..4c8c08227b 100755 --- a/coccinelle/run-coccinelle.sh +++ b/coccinelle/run-coccinelle.sh @@ -1,12 +1,20 @@ #!/bin/bash -e files="$(git ls-files ':/*.[ch]')" +args= + +case "$1" in + -i) + args="$args --in-place" + shift + ;; +esac for SCRIPT in ${@-coccinelle/*.cocci} ; do echo "--x-- Processing $SCRIPT --x--" TMPFILE=`mktemp` - echo "+ spatch --sp-file $SCRIPT ..." - spatch --sp-file $SCRIPT $files 2>"$TMPFILE" || cat "$TMPFILE" + echo "+ spatch --sp-file $SCRIPT $args ..." + spatch --sp-file $SCRIPT $args $files 2>"$TMPFILE" || cat "$TMPFILE" rm "$TMPFILE" echo -e "--x-- Processed $SCRIPT --x--\n" done From d94a24ca2ea769755beaed0659b966b1ec75c8d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 20 Apr 2018 15:36:20 +0200 Subject: [PATCH 3/4] Add macro for checking if some flags are set This way we don't need to repeat the argument twice. I didn't replace all instances. I think it's better to leave out: - asserts - comparisons like x & y == x, which are mathematically equivalent, but here we aren't checking if flags are set, but if the argument fits in the flags. --- coccinelle/flags-set.cocci | 15 +++++++++++++++ src/basic/capability-util.h | 2 +- src/basic/copy.c | 2 +- src/basic/fs-util.c | 4 ++-- src/basic/macro.h | 2 ++ src/basic/process-util.c | 2 +- src/basic/rm-rf.c | 2 +- src/core/cgroup.c | 2 +- src/core/unit.c | 6 +++--- src/import/curl-util.c | 2 +- src/shared/condition.c | 2 +- src/udev/udev-builtin-input_id.c | 2 +- 12 files changed, 30 insertions(+), 13 deletions(-) create mode 100644 coccinelle/flags-set.cocci diff --git a/coccinelle/flags-set.cocci b/coccinelle/flags-set.cocci new file mode 100644 index 0000000000..1a70717e76 --- /dev/null +++ b/coccinelle/flags-set.cocci @@ -0,0 +1,15 @@ +@@ +expression x, y; +@@ +- ((x) & (y)) == (y) ++ FLAGS_SET(x, y) +@@ +expression x, y; +@@ +- (x & (y)) == (y) ++ FLAGS_SET(x, y) +@@ +expression x, y; +@@ +- ((x) & y) == y ++ FLAGS_SET(x, y) diff --git a/src/basic/capability-util.h b/src/basic/capability-util.h index f90efe2550..7e51791681 100644 --- a/src/basic/capability-util.h +++ b/src/basic/capability-util.h @@ -41,7 +41,7 @@ static inline void cap_free_charpp(char **p) { static inline bool cap_test_all(uint64_t caps) { uint64_t m; m = (UINT64_C(1) << (cap_last_cap() + 1)) - 1; - return (caps & m) == m; + return FLAGS_SET(caps, m); } bool ambient_capabilities_supported(void); diff --git a/src/basic/copy.c b/src/basic/copy.c index 5b6cae39d0..650de612b8 100644 --- a/src/basic/copy.c +++ b/src/basic/copy.c @@ -81,7 +81,7 @@ static int fd_is_nonblock_pipe(int fd) { if (flags < 0) return -errno; - return (flags & O_NONBLOCK) == O_NONBLOCK ? FD_IS_NONBLOCKING_PIPE : FD_IS_BLOCKING_PIPE; + return FLAGS_SET(flags, O_NONBLOCK) ? FD_IS_NONBLOCKING_PIPE : FD_IS_BLOCKING_PIPE; } int copy_bytes_full( diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index a0d7a43d27..6146c66782 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -602,10 +602,10 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags, assert(path); /* Either the file may be missing, or we return an fd to the final object, but both make no sense */ - if ((flags & (CHASE_NONEXISTENT|CHASE_OPEN)) == (CHASE_NONEXISTENT|CHASE_OPEN)) + if (FLAGS_SET(flags, CHASE_NONEXISTENT | CHASE_OPEN)) return -EINVAL; - if ((flags & (CHASE_STEP|CHASE_OPEN)) == (CHASE_STEP|CHASE_OPEN)) + if (FLAGS_SET(flags, CHASE_STEP | CHASE_OPEN)) return -EINVAL; if (isempty(path)) diff --git a/src/basic/macro.h b/src/basic/macro.h index cd3ae8d3ab..05f5e9fa68 100644 --- a/src/basic/macro.h +++ b/src/basic/macro.h @@ -357,6 +357,8 @@ static inline unsigned long ALIGN_POWER2(unsigned long u) { #define SET_FLAG(v, flag, b) \ (v) = (b) ? ((v) | (flag)) : ((v) & ~(flag)) +#define FLAGS_SET(v, flags) \ + (((v) & (flags)) == (flags)) #define CASE_F(X) case X: #define CASE_F_1(CASE, X) CASE_F(X) diff --git a/src/basic/process-util.c b/src/basic/process-util.c index e3be7b9793..61ab181e6f 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -1359,7 +1359,7 @@ int safe_fork_full( } } - if ((flags & (FORK_NEW_MOUNTNS|FORK_MOUNTNS_SLAVE)) == (FORK_NEW_MOUNTNS|FORK_MOUNTNS_SLAVE)) { + if (FLAGS_SET(flags, FORK_NEW_MOUNTNS | FORK_MOUNTNS_SLAVE)) { /* Optionally, make sure we never propagate mounts to the host. */ diff --git a/src/basic/rm-rf.c b/src/basic/rm-rf.c index 91474ad95c..20f094b9a1 100644 --- a/src/basic/rm-rf.c +++ b/src/basic/rm-rf.c @@ -178,7 +178,7 @@ int rm_rf(const char *path, RemoveFlags flags) { return -EPERM; } - if ((flags & (REMOVE_SUBVOLUME|REMOVE_ROOT|REMOVE_PHYSICAL)) == (REMOVE_SUBVOLUME|REMOVE_ROOT|REMOVE_PHYSICAL)) { + if (FLAGS_SET(flags, REMOVE_SUBVOLUME | REMOVE_ROOT | REMOVE_PHYSICAL)) { /* Try to remove as subvolume first */ r = btrfs_subvol_remove(path, BTRFS_REMOVE_RECURSIVE|BTRFS_REMOVE_QUOTA); if (r >= 0) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index b3f8600eb4..3927d6df85 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -1303,7 +1303,7 @@ const char *unit_get_realized_cgroup_path(Unit *u, CGroupMask mask) { if (u->cgroup_path && u->cgroup_realized && - (u->cgroup_realized_mask & mask) == mask) + FLAGS_SET(u->cgroup_realized_mask, mask)) return u->cgroup_path; u = UNIT_DEREF(u->slice); diff --git a/src/core/unit.c b/src/core/unit.c index fd0d8da3aa..0c49f6115e 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -1053,7 +1053,7 @@ static void print_unit_dependency_mask(FILE *f, const char *kind, UnitDependency if (mask == 0) break; - if ((mask & table[i].mask) == table[i].mask) { + if (FLAGS_SET(mask, table[i].mask)) { if (*space) fputc(' ', f); else @@ -2695,8 +2695,8 @@ static int unit_add_dependency_hashmap( if (info.data) { /* Entry already exists. Add in our mask. */ - if ((info.origin_mask & origin_mask) == info.origin_mask && - (info.destination_mask & destination_mask) == info.destination_mask) + if (FLAGS_SET(origin_mask, info.origin_mask) && + FLAGS_SET(destination_mask, info.destination_mask)) return 0; /* NOP */ info.origin_mask |= origin_mask; diff --git a/src/import/curl-util.c b/src/import/curl-util.c index 2fba6e29dd..c19a01a266 100644 --- a/src/import/curl-util.c +++ b/src/import/curl-util.c @@ -37,7 +37,7 @@ static int curl_glue_on_io(sd_event_source *s, int fd, uint32_t revents, void *u translated_fd = PTR_TO_FD(hashmap_get(g->translate_fds, FD_TO_PTR(fd))); - if ((revents & (EPOLLIN|EPOLLOUT)) == (EPOLLIN|EPOLLOUT)) + if (FLAGS_SET(revents, EPOLLIN | EPOLLOUT)) action = CURL_POLL_INOUT; else if (revents & EPOLLIN) action = CURL_POLL_IN; diff --git a/src/shared/condition.c b/src/shared/condition.c index bf6476c339..a41a782664 100644 --- a/src/shared/condition.c +++ b/src/shared/condition.c @@ -251,7 +251,7 @@ static int condition_test_control_group_controller(Condition *c) { return 1; } - return (system_mask & wanted_mask) == wanted_mask; + return FLAGS_SET(system_mask, wanted_mask); } static int condition_test_group(Condition *c) { diff --git a/src/udev/udev-builtin-input_id.c b/src/udev/udev-builtin-input_id.c index 4d5b1233f1..f80a6a595c 100644 --- a/src/udev/udev-builtin-input_id.c +++ b/src/udev/udev-builtin-input_id.c @@ -307,7 +307,7 @@ static bool test_key(struct udev_device *dev, /* the first 32 bits are ESC, numbers, and Q to D; if we have all of * those, consider it a full keyboard; do not test KEY_RESERVED, though */ mask = 0xFFFFFFFE; - if ((bitmask_key[0] & mask) == mask) { + if (FLAGS_SET(bitmask_key[0], mask)) { udev_builtin_add_property(dev, test, "ID_INPUT_KEYBOARD", "1"); ret = true; } From 5c270a18da43d1a6a3488c38efc05c64345b1c21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 4 Jun 2018 10:29:23 +0200 Subject: [PATCH 4/4] basic/path-util: use FLAGS_SET in one more place --- src/basic/path-util.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/basic/path-util.c b/src/basic/path-util.c index 690159d89c..547b38e2a9 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -999,11 +999,9 @@ int path_simplify_and_warn( unsigned line, const char *lvalue) { - bool fatal, absolute; + bool absolute, fatal = flag & PATH_CHECK_FATAL; - assert((flag & (PATH_CHECK_ABSOLUTE | PATH_CHECK_RELATIVE)) != (PATH_CHECK_ABSOLUTE | PATH_CHECK_RELATIVE)); - - fatal = flag & PATH_CHECK_FATAL; + assert(!FLAGS_SET(flag, PATH_CHECK_ABSOLUTE | PATH_CHECK_RELATIVE)); if (!utf8_is_valid(path)) { log_syntax_invalid_utf8(unit, LOG_ERR, filename, line, path);