From b3fd7b53ffcdb3d4ed4adaa42c73762a26cc6df8 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Fri, 26 Apr 2019 21:42:16 +0200 Subject: [PATCH 01/10] coccinelle: add explicit statement isomorphisms Coccinelle needs a custom isomorphism file with rules (isomorphisms) how to correctly rewrite conditions with explicit NULL checks (i.e. if (ptr == NULL)) to their shorter form (i.e. if (!ptr)). Coccinelle already contains such isomorphisms in its default .iso file, however, they're in the opposite direction, which results in useless output from coccinelle/equals-null.cocci. With this fix, `spatch` should no longer report patches like: @@ -628,8 +628,9 @@ static int path_deserialize_item(Unit *u f = path_result_from_string(value); if (f < 0) log_unit_debug(u, "Failed to parse result value: %s", value); - else if (f != PATH_SUCCESS) - p->result = f; + else {if (f != PATH_SUCCESS) + p->result = f; + } } else log_unit_debug(u, "Unknown serialization key: %s", key); --- coccinelle/run-coccinelle.sh | 3 ++- coccinelle/systemd-definitions.iso | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 coccinelle/systemd-definitions.iso diff --git a/coccinelle/run-coccinelle.sh b/coccinelle/run-coccinelle.sh index 520de0ac42..4d882112e6 100755 --- a/coccinelle/run-coccinelle.sh +++ b/coccinelle/run-coccinelle.sh @@ -2,6 +2,7 @@ top="$(git rev-parse --show-toplevel)" files="$(git ls-files ':/*.[ch]')" +iso_defs="$top/coccinelle/systemd-definitions.iso" args= case "$1" in @@ -21,7 +22,7 @@ for SCRIPT in ${@-$top/coccinelle/*.cocci} ; do TMPFILE=`mktemp` echo "+ spatch --sp-file $SCRIPT $args ..." parallel --halt now,fail=1 --keep-order --noswap --max-args=20 \ - spatch --sp-file $SCRIPT $args ::: $files \ + spatch --iso-file $iso_defs --sp-file $SCRIPT $args ::: $files \ 2>"$TMPFILE" || cat "$TMPFILE" echo -e "--x-- Processed $SCRIPT --x--\n" done diff --git a/coccinelle/systemd-definitions.iso b/coccinelle/systemd-definitions.iso new file mode 100644 index 0000000000..92db763a29 --- /dev/null +++ b/coccinelle/systemd-definitions.iso @@ -0,0 +1,20 @@ +/* Statement isomorphisms - replace explicit checks against NULL with a + * shorter variant, which relies on C's downgrade-to-bool feature. + * The expression metavariables should be declared as pointers, however, + * that doesn't work well with complex expressions like: + * if (UNIT(p)->default_dependencies != NULL) + */ + +Statement +@@ +expression X; +statement S; +@@ +if (X == NULL) S => if (!X) S + +Statement +@@ +expression X; +statement S; +@@ +if (X != NULL) S => if (X) S From 17e3e37c059aca1db9946675dd3dd9ae1454b0de Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Sat, 27 Apr 2019 12:26:22 +0200 Subject: [PATCH 02/10] coccinelle: avoid matching 'errno' as a file descriptor The `coccinelle/take-fd.cocci` transformation file attempts to rewrite r = fd; fd = -1; to r = TAKE_FD(fd); Unfortunately, using `identifier` or `idexpression` as a metavariable type in this case wouldn't match more complex location descriptions, like: x->fd = fd fd = -1; Using 'expression' metavariable type generates false positives, as you can't specify scope of such expression. The only real example from the current codebase is the global 'errno' variable, which results in following patch generated by `spatch`: --- src/basic/errno-util.h +++ /tmp/cocci-output-28263-971baa-errno-util.h @@ -15,8 +15,7 @@ static inline void _reset_errno_(int *sa #define UNPROTECT_ERRNO \ do { \ - errno = _saved_errno_; \ - _saved_errno_ = -1; \ + errno = TAKE_FD(_saved_errno_); \ } while (false) static inline int negative_errno(void) { Let's explicitly state that the matched expression should not equal 'errno' to avoid this. It's not particularly nice, but it should be enough, at least for now. --- coccinelle/take-fd.cocci | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/coccinelle/take-fd.cocci b/coccinelle/take-fd.cocci index ba242483cd..f7124e7896 100644 --- a/coccinelle/take-fd.cocci +++ b/coccinelle/take-fd.cocci @@ -6,8 +6,15 @@ expression q; - q = -1; - return p; + return TAKE_FD(q); + +/* The ideal solution would use 'local idexpression' to avoid matching errno, + * which is a global variable. However, 'idexpression' nor 'identifier' + * would match, for example, "x->fd", which is considered 'expression' in + * the SmPL grammar + */ @@ -expression p, q; +expression p != errno; +expression q; @@ - p = q; - q = -1; From 4e361acc062bee3d6d9a41243c3e4e7396343fe6 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Sun, 28 Apr 2019 14:28:49 +0200 Subject: [PATCH 03/10] tree-wide: replace explicit NULL checks with their shorter variants Done by coccinelle/equals-null.cocci --- src/journal-remote/journal-remote.c | 2 +- src/libsystemd-network/network-internal.c | 2 +- src/libsystemd/sd-bus/bus-message.c | 2 +- src/network/networkd-dhcp6.c | 2 +- src/network/networkd-radv.c | 6 +++--- src/shared/journal-importer.c | 2 +- src/test/test-libudev.c | 16 ++++++++-------- src/test/test-nss.c | 2 +- src/tmpfiles/tmpfiles.c | 2 +- src/udev/ata_id/ata_id.c | 4 ++-- src/udev/cdrom_id/cdrom_id.c | 4 ++-- src/udev/scsi_id/scsi_id.c | 6 +++--- src/udev/scsi_id/scsi_serial.c | 6 +++--- src/udev/udev-builtin-usb_id.c | 2 +- src/udev/v4l_id/v4l_id.c | 2 +- 15 files changed, 30 insertions(+), 30 deletions(-) diff --git a/src/journal-remote/journal-remote.c b/src/journal-remote/journal-remote.c index 04c66a29ce..35efe7002b 100644 --- a/src/journal-remote/journal-remote.c +++ b/src/journal-remote/journal-remote.c @@ -181,7 +181,7 @@ static int get_source_for_fd(RemoteServer *s, return log_warning_errno(r, "Failed to get writer for source %s: %m", name); - if (s->sources[fd] == NULL) { + if (!s->sources[fd]) { s->sources[fd] = source_new(fd, false, name, writer); if (!s->sources[fd]) { writer_unref(writer); diff --git a/src/libsystemd-network/network-internal.c b/src/libsystemd-network/network-internal.c index 2154cf7eac..a112b9f70f 100644 --- a/src/libsystemd-network/network-internal.c +++ b/src/libsystemd-network/network-internal.c @@ -612,7 +612,7 @@ int serialize_dhcp_option(FILE *f, const char *key, const void *data, size_t siz assert(data); hex_buf = hexmem(data, size); - if (hex_buf == NULL) + if (!hex_buf) return -ENOMEM; fprintf(f, "%s=%s\n", key, hex_buf); diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index 427d42f296..d857ca121f 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -26,7 +26,7 @@ static int message_append_basic(sd_bus_message *m, char type, const void *p, con static void *adjust_pointer(const void *p, void *old_base, size_t sz, void *new_base) { - if (p == NULL) + if (!p) return NULL; if (old_base == new_base) diff --git a/src/network/networkd-dhcp6.c b/src/network/networkd-dhcp6.c index 90361c9f4a..ba795decfe 100644 --- a/src/network/networkd-dhcp6.c +++ b/src/network/networkd-dhcp6.c @@ -229,7 +229,7 @@ static int dhcp6_pd_prefix_distribute(Link *dhcp6_link, Iterator *i, strnull(assigned_buf), strnull(buf), pd_prefix_len); - if (assigned_link == NULL) + if (!assigned_link) continue; } else diff --git a/src/network/networkd-radv.c b/src/network/networkd-radv.c index 8cb14b588c..fdbf7cac62 100644 --- a/src/network/networkd-radv.c +++ b/src/network/networkd-radv.c @@ -356,7 +356,7 @@ static int radv_set_dns(Link *link, Link *uplink) { if (link->network->router_dns) { dns = newdup(struct in6_addr, link->network->router_dns, link->network->n_router_dns); - if (dns == NULL) + if (!dns) return -ENOMEM; n_dns = link->network->n_router_dns; @@ -372,7 +372,7 @@ static int radv_set_dns(Link *link, Link *uplink) { goto set_dns; if (uplink) { - if (uplink->network == NULL) { + if (!uplink->network) { log_link_debug(uplink, "Cannot fetch DNS servers as uplink interface is not managed by us"); return 0; } @@ -411,7 +411,7 @@ static int radv_set_domains(Link *link, Link *uplink) { goto set_domains; if (uplink) { - if (uplink->network == NULL) { + if (!uplink->network) { log_link_debug(uplink, "Cannot fetch DNS search domains as uplink interface is not managed by us"); return 0; } diff --git a/src/shared/journal-importer.c b/src/shared/journal-importer.c index 8638cd3cc9..8dc2c42ad1 100644 --- a/src/shared/journal-importer.c +++ b/src/shared/journal-importer.c @@ -94,7 +94,7 @@ static int get_line(JournalImporter *imp, char **line, size_t *size) { c = memchr(imp->buf + start, '\n', imp->filled - start); - if (c != NULL) + if (c) break; } diff --git a/src/test/test-libudev.c b/src/test/test-libudev.c index f634ca28db..7878512465 100644 --- a/src/test/test-libudev.c +++ b/src/test/test-libudev.c @@ -158,7 +158,7 @@ static int enumerate_print_list(struct udev_enumerate *enumerate) { device = udev_device_new_from_syspath(udev_enumerate_get_udev(enumerate), udev_list_entry_get_name(list_entry)); - if (device != NULL) { + if (device) { log_info("device: '%s' (%s)", udev_device_get_syspath(device), udev_device_get_subsystem(device)); @@ -249,7 +249,7 @@ static int test_enumerate(struct udev *udev, const char *subsystem) { log_info("enumerate '%s'", subsystem == NULL ? "" : subsystem); udev_enumerate = udev_enumerate_new(udev); - if (udev_enumerate == NULL) + if (!udev_enumerate) return -1; udev_enumerate_add_match_subsystem(udev_enumerate, subsystem); udev_enumerate_scan_devices(udev_enumerate); @@ -258,7 +258,7 @@ static int test_enumerate(struct udev *udev, const char *subsystem) { log_info("enumerate 'net' + duplicated scan + null + zero"); udev_enumerate = udev_enumerate_new(udev); - if (udev_enumerate == NULL) + if (!udev_enumerate) return -1; udev_enumerate_add_match_subsystem(udev_enumerate, "net"); udev_enumerate_scan_devices(udev_enumerate); @@ -278,7 +278,7 @@ static int test_enumerate(struct udev *udev, const char *subsystem) { log_info("enumerate 'block'"); udev_enumerate = udev_enumerate_new(udev); - if (udev_enumerate == NULL) + if (!udev_enumerate) return -1; udev_enumerate_add_match_subsystem(udev_enumerate,"block"); r = udev_enumerate_add_match_is_initialized(udev_enumerate); @@ -292,7 +292,7 @@ static int test_enumerate(struct udev *udev, const char *subsystem) { log_info("enumerate 'not block'"); udev_enumerate = udev_enumerate_new(udev); - if (udev_enumerate == NULL) + if (!udev_enumerate) return -1; udev_enumerate_add_nomatch_subsystem(udev_enumerate, "block"); udev_enumerate_scan_devices(udev_enumerate); @@ -301,7 +301,7 @@ static int test_enumerate(struct udev *udev, const char *subsystem) { log_info("enumerate 'pci, mem, vc'"); udev_enumerate = udev_enumerate_new(udev); - if (udev_enumerate == NULL) + if (!udev_enumerate) return -1; udev_enumerate_add_match_subsystem(udev_enumerate, "pci"); udev_enumerate_add_match_subsystem(udev_enumerate, "mem"); @@ -312,7 +312,7 @@ static int test_enumerate(struct udev *udev, const char *subsystem) { log_info("enumerate 'subsystem'"); udev_enumerate = udev_enumerate_new(udev); - if (udev_enumerate == NULL) + if (!udev_enumerate) return -1; udev_enumerate_scan_subsystems(udev_enumerate); enumerate_print_list(udev_enumerate); @@ -320,7 +320,7 @@ static int test_enumerate(struct udev *udev, const char *subsystem) { log_info("enumerate 'property IF_FS_*=filesystem'"); udev_enumerate = udev_enumerate_new(udev); - if (udev_enumerate == NULL) + if (!udev_enumerate) return -1; udev_enumerate_add_match_property(udev_enumerate, "ID_FS*", "filesystem"); udev_enumerate_scan_devices(udev_enumerate); diff --git a/src/test/test-nss.c b/src/test/test-nss.c index 27afe36082..d0f9898378 100644 --- a/src/test/test-nss.c +++ b/src/test/test-nss.c @@ -88,7 +88,7 @@ static int print_gaih_addrtuples(const struct gaih_addrtuple *tuples) { if (it->scopeid == 0) goto numerical_index; - if (if_indextoname(it->scopeid, ifname) == NULL) { + if (!if_indextoname(it->scopeid, ifname)) { log_warning_errno(errno, "if_indextoname(%d) failed: %m", it->scopeid); numerical_index: xsprintf(ifname, "%i", it->scopeid); diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index d9d1cc1c1a..73e0e33d0a 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -2397,7 +2397,7 @@ static int specifier_expansion_from_arg(Item *i) { assert(i); - if (i->argument == NULL) + if (!i->argument) return 0; switch (i->type) { diff --git a/src/udev/ata_id/ata_id.c b/src/udev/ata_id/ata_id.c index 8ea2e1e327..9aca82b1fb 100644 --- a/src/udev/ata_id/ata_id.c +++ b/src/udev/ata_id/ata_id.c @@ -381,7 +381,7 @@ static int disk_identify(int fd, } out: - if (out_is_packet_device != NULL) + if (out_is_packet_device) *out_is_packet_device = is_packet_device; return ret; } @@ -432,7 +432,7 @@ int main(int argc, char *argv[]) { } node = argv[optind]; - if (node == NULL) { + if (!node) { log_error("no node specified"); return 1; } diff --git a/src/udev/cdrom_id/cdrom_id.c b/src/udev/cdrom_id/cdrom_id.c index 3f882f557b..7581ddb5b4 100644 --- a/src/udev/cdrom_id/cdrom_id.c +++ b/src/udev/cdrom_id/cdrom_id.c @@ -95,7 +95,7 @@ static bool is_mounted(const char *device) { return false; fp = fopen("/proc/self/mountinfo", "re"); - if (fp == NULL) + if (!fp) return false; while (fscanf(fp, "%*s %*s %i:%i %*[^\n]", &maj, &min) == 2) { if (makedev(maj, min) == statbuf.st_rdev) { @@ -1026,7 +1026,7 @@ work: if (cd_media_hddvd_rw) printf("ID_CDROM_MEDIA_HDDVD_RW=1\n"); - if (cd_media_state != NULL) + if (cd_media_state) printf("ID_CDROM_MEDIA_STATE=%s\n", cd_media_state); if (cd_media_session_next > 0) printf("ID_CDROM_MEDIA_SESSION_NEXT=%u\n", cd_media_session_next); diff --git a/src/udev/scsi_id/scsi_id.c b/src/udev/scsi_id/scsi_id.c index 2698cdd82f..f57ae9be75 100644 --- a/src/udev/scsi_id/scsi_id.c +++ b/src/udev/scsi_id/scsi_id.c @@ -157,7 +157,7 @@ static int get_file_options(const char *vendor, const char *model, int retval = 0; f = fopen(config_file, "re"); - if (f == NULL) { + if (!f) { if (errno == ENOENT) return 1; else { @@ -181,7 +181,7 @@ static int get_file_options(const char *vendor, const char *model, vendor_in = model_in = options_in = NULL; buf = fgets(buffer, MAX_BUFFER_LEN, f); - if (buf == NULL) + if (!buf) break; lineno++; if (buf[strlen(buffer) - 1] != '\n') { @@ -239,7 +239,7 @@ static int get_file_options(const char *vendor, const char *model, break; } if (vendor == NULL) { - if (vendor_in == NULL) + if (!vendor_in) break; } else if (vendor_in && startswith(vendor, vendor_in) && diff --git a/src/udev/scsi_id/scsi_serial.c b/src/udev/scsi_id/scsi_serial.c index 7ca01858d1..8718f5ef1b 100644 --- a/src/udev/scsi_id/scsi_serial.c +++ b/src/udev/scsi_id/scsi_serial.c @@ -524,7 +524,7 @@ static int check_fill_0x83_id(struct scsi_id_device *dev_scsi, if (id_search->id_type == SCSI_ID_NAA && wwn != NULL) { strncpy(wwn, &serial[s], 16); - if (wwn_vendor_extension != NULL) + if (wwn_vendor_extension) strncpy(wwn_vendor_extension, &serial[s + 16], 16); } @@ -729,7 +729,7 @@ static int do_scsi_page80_inquiry(struct scsi_id_device *dev_scsi, int fd, * specific type where we prepend '0' + vendor + model. */ len = buf[3]; - if (serial != NULL) { + if (serial) { serial[0] = 'S'; ser_ind = prepend_vendor_model(dev_scsi, &serial[1]); if (ser_ind < 0) @@ -738,7 +738,7 @@ static int do_scsi_page80_inquiry(struct scsi_id_device *dev_scsi, int fd, for (i = 4; i < len + 4; i++, ser_ind++) serial[ser_ind] = buf[i]; } - if (serial_short != NULL) { + if (serial_short) { memcpy(serial_short, &buf[4], len); serial_short[len] = '\0'; } diff --git a/src/udev/udev-builtin-usb_id.c b/src/udev/udev-builtin-usb_id.c index 7bdf6cfbb5..58af63f11a 100644 --- a/src/udev/udev-builtin-usb_id.c +++ b/src/udev/udev-builtin-usb_id.c @@ -196,7 +196,7 @@ static int dev_if_packed_info(sd_device *dev, char *ifs_str, size_t len) { desc->bInterfaceProtocol) != 7) continue; - if (strstr(ifs_str, if_str) != NULL) + if (strstr(ifs_str, if_str)) continue; memcpy(&ifs_str[strpos], if_str, 8), diff --git a/src/udev/v4l_id/v4l_id.c b/src/udev/v4l_id/v4l_id.c index 4d60ee3c85..5de26b8062 100644 --- a/src/udev/v4l_id/v4l_id.c +++ b/src/udev/v4l_id/v4l_id.c @@ -56,7 +56,7 @@ int main(int argc, char *argv[]) { } device = argv[optind]; - if (device == NULL) + if (!device) return 2; fd = open(device, O_RDONLY); From 55033662f94baa3a1d0019f0f8398bd9d481e6be Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Sun, 28 Apr 2019 14:32:19 +0200 Subject: [PATCH 04/10] tree-wide: drop !! casts to booleans Done by coccinelle/bool-cast.cocci --- src/shared/efivars.c | 2 +- src/timesync/timesyncd-manager.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/shared/efivars.c b/src/shared/efivars.c index fc884dedf0..1bd8835633 100644 --- a/src/shared/efivars.c +++ b/src/shared/efivars.c @@ -514,7 +514,7 @@ int efi_get_boot_option( if (path) *path = TAKE_PTR(p); if (active) - *active = !!(header->attr & LOAD_OPTION_ACTIVE); + *active = header->attr & LOAD_OPTION_ACTIVE; return 0; } diff --git a/src/timesync/timesyncd-manager.c b/src/timesync/timesyncd-manager.c index 4c00fa409f..3c3a7fe69a 100644 --- a/src/timesync/timesyncd-manager.c +++ b/src/timesync/timesyncd-manager.c @@ -1024,7 +1024,7 @@ static int manager_network_event_handler(sd_event_source *s, int fd, uint32_t re sd_network_monitor_flush(m->network_monitor); /* When manager_network_read_link_servers() failed, we assume that the servers are changed. */ - changed = !!manager_network_read_link_servers(m); + changed = manager_network_read_link_servers(m); /* check if the machine is online */ online = network_is_online(); From 33af88cf70fce38a39642e92609cfba655925d55 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Sun, 28 Apr 2019 15:03:47 +0200 Subject: [PATCH 05/10] coccinelle: ignore macro transformations in the macros themselves For example, the following transformation: - sizeof(s)-1 + STRLEN(s) would replace sizeof by STRLEN even in the STRLEN macro definition itself, which generates following nonsensical patch: --- src/basic/macro.h +++ /tmp/cocci-output-8753-b50773-macro.h @@ -182,7 +182,7 @@ static inline unsigned long ALIGN_POWER2 * Contrary to strlen(), this is a constant expression. * @x: a string literal. */ -#define STRLEN(x) (sizeof(""x"") - 1) +#define STRLEN(x) (STRLEN("" x "")) /* * container_of - cast a member of a structure out to the containing structure Let's exclude the macro itself from the transformation to avoid this --- coccinelle/const-strlen.cocci | 4 ++++ coccinelle/debug-logging.cocci | 8 ++++++++ coccinelle/memzero.cocci | 8 ++++++++ 3 files changed, 20 insertions(+) diff --git a/coccinelle/const-strlen.cocci b/coccinelle/const-strlen.cocci index 38bf9b118f..30a6e5a88e 100644 --- a/coccinelle/const-strlen.cocci +++ b/coccinelle/const-strlen.cocci @@ -1,8 +1,12 @@ @@ constant s; @@ +( +#define STRLEN +& - sizeof(s)-1 + STRLEN(s) +) @@ constant s; @@ diff --git a/coccinelle/debug-logging.cocci b/coccinelle/debug-logging.cocci index 9084cf773b..a679dab011 100644 --- a/coccinelle/debug-logging.cocci +++ b/coccinelle/debug-logging.cocci @@ -1,8 +1,16 @@ @@ @@ +( +#define DEBUG_LOGGING +& - _unlikely_(log_get_max_level() >= LOG_DEBUG) + DEBUG_LOGGING +) @@ @@ +( +#define DEBUG_LOGGING +& - log_get_max_level() >= LOG_DEBUG + DEBUG_LOGGING +) diff --git a/coccinelle/memzero.cocci b/coccinelle/memzero.cocci index ebdc3f6a2a..8198cc84b4 100644 --- a/coccinelle/memzero.cocci +++ b/coccinelle/memzero.cocci @@ -21,10 +21,18 @@ expression s; @@ expression a, b; @@ +( +#define memzero +& - memset(a, 0, b) + memzero(a, b) +) @@ expression a, b; @@ +( +#define memzero +& - bzero(a, b) + memzero(a, b) +) From 60d9959dd8ba90e4a112e6e65429b112334d1bf8 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Sun, 28 Apr 2019 17:13:29 +0200 Subject: [PATCH 06/10] coccinelle: ignore function transformations causing recursion For example, following transformation: - isempty(s) ? NULL : s + empty_to_null(s) would get applied to the empty_to_null function itself as well, causing an infinite recursion, like: --- src/basic/string-util.h +++ /tmp/cocci-output-307-9f76e6-string-util.h @@ -50,11 +50,11 @@ static inline bool isempty(const char *p } static inline const char *empty_to_null(const char *p) { - return isempty(p) ? NULL : p; + return empty_to_null(p); } Let's avoid that by checking the current match position --- coccinelle/empty-to-null.cocci | 5 +++- coccinelle/mfree_return.cocci | 8 +++--- coccinelle/strempty.cocci | 48 +++++++++++++++++++++------------- 3 files changed, 39 insertions(+), 22 deletions(-) diff --git a/coccinelle/empty-to-null.cocci b/coccinelle/empty-to-null.cocci index fbc75b9c34..bc6c656e79 100644 --- a/coccinelle/empty-to-null.cocci +++ b/coccinelle/empty-to-null.cocci @@ -1,5 +1,8 @@ @@ +/* Avoid running this transformation on the empty_to_null function itself */ +position p : script:python() { p[0].current_element != "empty_to_null" }; expression s; @@ -- isempty(s) ? NULL : s + +- isempty@p(s) ? NULL : s + empty_to_null(s) diff --git a/coccinelle/mfree_return.cocci b/coccinelle/mfree_return.cocci index 8119fe07f2..15e6c7d566 100644 --- a/coccinelle/mfree_return.cocci +++ b/coccinelle/mfree_return.cocci @@ -1,6 +1,8 @@ @@ -expression p; +/* Avoid running this transformation on the mfree function itself */ +position p : script:python() { p[0].current_element != "mfree" }; +expression e; @@ -- free(p); +- free@p(e); - return NULL; -+ return mfree(p); ++ return mfree(e); diff --git a/coccinelle/strempty.cocci b/coccinelle/strempty.cocci index 13ceb338f1..7901da3652 100644 --- a/coccinelle/strempty.cocci +++ b/coccinelle/strempty.cocci @@ -1,48 +1,60 @@ @@ +/* Avoid running this transformation on the strempty function itself */ +position p : script:python() { p[0].current_element != "strempty" }; expression s; @@ -- s ?: "" +( +- s@p ?: "" + strempty(s) -@@ -expression s; -@@ -- s ? s : "" +| +- s@p ? s : "" + strempty(s) +) + @@ +position p : script:python() { p[0].current_element != "strempty" }; expression s; @@ -- if (!s) +- if (!s@p) - s = ""; + s = strempty(s); + @@ +position p : script:python() { p[0].current_element != "strnull" }; expression s; @@ -- s ?: "(null)" +( +- s@p ?: "(null)" + strnull(s) -@@ -expression s; -@@ -- s ? s : "(null)" +| +- s@p ? s : "(null)" + strnull(s) +) + @@ +position p : script:python() { p[0].current_element != "strnull" }; expression s; @@ -- if (!s) +- if (!s@p) - s = "(null)"; + s = strnull(s); + @@ +position p : script:python() { p[0].current_element != "strna" }; expression s; @@ -- s ?: "n/a" +( +- s@p ?: "n/a" + strna(s) -@@ -expression s; -@@ -- s ? s : "n/a" +| +- s@p ? s : "n/a" + strna(s) +) + @@ +position p : script:python() { p[0].current_element != "strna" }; expression s; @@ -- if (!s) +- if (!s@p) - s = "n/a"; + s = strna(s); From 4a4eaade6072d5358f9b8cc7e1a6fca22e7d15e6 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Sun, 28 Apr 2019 20:43:00 +0200 Subject: [PATCH 07/10] coccinelle: exclude certain paths from the transformations There's no point in running these transformation for certain files, mainly anything from src/boot/efi and src/shared/linux, as this code doesn't have access to our internal utility functions --- coccinelle/run-coccinelle.sh | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/coccinelle/run-coccinelle.sh b/coccinelle/run-coccinelle.sh index 4d882112e6..be80a76a5f 100755 --- a/coccinelle/run-coccinelle.sh +++ b/coccinelle/run-coccinelle.sh @@ -1,10 +1,25 @@ #!/bin/bash -e +# Exclude following paths from the Coccinelle transformations +EXCLUDED_PATHS=( + "src/boot/efi/*" + "src/shared/linux/*" + "src/basic/linux/*" + # Symlinked to test-bus-vtable-cc.cc, which causes issues with the IN_SET macro + "src/libsystemd/sd-bus/test-bus-vtable.c" +) + top="$(git rev-parse --show-toplevel)" -files="$(git ls-files ':/*.[ch]')" iso_defs="$top/coccinelle/systemd-definitions.iso" args= +# Create an array from files tracked by git... +mapfile -t files < <(git ls-files ':/*.[ch]') +# ...and filter everything that matches patterns from EXCLUDED_PATHS +for excl in "${EXCLUDED_PATHS[@]}"; do + files=(${files[@]//$excl}) +done + case "$1" in -i) args="$args --in-place" @@ -22,7 +37,7 @@ for SCRIPT in ${@-$top/coccinelle/*.cocci} ; do TMPFILE=`mktemp` echo "+ spatch --sp-file $SCRIPT $args ..." parallel --halt now,fail=1 --keep-order --noswap --max-args=20 \ - spatch --iso-file $iso_defs --sp-file $SCRIPT $args ::: $files \ + spatch --iso-file $iso_defs --sp-file $SCRIPT $args ::: "${files[@]}" \ 2>"$TMPFILE" || cat "$TMPFILE" echo -e "--x-- Processed $SCRIPT --x--\n" done From ed0cb346821972ec2c505ee11ed3d383aba6256e Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Mon, 29 Apr 2019 18:22:22 +0200 Subject: [PATCH 08/10] tree-wide: code improvements suggested by Coccinelle --- man/vtable-example.c | 2 +- src/core/dbus-socket.c | 8 ++++-- src/coredump/coredump.c | 5 +--- src/import/importd.c | 2 +- src/login/logind-core.c | 3 +- src/login/logind-user.c | 2 +- src/shared/dns-domain.c | 2 +- src/shared/machine-image.c | 2 +- src/udev/cdrom_id/cdrom_id.c | 21 ++++++-------- src/udev/scsi_id/scsi_id.c | 10 ++++--- src/udev/scsi_id/scsi_serial.c | 52 +++++++++++++++++++--------------- 11 files changed, 57 insertions(+), 52 deletions(-) diff --git a/man/vtable-example.c b/man/vtable-example.c index a2a6cd18d7..98c20eec52 100644 --- a/man/vtable-example.c +++ b/man/vtable-example.c @@ -59,7 +59,7 @@ int main(int argc, char **argv) { vtable, &object)); - while (true) { + for (;;) { check(sd_bus_wait(bus, UINT64_MAX)); check(sd_bus_process(bus, NULL)); } diff --git a/src/core/dbus-socket.c b/src/core/dbus-socket.c index 83ac7ad110..0aadc98c9b 100644 --- a/src/core/dbus-socket.c +++ b/src/core/dbus-socket.c @@ -303,8 +303,12 @@ static int bus_socket_set_transient_property( if (streq(name, "SocketProtocol")) return bus_set_transient_socket_protocol(u, name, &s->socket_protocol, message, flags, error); - if ((ci = socket_exec_command_from_string(name)) >= 0) - return bus_set_transient_exec_command(u, name, &s->exec_command[ci], message, flags, error); + ci = socket_exec_command_from_string(name); + if (ci >= 0) + return bus_set_transient_exec_command(u, name, + &s->exec_command[ci], + message, flags, error); + if (streq(name, "Symlinks")) { _cleanup_strv_free_ char **l = NULL; diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index ac7b972026..ed6d4ea14d 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -1036,10 +1036,7 @@ static int send_iovec(const struct iovec iovec[], size_t n_iovec, int input_fd) * (truncated) copy of what we want to send, and the second one * contains the trailing dots. */ copy[0] = iovec[i]; - copy[1] = (struct iovec) { - .iov_base = (char[]) { '.', '.', '.' }, - .iov_len = 3, - }; + copy[1] = IOVEC_MAKE(((char[]){'.', '.', '.'}), 3); mh.msg_iov = copy; mh.msg_iovlen = 2; diff --git a/src/import/importd.c b/src/import/importd.c index 2426933558..f4ca8f4f59 100644 --- a/src/import/importd.c +++ b/src/import/importd.c @@ -250,7 +250,7 @@ static void transfer_send_logs(Transfer *t, bool flush) { n = strndup(t->log_message, e - t->log_message); /* Skip over NUL and newlines */ - while (e < t->log_message + t->log_message_size && (*e == 0 || *e == '\n')) + while (e < t->log_message + t->log_message_size && IN_SET(*e, 0, '\n')) e++; memmove(t->log_message, e, t->log_message + sizeof(t->log_message) - e); diff --git a/src/login/logind-core.c b/src/login/logind-core.c index 2467da18ee..dacd3b3d9c 100644 --- a/src/login/logind-core.c +++ b/src/login/logind-core.c @@ -693,8 +693,7 @@ bool manager_all_buttons_ignored(Manager *m) { return false; if (m->handle_lid_switch != HANDLE_IGNORE) return false; - if (m->handle_lid_switch_ep != _HANDLE_ACTION_INVALID && - m->handle_lid_switch_ep != HANDLE_IGNORE) + if (!IN_SET(m->handle_lid_switch_ep, _HANDLE_ACTION_INVALID, HANDLE_IGNORE)) return false; if (m->handle_lid_switch_docked != HANDLE_IGNORE) return false; diff --git a/src/login/logind-user.c b/src/login/logind-user.c index 8356a9089a..62dba2adc9 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -754,7 +754,7 @@ void user_update_last_session_timer(User *u) { assert(!u->timer_event_source); - if (u->manager->user_stop_delay == 0 || u->manager->user_stop_delay == USEC_INFINITY) + if (IN_SET(u->manager->user_stop_delay, 0, USEC_INFINITY)) return; if (sd_event_get_state(u->manager->event) == SD_EVENT_FINISHED) { diff --git a/src/shared/dns-domain.c b/src/shared/dns-domain.c index 4b31cb36ed..f62ad0a0f5 100644 --- a/src/shared/dns-domain.c +++ b/src/shared/dns-domain.c @@ -37,7 +37,7 @@ int dns_label_unescape(const char **name, char *dest, size_t sz, DNSLabelFlags f d = dest; for (;;) { - if (*n == 0 || *n == '.') { + if (IN_SET(*n, 0, '.')) { if (FLAGS_SET(flags, DNS_LABEL_LDH) && last_char == '-') /* Trailing dash */ return -EINVAL; diff --git a/src/shared/machine-image.c b/src/shared/machine-image.c index 4ad112740d..6b9d8fb97a 100644 --- a/src/shared/machine-image.c +++ b/src/shared/machine-image.c @@ -395,7 +395,7 @@ static int image_make( if (r < 0) return r; - if (size != 0 && size != UINT64_MAX) + if (!IN_SET(size, 0, UINT64_MAX)) (*ret)->usage = (*ret)->usage_exclusive = (*ret)->limit = (*ret)->limit_exclusive = size; return 0; diff --git a/src/udev/cdrom_id/cdrom_id.c b/src/udev/cdrom_id/cdrom_id.c index 7581ddb5b4..231d39dcdd 100644 --- a/src/udev/cdrom_id/cdrom_id.c +++ b/src/udev/cdrom_id/cdrom_id.c @@ -203,8 +203,7 @@ static int cd_capability_compat(int fd) { capability = ioctl(fd, CDROM_GET_CAPABILITY, NULL); if (capability < 0) { - log_debug("CDROM_GET_CAPABILITY failed"); - return -1; + return log_debug_errno(errno, "CDROM_GET_CAPABILITY failed"); } if (capability & CDC_CD_R) @@ -226,8 +225,7 @@ static int cd_capability_compat(int fd) { static int cd_media_compat(int fd) { if (ioctl(fd, CDROM_DRIVE_STATUS, CDSL_CURRENT) != CDS_DISC_OK) { - log_debug("CDROM_DRIVE_STATUS != CDS_DISC_OK"); - return -1; + return log_debug_errno(errno, "CDROM_DRIVE_STATUS != CDS_DISC_OK"); } cd_media = 1; return 0; @@ -249,8 +247,7 @@ static int cd_inquiry(int fd) { } if ((inq[0] & 0x1F) != 5) { - log_debug("not an MMC unit"); - return -1; + return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "not an MMC unit"); } log_debug("INQUIRY: [%.8s][%.16s][%.4s]", inq + 8, inq + 16, inq + 32); @@ -474,8 +471,8 @@ static int cd_profiles_old_mmc(int fd) { cd_media_track_count_data = 1; return 0; } else { - log_debug("no current profile, assuming no media"); - return -1; + return log_debug_errno(SYNTHETIC_ERRNO(ENOMEDIUM), + "no current profile, assuming no media"); } }; @@ -660,8 +657,8 @@ static int cd_media_info(int fd) { len = format[3]; if (len & 7 || len < 16) { - log_debug("invalid format capacities length"); - return -1; + return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), + "invalid format capacities length"); } switch(format[8] & 3) { @@ -680,8 +677,8 @@ static int cd_media_info(int fd) { case 3: cd_media = 0; //return no media - log_debug("format capacities returned no media"); - return -1; + return log_debug_errno(SYNTHETIC_ERRNO(ENOMEDIUM), + "format capacities returned no media"); } } diff --git a/src/udev/scsi_id/scsi_id.c b/src/udev/scsi_id/scsi_id.c index f57ae9be75..bb9801e7cc 100644 --- a/src/udev/scsi_id/scsi_id.c +++ b/src/udev/scsi_id/scsi_id.c @@ -347,16 +347,18 @@ static int set_options(int argc, char **argv, else if (streq(optarg, "pre-spc3-83")) default_page_code = PAGE_83_PRE_SPC3; else { - log_error("Unknown page code '%s'", optarg); - return -1; + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "Unknown page code '%s'", + optarg); } break; case 's': sg_version = atoi(optarg); if (sg_version < 3 || sg_version > 4) { - log_error("Unknown SG version '%s'", optarg); - return -1; + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "Unknown SG version '%s'", + optarg); } break; diff --git a/src/udev/scsi_id/scsi_serial.c b/src/udev/scsi_id/scsi_serial.c index 8718f5ef1b..fc62339a52 100644 --- a/src/udev/scsi_id/scsi_serial.c +++ b/src/udev/scsi_id/scsi_serial.c @@ -169,8 +169,9 @@ static int scsi_dump_sense(struct scsi_id_device *dev_scsi, */ if (sb_len < 1) { - log_debug("%s: sense buffer empty", dev_scsi->kernel); - return -1; + return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), + "%s: sense buffer empty", + dev_scsi->kernel); } sense_class = (sense_buffer[0] >> 4) & 0x07; @@ -182,9 +183,10 @@ static int scsi_dump_sense(struct scsi_id_device *dev_scsi, */ s = sense_buffer[7] + 8; if (sb_len < s) { - log_debug("%s: sense buffer too small %d bytes, %d bytes too short", - dev_scsi->kernel, sb_len, s - sb_len); - return -1; + return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), + "%s: sense buffer too small %d bytes, %d bytes too short", + dev_scsi->kernel, sb_len, + s - sb_len); } if (IN_SET(code, 0x0, 0x1)) { sense_key = sense_buffer[2] & 0xf; @@ -192,9 +194,9 @@ static int scsi_dump_sense(struct scsi_id_device *dev_scsi, /* * Possible? */ - log_debug("%s: sense result too" " small %d bytes", - dev_scsi->kernel, s); - return -1; + return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), + "%s: sense result too small %d bytes", + dev_scsi->kernel, s); } asc = sense_buffer[12]; ascq = sense_buffer[13]; @@ -203,17 +205,18 @@ static int scsi_dump_sense(struct scsi_id_device *dev_scsi, asc = sense_buffer[2]; ascq = sense_buffer[3]; } else { - log_debug("%s: invalid sense code 0x%x", - dev_scsi->kernel, code); - return -1; + return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), + "%s: invalid sense code 0x%x", + dev_scsi->kernel, code); } log_debug("%s: sense key 0x%x ASC 0x%x ASCQ 0x%x", dev_scsi->kernel, sense_key, asc, ascq); } else { if (sb_len < 4) { - log_debug("%s: sense buffer too small %d bytes, %d bytes too short", - dev_scsi->kernel, sb_len, 4 - sb_len); - return -1; + return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), + "%s: sense buffer too small %d bytes, %d bytes too short", + dev_scsi->kernel, sb_len, + 4 - sb_len); } if (sense_buffer[0] < 15) @@ -235,8 +238,9 @@ static int scsi_dump(struct scsi_id_device *dev_scsi, struct sg_io_hdr *io) { /* * Impossible, should not be called. */ - log_debug("%s: called with no error", __FUNCTION__); - return -1; + return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), + "%s: called with no error", + __FUNCTION__); } log_debug("%s: sg_io failed status 0x%x 0x%x 0x%x 0x%x", @@ -253,8 +257,9 @@ static int scsi_dump_v4(struct scsi_id_device *dev_scsi, struct sg_io_v4 *io) { /* * Impossible, should not be called. */ - log_debug("%s: called with no error", __FUNCTION__); - return -1; + return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), + "%s: called with no error", + __FUNCTION__); } log_debug("%s: sg_io failed status 0x%x 0x%x 0x%x", @@ -279,8 +284,8 @@ static int scsi_inquiry(struct scsi_id_device *dev_scsi, int fd, int retval; if (buflen > SCSI_INQ_BUFF_LEN) { - log_debug("buflen %d too long", buflen); - return -1; + return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), + "buflen %d too long", buflen); } resend: @@ -412,9 +417,10 @@ static int prepend_vendor_model(struct scsi_id_device *dev_scsi, char *serial) { * above, ind will never be too large. */ if (ind != (VENDOR_LENGTH + MODEL_LENGTH)) { - log_debug("%s: expected length %d, got length %d", - dev_scsi->kernel, (VENDOR_LENGTH + MODEL_LENGTH), ind); - return -1; + return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), + "%s: expected length %d, got length %d", + dev_scsi->kernel, + (VENDOR_LENGTH + MODEL_LENGTH), ind); } return ind; } From 1f7247903751a79caadc7999f1f5c0ed1d08c24d Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Mon, 29 Apr 2019 13:15:27 +0200 Subject: [PATCH 09/10] coccinelle: exclude JsonVariant* from the IN_SET transformation JsonVariant* doesn't work with the current IN_SET implementation, so let's exclude it from the transformation altogether --- coccinelle/in_set.cocci | 49 ++++++++++++------------------------- coccinelle/not_in_set.cocci | 46 ++++++++++------------------------ 2 files changed, 29 insertions(+), 66 deletions(-) diff --git a/coccinelle/in_set.cocci b/coccinelle/in_set.cocci index 12d5475fd9..2c9b94ceb6 100644 --- a/coccinelle/in_set.cocci +++ b/coccinelle/in_set.cocci @@ -1,54 +1,37 @@ @@ expression e; -constant n0, n1, n2, n3, n4, n5, n6, n7, n8, n9; +/* Exclude JsonVariant * from the transformation, as it can't work with the + * current version of the IN_SET macro */ +typedef JsonVariant; +type T != JsonVariant*; +constant T n0, n1, n2, n3, n4, n5, n6, n7, n8, n9; @@ + +( - e == n0 || e == n1 || e == n2 || e == n3 || e == n4 || e == n5 || e == n6 || e == n7 || e == n8 || e == n9 + IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7, n8, n9) -@@ -expression e; -constant n0, n1, n2, n3, n4, n5, n6, n7, n8; -@@ +| - e == n0 || e == n1 || e == n2 || e == n3 || e == n4 || e == n5 || e == n6 || e == n7 || e == n8 + IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7, n8) -@@ -expression e; -constant n0, n1, n2, n3, n4, n5, n6, n7; -@@ +| - e == n0 || e == n1 || e == n2 || e == n3 || e == n4 || e == n5 || e == n6 || e == n7 + IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7) -@@ -expression e; -constant n0, n1, n2, n3, n4, n5, n6; -@@ +| - e == n0 || e == n1 || e == n2 || e == n3 || e == n4 || e == n5 || e == n6 + IN_SET(e, n0, n1, n2, n3, n4, n5, n6) -@@ -expression e; -constant n0, n1, n2, n3, n4, n5; -@@ +| - e == n0 || e == n1 || e == n2 || e == n3 || e == n4 || e == n5 + IN_SET(e, n0, n1, n2, n3, n4, n5) -@@ -expression e; -constant n0, n1, n2, n3, n4; -@@ +| - e == n0 || e == n1 || e == n2 || e == n3 || e == n4 + IN_SET(e, n0, n1, n2, n3, n4) -@@ -expression e; -constant n0, n1, n2, n3; -@@ +| - e == n0 || e == n1 || e == n2 || e == n3 + IN_SET(e, n0, n1, n2, n3) -@@ -expression e; -constant n0, n1, n2; -@@ +| - e == n0 || e == n1 || e == n2 + IN_SET(e, n0, n1, n2) -@@ -expression e; -constant n0, n1; -@@ +| - e == n0 || e == n1 + IN_SET(e, n0, n1) +) diff --git a/coccinelle/not_in_set.cocci b/coccinelle/not_in_set.cocci index 7cf98500cd..aed2c3490c 100644 --- a/coccinelle/not_in_set.cocci +++ b/coccinelle/not_in_set.cocci @@ -1,54 +1,34 @@ @@ expression e; -constant n0, n1, n2, n3, n4, n5, n6, n7, n8, n9; +typedef JsonVariant; +type T != JsonVariant*; +constant T n0, n1, n2, n3, n4, n5, n6, n7, n8, n9; @@ +( - e != n0 && e != n1 && e != n2 && e != n3 && e != n4 && e != n5 && e != n6 && e != n7 && e != n8 && e != n9 + !IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7, n8, n9) -@@ -expression e; -constant n0, n1, n2, n3, n4, n5, n6, n7, n8; -@@ +| - e != n0 && e != n1 && e != n2 && e != n3 && e != n4 && e != n5 && e != n6 && e != n7 && e != n8 + !IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7, n8) -@@ -expression e; -constant n0, n1, n2, n3, n4, n5, n6, n7; -@@ +| - e != n0 && e != n1 && e != n2 && e != n3 && e != n4 && e != n5 && e != n6 && e != n7 + !IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7) -@@ -expression e; -constant n0, n1, n2, n3, n4, n5, n6; -@@ +| - e != n0 && e != n1 && e != n2 && e != n3 && e != n4 && e != n5 && e != n6 + !IN_SET(e, n0, n1, n2, n3, n4, n5, n6) -@@ -expression e; -constant n0, n1, n2, n3, n4, n5; -@@ +| - e != n0 && e != n1 && e != n2 && e != n3 && e != n4 && e != n5 + !IN_SET(e, n0, n1, n2, n3, n4, n5) -@@ -expression e; -constant n0, n1, n2, n3, n4; -@@ +| - e != n0 && e != n1 && e != n2 && e != n3 && e != n4 + !IN_SET(e, n0, n1, n2, n3, n4) -@@ -expression e; -constant n0, n1, n2, n3; -@@ +| - e != n0 && e != n1 && e != n2 && e != n3 + !IN_SET(e, n0, n1, n2, n3) -@@ -expression e; -constant n0, n1, n2; -@@ +| - e != n0 && e != n1 && e != n2 + !IN_SET(e, n0, n1, n2) -@@ -expression e; -constant n0, n1; -@@ +| - e != n0 && e != n1 + !IN_SET(e, n0, n1) +) From ccd52940d06fc6ba06f44f7ea64f056529c0beb0 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Mon, 29 Apr 2019 16:12:41 +0200 Subject: [PATCH 10/10] coccinelle: further restrict certain transformations Some transformations generate results we don't want to keep, so let's disable such transformations for specific files. Also, disable const-strlen.cocci everywhere, as the STRLEN macro has a pretty limited scope, so the transformation generates false positives in most cases. --- ...nst-strlen.cocci => const-strlen.disabled} | 0 coccinelle/dup-fcntl.cocci | 4 +- coccinelle/flags-set.cocci | 19 ++--- coccinelle/isempty.cocci | 72 +++++++------------ coccinelle/synthetic-errno.cocci | 6 ++ 5 files changed, 46 insertions(+), 55 deletions(-) rename coccinelle/{const-strlen.cocci => const-strlen.disabled} (100%) diff --git a/coccinelle/const-strlen.cocci b/coccinelle/const-strlen.disabled similarity index 100% rename from coccinelle/const-strlen.cocci rename to coccinelle/const-strlen.disabled diff --git a/coccinelle/dup-fcntl.cocci b/coccinelle/dup-fcntl.cocci index ef13564282..8b133b3a24 100644 --- a/coccinelle/dup-fcntl.cocci +++ b/coccinelle/dup-fcntl.cocci @@ -1,5 +1,7 @@ @@ +/* We want to stick with dup() in test-fd-util.c */ +position p : script:python() { p[0].file != "src/test/test-fd-util.c" }; expression fd; @@ -- dup(fd) +- dup@p(fd) + fcntl(fd, F_DUPFD, 3) diff --git a/coccinelle/flags-set.cocci b/coccinelle/flags-set.cocci index 1a70717e76..73966b02e5 100644 --- a/coccinelle/flags-set.cocci +++ b/coccinelle/flags-set.cocci @@ -1,15 +1,16 @@ @@ +/* Disable this transformation for the securebits-util.h, as it makes + * the expression there confusing. */ +position p : script:python() { p[0].file != "src/shared/securebits-util.h" }; expression x, y; @@ -- ((x) & (y)) == (y) +( +- ((x@p) & (y)) == (y) + FLAGS_SET(x, y) -@@ -expression x, y; -@@ -- (x & (y)) == (y) +| +- (x@p & (y)) == (y) + FLAGS_SET(x, y) -@@ -expression x, y; -@@ -- ((x) & y) == y +| +- ((x@p) & y) == y + FLAGS_SET(x, y) +) diff --git a/coccinelle/isempty.cocci b/coccinelle/isempty.cocci index d8d5275889..e0a9f07ca6 100644 --- a/coccinelle/isempty.cocci +++ b/coccinelle/isempty.cocci @@ -1,60 +1,42 @@ @@ +/* Disable this transformation for the test-string-util.c */ +position p : script:python() { p[0].file != "src/test/test-string-util.c" }; expression s; @@ -- strv_length(s) == 0 +( +- strv_length@p(s) == 0 + strv_isempty(s) -@@ -expression s; -@@ -- strv_length(s) <= 0 +| +- strv_length@p(s) <= 0 + strv_isempty(s) -@@ -expression s; -@@ -- strv_length(s) > 0 +| +- strv_length@p(s) > 0 + !strv_isempty(s) -@@ -expression s; -@@ -- strv_length(s) != 0 +| +- strv_length@p(s) != 0 + !strv_isempty(s) -@@ -expression s; -@@ -- strlen(s) == 0 +| +- strlen@p(s) == 0 + isempty(s) -@@ -expression s; -@@ -- strlen(s) <= 0 +| +- strlen@p(s) <= 0 + isempty(s) -@@ -expression s; -@@ -- strlen(s) > 0 +| +- strlen@p(s) > 0 + !isempty(s) -@@ -expression s; -@@ -- strlen(s) != 0 +| +- strlen@p(s) != 0 + !isempty(s) -@@ -expression s; -@@ -- strlen_ptr(s) == 0 +| +- strlen_ptr@p(s) == 0 + isempty(s) -@@ -expression s; -@@ -- strlen_ptr(s) <= 0 +| +- strlen_ptr@p(s) <= 0 + isempty(s) -@@ -expression s; -@@ -- strlen_ptr(s) > 0 +| +- strlen_ptr@p(s) > 0 + !isempty(s) -@@ -expression s; -@@ -- strlen_ptr(s) != 0 +| +- strlen_ptr@p(s) != 0 + !isempty(s) +) diff --git a/coccinelle/synthetic-errno.cocci b/coccinelle/synthetic-errno.cocci index 645bfc945f..3ddb69cb4c 100644 --- a/coccinelle/synthetic-errno.cocci +++ b/coccinelle/synthetic-errno.cocci @@ -2,9 +2,15 @@ expression e; expression list args; @@ +( +/* Ignore one specific case in src/shared/bootspec.c where we want to stick + * with the log_debug() + return pattern */ +log_debug("Found no default boot entry :("); +| - log_debug(args); - return -e; + return log_debug_errno(SYNTHETIC_ERRNO(e), args); +) @@ expression e; expression list args;