diff --git a/.github/workflows/linter.yml b/.github/workflows/linter.yml index 2a7ff69dadc..78fbf6b0498 100644 --- a/.github/workflows/linter.yml +++ b/.github/workflows/linter.yml @@ -43,7 +43,7 @@ jobs: - uses: systemd/mkosi@131adaca5eb5f1551bc564ae4497ff940890a374 - name: Check that tabs are not used in Python code - run: sh -c '! git grep -P "\\t" -- src/core/generate-bpf-delegate-configs.py src/boot/generate-hwids-section.py src/ukify/ukify.py test/integration-tests/integration-test-wrapper.py' + run: sh -c '! git grep -P "\\t" -- src/core/generate-bpf-delegate-configs.py src/boot/generate-hwids-section.py src/ukify/ukify.py test/integration-tests/integration-test-wrapper.py tools/check-argument-comments.py' - name: Build tools tree run: | @@ -58,7 +58,7 @@ jobs: - name: Run mypy run: | mkosi box -- mypy --version - mkosi box -- mypy src/core/generate-bpf-delegate-configs.py src/boot/generate-hwids-section.py src/test/generate-sym-test.py src/ukify/ukify.py test/integration-tests/integration-test-wrapper.py + mkosi box -- mypy src/core/generate-bpf-delegate-configs.py src/boot/generate-hwids-section.py src/test/generate-sym-test.py src/ukify/ukify.py test/integration-tests/integration-test-wrapper.py tools/check-argument-comments.py - name: Run ruff format run: | @@ -86,6 +86,11 @@ jobs: - name: Run clang-tidy run: mkosi box -- meson test -C build --suite=clang-tidy --print-errorlogs --no-stdsplit --quiet + - name: Run argument comment checks + run: | + mkosi box -- python3 -c 'import clang.cindex' + mkosi box -- meson test -C build --suite=argument-comments --print-errorlogs --no-stdsplit --quiet + - name: Run coccinelle checks run: mkosi box -- meson test -C build --suite=coccinelle --print-errorlogs --no-stdsplit diff --git a/docs/CODING_STYLE.md b/docs/CODING_STYLE.md index 378de8248e2..6b497a8d2f8 100644 --- a/docs/CODING_STYLE.md +++ b/docs/CODING_STYLE.md @@ -286,6 +286,16 @@ SPDX-License-Identifier: LGPL-2.1-or-later ``` This guidance should be applied tree-wide, including in test files. + `tools/check-argument-comments.py` enforces it for every bare `NULL`, `0`, + `true` and `false`, not only the ones meaning "unset", passed for a parameter + with a telling name. Single-letter names, `argN`, reserved `__names`, calls + into libc and other code outside the tree, and calls produced by macro bodies + are exempt. Calls through the `sym_*` pointers of the dlopen() wrappers use + the names from the library's header. It also checks that an existing comment + names the right parameter. `meson test -C build --suite=argument-comments` + checks the whole tree. Once `meson compile -C build gensources` has run, + `tools/check-argument-comments.py -p build --diff "$(git merge-base main HEAD)" src` + checks only what a branch changed, and `--fix` inserts the missing comments. - Please do not introduce new circular dependencies between header files. Effectively this means that if a.h includes b.h, then b.h cannot include a.h, diff --git a/meson.build b/meson.build index 487dd758f80..32d2ebfc608 100644 --- a/meson.build +++ b/meson.build @@ -16,7 +16,7 @@ project('systemd', 'c', add_test_setup( 'default', - exclude_suites : ['clang-tidy', 'coccinelle', 'unused-symbols', 'integration-tests'], + exclude_suites : ['clang-tidy', 'coccinelle', 'argument-comments', 'unused-symbols', 'integration-tests'], exe_wrapper : find_program('tools/test-crash-trace.sh'), is_default : true, ) @@ -2889,6 +2889,7 @@ test_dlopen = executables_by_name.get('test-dlopen') nss_targets = [] pam_targets = [] module_targets = [] +module_sources = [] modules_by_name = {} foreach dict : modules name = dict.get('name') @@ -2906,6 +2907,8 @@ foreach dict : modules continue endif + module_sources += dict.get('sources', []) + kwargs = {} foreach key, val : dict if key in ['name', 'conditions', 'version-script'] @@ -3275,6 +3278,36 @@ if meson.version().version_compare('>=1.4.0') endif endforeach endforeach + + # One test per top-level source directory over the compiled .c files, which covers the headers + # they include. Generated sources are not below src/ relative to the source directory. The NSS, + # PAM and cryptsetup token modules are not in sources. + argument_comments_sources = {} + foreach source : sources + module_sources + if not source.full_path().endswith('.c') or source.full_path().endswith('.bpf.c') + continue + endif + + source_parts = fs.relative_to(source.full_path(), meson.project_source_root()).split('/') + if source_parts.length() > 2 and source_parts[0] == 'src' + argument_comments_sources += { + source_parts[1] : argument_comments_sources.get(source_parts[1], []) + [source], + } + endif + endforeach + + argument_comments_args = ['-p', meson.project_build_root(), '--root', meson.project_source_root()] + foreach source_dir, source_dir_sources : argument_comments_sources + test( + 'argument-comments-@0@'.format(source_dir), + check_argument_comments_py, + args : argument_comments_args + source_dir_sources, + suite : 'argument-comments', + depends : generated_sources, + is_parallel : false, # the script parallelizes over its files by itself + timeout : 300, + ) + endforeach endif spatch = find_program('spatch', required : false) diff --git a/mkosi/mkosi.tools.conf/mkosi.conf.d/centos-fedora.conf b/mkosi/mkosi.tools.conf/mkosi.conf.d/centos-fedora.conf index 87a8903dea2..de20cd5203d 100644 --- a/mkosi/mkosi.tools.conf/mkosi.conf.d/centos-fedora.conf +++ b/mkosi/mkosi.tools.conf/mkosi.conf.d/centos-fedora.conf @@ -10,6 +10,7 @@ Packages= bindgen-cli clang-tools-extra clippy + python3-clang python3-mypy rpm-build libasan diff --git a/mkosi/mkosi.tools.conf/mkosi.conf.d/debian-ubuntu.conf b/mkosi/mkosi.tools.conf/mkosi.conf.d/debian-ubuntu.conf index 39159797785..0313131a237 100644 --- a/mkosi/mkosi.tools.conf/mkosi.conf.d/debian-ubuntu.conf +++ b/mkosi/mkosi.tools.conf/mkosi.conf.d/debian-ubuntu.conf @@ -10,6 +10,7 @@ Packages= bindgen clang-tidy coccinelle + python3-clang lcov mypy rust-clippy diff --git a/mkosi/mkosi.tools.conf/mkosi.conf.d/opensuse.conf b/mkosi/mkosi.tools.conf/mkosi.conf.d/opensuse.conf index 1ac0b4a704c..38addcfdc45 100644 --- a/mkosi/mkosi.tools.conf/mkosi.conf.d/opensuse.conf +++ b/mkosi/mkosi.tools.conf/mkosi.conf.d/opensuse.conf @@ -12,6 +12,7 @@ Packages= lcov libtss2-tcti-device0 mypy + python3-clang python3-ruff rpm-build rust diff --git a/src/analyze/analyze-restart-delays.c b/src/analyze/analyze-restart-delays.c index 8bc00e6239a..b1b85d9f099 100644 --- a/src/analyze/analyze-restart-delays.c +++ b/src/analyze/analyze-restart-delays.c @@ -129,7 +129,7 @@ static int show_never_restarted(const RestartSettings *rs, const char *setting) if (r < 0) return log_oom(); - return sd_json_variant_dump(v, arg_json_format_flags, stdout, NULL); + return sd_json_variant_dump(v, arg_json_format_flags, stdout, /* prefix= */ NULL); } static int show_restart_delays(const RestartSettings *rs, bool *printed) { @@ -207,7 +207,7 @@ static int show_restart_delays(const RestartSettings *rs, bool *printed) { if (r < 0) return log_oom(); - return sd_json_variant_dump(v, arg_json_format_flags, stdout, NULL); + return sd_json_variant_dump(v, arg_json_format_flags, stdout, /* prefix= */ NULL); } r = table_print_with_pager(table, SD_JSON_FORMAT_OFF, arg_pager_flags, arg_legend); @@ -289,7 +289,7 @@ static int restart_delays_online(sd_bus *bus, const char *arg, bool *printed) { assert(arg); assert(printed); - r = unit_name_mangle_with_suffix(arg, NULL, UNIT_NAME_MANGLE_WARN, ".service", &unit); + r = unit_name_mangle_with_suffix(arg, /* operation= */ NULL, UNIT_NAME_MANGLE_WARN, ".service", &unit); if (r < 0) return log_error_errno(r, "Failed to mangle name '%s': %m", arg); @@ -317,7 +317,7 @@ static int restart_delays_online(sd_bus *bus, const char *arg, bool *printed) { map, BUS_MAP_STRDUP, &error, - NULL, + /* ret_reply= */ NULL, &rs); if (r < 0) return log_error_errno(r, "Failed to get properties of %s: %s", unit, bus_error_message(&error, r)); @@ -403,7 +403,7 @@ int verb_restart_delays(int argc, char *argv[], uintptr_t _data, void *userdata) int r, ret = 0; if (!arg_offline) { - r = acquire_bus(&bus, NULL); + r = acquire_bus(&bus, /* use_full_bus= */ NULL); if (r < 0) return bus_log_connect_error(r, arg_transport, arg_runtime_scope); } diff --git a/src/basic/io-uring-util.h b/src/basic/io-uring-util.h index 940cd1cdffe..291b9e320bb 100644 --- a/src/basic/io-uring-util.h +++ b/src/basic/io-uring-util.h @@ -110,11 +110,11 @@ static inline void io_uring_prep_rw( } static inline void io_uring_prep_nop(struct io_uring_sqe *sqe) { - io_uring_prep_rw(sqe, IORING_OP_NOP, -EBADF, NULL, 0, 0); + io_uring_prep_rw(sqe, IORING_OP_NOP, -EBADF, /* addr= */ NULL, /* len= */ 0, /* offset= */ 0); } static inline void io_uring_prep_cancel(struct io_uring_sqe *sqe, const void *user_data, uint32_t flags) { - io_uring_prep_rw(sqe, IORING_OP_ASYNC_CANCEL, -EBADF, user_data, 0, 0); + io_uring_prep_rw(sqe, IORING_OP_ASYNC_CANCEL, -EBADF, user_data, /* len= */ 0, /* offset= */ 0); sqe->cancel_flags = flags; } @@ -135,30 +135,30 @@ static inline void io_uring_prep_writev(struct io_uring_sqe *sqe, int fd, const } static inline void io_uring_prep_connect(struct io_uring_sqe *sqe, int fd, const struct sockaddr *addr, socklen_t addrlen) { - io_uring_prep_rw(sqe, IORING_OP_CONNECT, fd, addr, 0, addrlen); + io_uring_prep_rw(sqe, IORING_OP_CONNECT, fd, addr, /* len= */ 0, addrlen); } static inline void io_uring_prep_recv(struct io_uring_sqe *sqe, int fd, void *buf, uint32_t n, int flags) { - io_uring_prep_rw(sqe, IORING_OP_RECV, fd, buf, n, 0); + io_uring_prep_rw(sqe, IORING_OP_RECV, fd, buf, n, /* offset= */ 0); sqe->msg_flags = (uint32_t) flags; } static inline void io_uring_prep_send(struct io_uring_sqe *sqe, int fd, const void *buf, uint32_t n, int flags) { - io_uring_prep_rw(sqe, IORING_OP_SEND, fd, buf, n, 0); + io_uring_prep_rw(sqe, IORING_OP_SEND, fd, buf, n, /* offset= */ 0); sqe->msg_flags = (uint32_t) flags; } static inline void io_uring_prep_recvmsg(struct io_uring_sqe *sqe, int fd, struct msghdr *msg, int flags) { - io_uring_prep_rw(sqe, IORING_OP_RECVMSG, fd, msg, 1, 0); + io_uring_prep_rw(sqe, IORING_OP_RECVMSG, fd, msg, 1, /* offset= */ 0); sqe->msg_flags = (uint32_t) flags; } static inline void io_uring_prep_sendmsg(struct io_uring_sqe *sqe, int fd, const struct msghdr *msg, int flags) { - io_uring_prep_rw(sqe, IORING_OP_SENDMSG, fd, msg, 1, 0); + io_uring_prep_rw(sqe, IORING_OP_SENDMSG, fd, msg, 1, /* offset= */ 0); sqe->msg_flags = (uint32_t) flags; } static inline void io_uring_prep_accept(struct io_uring_sqe *sqe, int fd, struct sockaddr *addr, socklen_t *addrlen, int flags) { - io_uring_prep_rw(sqe, IORING_OP_ACCEPT, fd, addr, 0, (uint64_t) (uintptr_t) addrlen); + io_uring_prep_rw(sqe, IORING_OP_ACCEPT, fd, addr, /* len= */ 0, (uint64_t) (uintptr_t) addrlen); sqe->accept_flags = (uint32_t) flags; } diff --git a/src/basic/mountpoint-util.c b/src/basic/mountpoint-util.c index 2c7915b8f64..a25d32016fb 100644 --- a/src/basic/mountpoint-util.c +++ b/src/basic/mountpoint-util.c @@ -660,7 +660,7 @@ bool mount_new_api_supported(void) { return cache; /* This is the newest API among the ones we use, so use it as boundary */ - r = RET_NERRNO(mount_setattr(-EBADF, NULL, 0, NULL, 0)); + r = RET_NERRNO(mount_setattr(-EBADF, /* path= */ NULL, /* flags= */ 0, /* attr= */ NULL, /* size= */ 0)); if (r == 0 || ERRNO_IS_NOT_SUPPORTED(r)) /* This should return an error if it is working properly */ return (cache = false); @@ -689,7 +689,7 @@ int mount_option_supported(const char *fstype, const char *key, const char *valu * fsconfig() with FSCONFIG_SET_STRING/FSCONFIG_SET_FLAG never fail. Which sucks, because we want to * use it for testing support, after all. Let's hence do a check if the file system got converted yet * first. */ - if (fsconfig(fd, FSCONFIG_SET_FD, "adefinitelynotexistingmountoption", NULL, fd) < 0) { + if (fsconfig(fd, FSCONFIG_SET_FD, "adefinitelynotexistingmountoption", /* value= */ NULL, fd) < 0) { /* If FSCONFIG_SET_FD is not supported for the fs, then the file system was not converted to * the new mount API yet. If it returns EINVAL the mount option doesn't exist, but the fstype * is converted. */ @@ -704,9 +704,9 @@ int mount_option_supported(const char *fstype, const char *key, const char *valu return log_debug_errno(SYNTHETIC_ERRNO(EAGAIN), "FSCONFIG_SET_FD worked unexpectedly for '%s', whoa!", fstype); if (value) - r = fsconfig(fd, FSCONFIG_SET_STRING, key, value, 0); + r = fsconfig(fd, FSCONFIG_SET_STRING, key, value, /* aux= */ 0); else - r = fsconfig(fd, FSCONFIG_SET_FLAG, key, NULL, 0); + r = fsconfig(fd, FSCONFIG_SET_FLAG, key, /* value= */ NULL, /* aux= */ 0); if (r < 0) { if (errno == EINVAL) return false; /* EINVAL means option not supported. */ diff --git a/src/basic/pidfd-util.c b/src/basic/pidfd-util.c index a1bb9e9379b..b0fcea662e7 100644 --- a/src/basic/pidfd-util.c +++ b/src/basic/pidfd-util.c @@ -344,7 +344,7 @@ int pidfd_get_inode_id_self_cached(uint64_t *ret) { if (cached_pidfd_id_initialized < 0) return cached_pidfd_id_initialized; - _cleanup_close_ int fd = pidfd_open(getpid_cached(), 0); + _cleanup_close_ int fd = pidfd_open(getpid_cached(), /* flags= */ 0); if (fd < 0) return -errno; diff --git a/src/basic/pidref.c b/src/basic/pidref.c index c90a66c75c2..cd7b11eaf88 100644 --- a/src/basic/pidref.c +++ b/src/basic/pidref.c @@ -354,7 +354,7 @@ int pidref_kill(const PidRef *pidref, int sig) { return -EREMOTE; if (pidref->fd >= 0) - return RET_NERRNO(pidfd_send_signal(pidref->fd, sig, NULL, 0)); + return RET_NERRNO(pidfd_send_signal(pidref->fd, sig, /* info= */ NULL, /* flags= */ 0)); if (pidref->pid > 0) return RET_NERRNO(kill(pidref->pid, sig)); @@ -396,7 +396,7 @@ int pidref_sigqueue(const PidRef *pidref, int sig, int value) { si.si_uid = getuid(); si.si_value.sival_int = value; - return RET_NERRNO(pidfd_send_signal(pidref->fd, sig, &si, 0)); + return RET_NERRNO(pidfd_send_signal(pidref->fd, sig, &si, /* flags= */ 0)); } if (pidref->pid > 0) diff --git a/src/basic/process-util.c b/src/basic/process-util.c index b4ac4761528..ce23addaa11 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -2199,7 +2199,7 @@ int posix_spawn_wrapper( _cleanup_close_ int pidfd = -EBADF; - r = pidfd_spawn(&pidfd, path, NULL, &attr, argv, envp); + r = pidfd_spawn(&pidfd, path, /* file_actions= */ NULL, &attr, argv, envp); if (ERRNO_IS_NOT_SUPPORTED(r) && FLAGS_SET(flags, POSIX_SPAWN_SETCGROUP) && cg_is_threaded(cgroup) > 0) return -EUCLEAN; /* clone3() could also return EOPNOTSUPP if the target cgroup is in threaded mode, turn that into something recognizable */ @@ -2218,7 +2218,7 @@ int posix_spawn_wrapper( if (r != 0) return -r; - r = pidfd_spawn(&pidfd, path, NULL, &attr, argv, envp); + r = pidfd_spawn(&pidfd, path, /* file_actions= */ NULL, &attr, argv, envp); } if (r == 0) { r = pidref_set_pidfd_consume(ret_pidref, TAKE_FD(pidfd)); diff --git a/src/boot/util.h b/src/boot/util.h index 1c0a0412492..604b7c8cd23 100644 --- a/src/boot/util.h +++ b/src/boot/util.h @@ -129,7 +129,7 @@ static inline Pages xmalloc_initrd_pages(size_t n_pages) { AllocateAnyPages, EfiLoaderData, EFI_SIZE_TO_PAGES(n_pages), - 0 /* Ignored. */); + /* addr= */ 0 /* Ignored. */); } void convert_efi_path(char16_t *path); diff --git a/src/core/exec-credential.c b/src/core/exec-credential.c index 1081efdd9cc..dd0ea3fc9f9 100644 --- a/src/core/exec-credential.c +++ b/src/core/exec-credential.c @@ -1076,12 +1076,12 @@ static int setup_credentials_internal( // Work around a kernel bug that results in tmpfs reconfiguration failure. // FIXME: drop this once https://lore.kernel.org/linux-fsdevel/20251108190930.440685-1-me@yhndnzj.com/ // is merged and hits the distro kernels. - (void) fsconfig(fs_fd, FSCONFIG_SET_FLAG, "noswap", NULL, 0); + (void) fsconfig(fs_fd, FSCONFIG_SET_FLAG, "noswap", /* value= */ NULL, /* aux= */ 0); - if (fsconfig(fs_fd, FSCONFIG_SET_FLAG, "ro", NULL, 0) < 0) + if (fsconfig(fs_fd, FSCONFIG_SET_FLAG, "ro", /* value= */ NULL, /* aux= */ 0) < 0) return -errno; - if (fsconfig(fs_fd, FSCONFIG_CMD_RECONFIGURE, NULL, NULL, 0) < 0) + if (fsconfig(fs_fd, FSCONFIG_CMD_RECONFIGURE, /* key= */ NULL, /* value= */ NULL, /* aux= */ 0) < 0) return -errno; log_debug("Successfully reconfigured credentials fs to be read only."); diff --git a/src/core/exec-invoke.c b/src/core/exec-invoke.c index bc7a5226b70..e2a50141806 100644 --- a/src/core/exec-invoke.c +++ b/src/core/exec-invoke.c @@ -6154,7 +6154,7 @@ int exec_invoke( } if (context->ioprio_is_set) - if (ioprio_set(IOPRIO_WHO_PROCESS, 0, context->ioprio) < 0) { + if (ioprio_set(IOPRIO_WHO_PROCESS, /* who= */ 0, context->ioprio) < 0) { *exit_status = EXIT_IOPRIO; return log_error_errno(errno, "Failed to set up IO scheduling priority: %m"); } diff --git a/src/core/execute.c b/src/core/execute.c index 3a7ca78239c..091e696614b 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -1752,7 +1752,7 @@ int exec_context_get_effective_ioprio(const ExecContext *c) { if (c->ioprio_is_set) return c->ioprio; - p = ioprio_get(IOPRIO_WHO_PROCESS, 0); + p = ioprio_get(IOPRIO_WHO_PROCESS, /* who= */ 0); if (p < 0) return IOPRIO_DEFAULT_CLASS_AND_PRIO; diff --git a/src/core/namespace.c b/src/core/namespace.c index 91cfe7f693f..34ceb6b054a 100644 --- a/src/core/namespace.c +++ b/src/core/namespace.c @@ -1809,7 +1809,7 @@ static int mount_bpffs(const MountEntry *m, PidRef *pidref, int socket_fd, int e pidref_done(pidref); - _cleanup_close_ int mnt_fd = fsmount(fs_fd, FSMOUNT_CLOEXEC, /* mount_attrs= */ 0); + _cleanup_close_ int mnt_fd = fsmount(fs_fd, FSMOUNT_CLOEXEC, /* attr_flags= */ 0); if (mnt_fd < 0) return log_debug_errno(errno, "Failed to fsmount bpffs: %m"); @@ -3771,7 +3771,7 @@ static int unpeel_mount_and_setup_overlay(int pair_fd, const char *mount_path) { return log_debug_errno(r, "Failed to receive configured overlay: %m"); /* Create the mount */ - mount_fd = fsmount(overlay_fs_fd, FSMOUNT_CLOEXEC, /* flags= */ 0); + mount_fd = fsmount(overlay_fs_fd, FSMOUNT_CLOEXEC, /* attr_flags= */ 0); if (mount_fd < 0) return log_debug_errno(errno, "Failed to create overlay mount: %m"); @@ -3925,14 +3925,14 @@ static int handle_mount_from_grandchild( if (!options) return log_oom_debug(); - if (fsconfig(overlay_fs_fd, FSCONFIG_SET_STRING, "lowerdir", options, 0) < 0) + if (fsconfig(overlay_fs_fd, FSCONFIG_SET_STRING, "lowerdir", options, /* aux= */ 0) < 0) return log_debug_errno(errno, "Failed to set lowerdir=%s: %m", options); - if (fsconfig(overlay_fs_fd, FSCONFIG_SET_STRING, "source", "systemd-extensions", 0) < 0) + if (fsconfig(overlay_fs_fd, FSCONFIG_SET_STRING, "source", "systemd-extensions", /* aux= */ 0) < 0) return log_debug_errno(errno, "Failed to set source=systemd-extensions: %m"); /* Create the superblock */ - if (fsconfig(overlay_fs_fd, FSCONFIG_CMD_CREATE, NULL, NULL, 0) < 0) + if (fsconfig(overlay_fs_fd, FSCONFIG_CMD_CREATE, /* key= */ NULL, /* value= */ NULL, /* aux= */ 0) < 0) return log_debug_errno(errno, "Failed to create overlay superblock: %m"); /* Signal completion to grandchild */ diff --git a/src/home/homework-luks.c b/src/home/homework-luks.c index a605babb7d0..af938741e1c 100644 --- a/src/home/homework-luks.c +++ b/src/home/homework-luks.c @@ -2558,7 +2558,7 @@ int home_create_luks( setup->loop = loop_device_unref(setup->loop); if (!user_record_luks_offline_discard(h)) { - r = run_fallocate(setup->image_fd, NULL /* refresh stat() data */); + r = run_fallocate(setup->image_fd, /* st= */ NULL /* refresh stat() data */); if (r < 0) return r; } diff --git a/src/home/homework-password-cache.c b/src/home/homework-password-cache.c index fb205153bce..a2e911d620e 100644 --- a/src/home/homework-password-cache.c +++ b/src/home/homework-password-cache.c @@ -30,7 +30,7 @@ void password_cache_load_keyring(UserRecord *h, PasswordCache *cache) { if (!name) return (void) log_oom(); - serial = request_key("user", name, NULL, 0); + serial = request_key("user", name, /* callout_info= */ NULL, /* destringid= */ 0); if (serial == -1) { if (errno == ENOKEY) { log_info("Home volume key is not available in kernel keyring."); diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c index 2bb34631c44..fadb2d6eda7 100644 --- a/src/hostname/hostnamed.c +++ b/src/hostname/hostnamed.c @@ -2619,7 +2619,7 @@ static int vl_method_apply_tags(sd_varlink *link, sd_json_variant *parameters, s return r; if (strv_equal(tags, current)) - return sd_varlink_reply(link, NULL); + return sd_varlink_reply(link, /* parameters= */ NULL); r = varlink_verify_polkit_async( link, @@ -2634,7 +2634,7 @@ static int vl_method_apply_tags(sd_varlink *link, sd_json_variant *parameters, s if (r < 0) return r; - return sd_varlink_reply(link, NULL); + return sd_varlink_reply(link, /* parameters= */ NULL); } static int connect_varlink(Context *c) { diff --git a/src/include/override/sys/mount.h b/src/include/override/sys/mount.h index ba424a9e00d..763c2ff1464 100644 --- a/src/include/override/sys/mount.h +++ b/src/include/override/sys/mount.h @@ -46,7 +46,7 @@ int fsopen_shim(const char *fsname, unsigned flags); FLAGS with ATTR_FLAGS describing how the mount is to be performed. */ /* Defined since glibc-2.36. * Supported since kernel v5.2 (93766fbd2696c2c4453dd8e1070977e9cd4e6b6d). */ -int fsmount_shim(int fd, unsigned flags, unsigned ms_flags); +int fsmount_shim(int fsfd, unsigned flags, unsigned attr_flags); #define fsmount fsmount_shim /* Add the mounted FROM_DFD referenced by FROM_PATHNAME filesystem returned diff --git a/src/libc/mount.c b/src/libc/mount.c index 3e626e87acd..0d642101872 100644 --- a/src/libc/mount.c +++ b/src/libc/mount.c @@ -9,9 +9,9 @@ DEFINE_SYSCALL_SHIM(fsopen, int, unsigned, flags) DEFINE_SYSCALL_SHIM(fsmount, int, - int, fd, + int, fsfd, unsigned, flags, - unsigned, ms_flags) + unsigned, attr_flags) DEFINE_SYSCALL_SHIM(move_mount, int, int, from_dfd, diff --git a/src/libsystemd-network/test-dhcp-server.c b/src/libsystemd-network/test-dhcp-server.c index db2cf0c7d48..1a5cb7ec34b 100644 --- a/src/libsystemd-network/test-dhcp-server.c +++ b/src/libsystemd-network/test-dhcp-server.c @@ -493,7 +493,7 @@ TEST(dhcp_server_static_lease_special_addresses) { _cleanup_(sd_dhcp_server_unrefp) sd_dhcp_server *server = NULL; ASSERT_OK(sd_dhcp_server_new(&server, 4242)); ASSERT_OK(sd_dhcp_server_configure_pool(server, &(struct in_addr) { htobe32(INADDR_LOOPBACK) }, - 8, 0, 0)); + 8, /* offset= */ 0, /* size= */ 0)); sd_dhcp_client_id client_id_subnet = { .size = 7, diff --git a/src/libsystemd-network/test-dhcp6-client.c b/src/libsystemd-network/test-dhcp6-client.c index dc613ad1ebd..ac9fc97c0de 100644 --- a/src/libsystemd-network/test-dhcp6-client.c +++ b/src/libsystemd-network/test-dhcp6-client.c @@ -411,7 +411,7 @@ TEST(lease_timezone) { /* "UTC" in the main test messages bypasses the tzfile check in * verify_timezone(); exercise the validation with invalid inputs. */ - ASSERT_ERROR(dhcp6_lease_set_timezone(lease, (const uint8_t *) "", 0), EINVAL); + ASSERT_ERROR(dhcp6_lease_set_timezone(lease, (const uint8_t *) "", /* optlen= */ 0), EINVAL); ASSERT_ERROR(dhcp6_lease_set_timezone(lease, (const uint8_t *) "UTC\0", 4), EINVAL); ASSERT_ERROR(dhcp6_lease_set_timezone(lease, (const uint8_t *) "/etc/passwd", 11), EINVAL); ASSERT_ERROR(dhcp6_lease_set_timezone(lease, (const uint8_t *) "../passwd", 9), EINVAL); diff --git a/src/libsystemd/sd-daemon/sd-daemon.c b/src/libsystemd/sd-daemon/sd-daemon.c index ba1cfedf0f9..1819f03501e 100644 --- a/src/libsystemd/sd-daemon/sd-daemon.c +++ b/src/libsystemd/sd-daemon/sd-daemon.c @@ -679,7 +679,7 @@ _public_ int sd_pid_notify_barrier(pid_t pid, int unset_environment, uint64_t ti pipe_fd[1] = safe_close(pipe_fd[1]); - r = fd_wait_for_event(pipe_fd[0], 0 /* POLLHUP is implicit */, timeout); + r = fd_wait_for_event(pipe_fd[0], /* event= */ 0 /* POLLHUP is implicit */, timeout); if (r < 0) goto finish; if (r == 0) { diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 888b85a1df3..5ff04db13c0 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -2973,7 +2973,7 @@ _public_ int sd_device_open(sd_device *device, int flags) { /* We've historically returned non-cloexec fds. */ if (!FLAGS_SET(flags, O_CLOEXEC)) { - r = fd_cloexec(fd, false); + r = fd_cloexec(fd, /* cloexec= */ false); if (r < 0) return r; } diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c index 400c89e13ff..40998a4ab13 100644 --- a/src/libsystemd/sd-event/sd-event.c +++ b/src/libsystemd/sd-event/sd-event.c @@ -1232,7 +1232,7 @@ static void source_disown(sd_event_source *s) { assert(s->child.pidfd >= 0); if (!s->child.exited) { - r = RET_NERRNO(pidfd_send_signal(s->child.pidfd, SIGKILL, NULL, 0)); + r = RET_NERRNO(pidfd_send_signal(s->child.pidfd, SIGKILL, /* info= */ NULL, /* flags= */ 0)); if (r < 0 && r != -ESRCH) log_debug_errno(r, "Failed to kill process " PID_FMT ", ignoring: %m", s->child.pid); @@ -1798,7 +1798,7 @@ _public_ int sd_event_add_child( /* We always take a pidfd here, even if we wait for anything else than WEXITED, so that we pin the * PID, and make regular waitid() handling race-free. */ - s->child.pidfd = pidfd_open(pid, 0); + s->child.pidfd = pidfd_open(pid, /* flags= */ 0); if (s->child.pidfd < 0) return -errno; diff --git a/src/libsystemd/sd-event/test-event-future.c b/src/libsystemd/sd-event/test-event-future.c index dd13f05289f..9e9310ac8f8 100644 --- a/src/libsystemd/sd-event/test-event-future.c +++ b/src/libsystemd/sd-event/test-event-future.c @@ -905,7 +905,7 @@ static int io_uring_recv_fiber(void *userdata) { return r; st->got = sd_future_result(f); - return sd_event_exit(e, 0); + return sd_event_exit(e, /* code= */ 0); } TEST(io_uring_future_recv) { diff --git a/src/libsystemd/sd-event/test-event.c b/src/libsystemd/sd-event/test-event.c index a8898747ca9..b0839cdc4c6 100644 --- a/src/libsystemd/sd-event/test-event.c +++ b/src/libsystemd/sd-event/test-event.c @@ -580,7 +580,7 @@ TEST(pidfd) { /* child */ _exit(66); - ASSERT_OK(pidfd = pidfd_open(pid, 0)); + ASSERT_OK(pidfd = pidfd_open(pid, /* flags= */ 0)); ASSERT_OK_ERRNO(pid2 = fork()); if (pid2 == 0) @@ -1131,7 +1131,7 @@ TEST(child_pidfd_wnowait) { _exit(42); _cleanup_close_ int pidfd = -EBADF; - ASSERT_OK_ERRNO(pidfd = pidfd_open(pid, 0)); + ASSERT_OK_ERRNO(pidfd = pidfd_open(pid, /* flags= */ 0)); /* Add a child source with WNOWAIT flag */ _cleanup_(sd_event_source_unrefp) sd_event_source *s = NULL; @@ -1364,7 +1364,7 @@ TEST(io_uring_future_many) { ASSERT_OK(sd_event_new(&e)); /* The ring can be created after the loop ran. */ - ASSERT_OK(sd_event_run(e, 0)); + ASSERT_OK(sd_event_run(e, /* timeout= */ 0)); if (!io_uring_available_or_skip(e)) return; @@ -1466,10 +1466,10 @@ TEST(io_uring_future_cancel_after_fork) { ASSERT_OK(add_recv(e, sv[0], buf, sizeof(buf), &f)); /* sd_event_run() submits the SQE. Nobody writes to the socket, so the recv() stays armed. */ - ASSERT_OK(sd_event_run(e, 0)); + ASSERT_OK(sd_event_run(e, /* timeout= */ 0)); ASSERT_EQ(sd_future_state(f), SD_FUTURE_PENDING); - r = ASSERT_OK(pidref_safe_fork("(future-cancel)", FORK_WAIT|FORK_LOG, NULL)); + r = ASSERT_OK(pidref_safe_fork("(future-cancel)", FORK_WAIT|FORK_LOG, /* ret= */ NULL)); if (r == 0) { ASSERT_ERROR(ASSERT_RETURN_EXPECTED(sd_future_cancel(f)), ECHILD); _exit(EXIT_SUCCESS); @@ -1501,7 +1501,7 @@ TEST(io_uring_external_poll_loop) { .events = POLLIN, }, 1, 5000), 1); - r = ASSERT_OK(sd_event_wait(e, 0)); + r = ASSERT_OK(sd_event_wait(e, /* timeout= */ 0)); if (r > 0) ASSERT_OK(sd_event_dispatch(e)); @@ -1552,7 +1552,7 @@ TEST(io_uring_wait_fd_during_exit) { _cleanup_(sd_future_unrefp) sd_future *f = NULL; ASSERT_OK(add_recv(e, sv[0], buf, sizeof(buf), &f)); - ASSERT_OK(sd_event_exit(e, 0)); + ASSERT_OK(sd_event_exit(e, /* code= */ 0)); ASSERT_OK(event_get_wait_fd(e)); ASSERT_NE(event_get_wait_fd(e), sd_event_get_fd(e)); @@ -1590,12 +1590,12 @@ TEST(io_uring_exit_between_wait_and_dispatch) { ASSERT_OK(add_recv(e, sv[0], buf, sizeof(buf), &f)); _cleanup_(sd_event_source_unrefp) sd_event_source *d = NULL, *x = NULL; - ASSERT_OK(sd_event_add_defer(e, &d, defer_noop, NULL)); + ASSERT_OK(sd_event_add_defer(e, &d, defer_noop, /* userdata= */ NULL)); ASSERT_OK(sd_event_add_exit(e, &x, exit_check_resolved, f)); /* The defer source is pending, so sd_event_prepare() reports a pending event. */ ASSERT_OK_POSITIVE(sd_event_prepare(e)); - ASSERT_OK(sd_event_exit(e, 0)); + ASSERT_OK(sd_event_exit(e, /* code= */ 0)); ASSERT_OK_POSITIVE(sd_event_dispatch(e)); ASSERT_OK(sd_event_loop(e)); @@ -1681,13 +1681,13 @@ TEST(io_uring_drain_on_exit_skips_ratelimit_expiry) { ASSERT_OK(add_recv(e, sv[0], buf, sizeof(buf), &f)); while (sd_event_source_is_ratelimited(s) == 0) - ASSERT_OK(sd_event_run(e, 0)); + ASSERT_OK(sd_event_run(e, /* timeout= */ 0)); /* The ratelimit window ends during the sleep. Timer processing after the exit would then run the * ratelimit expiry callback. */ ASSERT_OK(usleep_safe(20 * USEC_PER_MSEC)); - ASSERT_OK(sd_event_exit(e, 0)); + ASSERT_OK(sd_event_exit(e, /* code= */ 0)); ASSERT_OK(sd_event_loop(e)); ASSERT_ERROR(sd_future_result(f), ECANCELED); diff --git a/src/libsystemd/sd-future/test-fiber-io.c b/src/libsystemd/sd-future/test-fiber-io.c index bd4c1fa1bcd..dcd51c7ad26 100644 --- a/src/libsystemd/sd-future/test-fiber-io.c +++ b/src/libsystemd/sd-future/test-fiber-io.c @@ -266,7 +266,7 @@ TEST(fiber_io_cancel_after_data) { CancelledRead c = { .fd = sockfd[0] }; _cleanup_(sd_future_unrefp) sd_future *f = NULL; ASSERT_OK(sd_fiber_new(e, "cancelled-recv", cancelled_recv_fiber, &c, &f)); - ASSERT_OK_POSITIVE(sd_event_run(e, 0)); + ASSERT_OK_POSITIVE(sd_event_run(e, /* timeout= */ 0)); /* The data arrives before the fiber runs again to see the cancellation. */ ASSERT_OK_EQ_ERRNO(write(sockfd[1], "x", 1), 1); @@ -306,9 +306,9 @@ TEST(fiber_io_after_exit) { _cleanup_(sd_future_unrefp) sd_future *f = NULL; ASSERT_OK(sd_fiber_new(e, "send-after-exit", send_after_exit_fiber, INT_TO_PTR(sockfd[1]), &f)); - ASSERT_OK_POSITIVE(sd_event_run(e, 0)); + ASSERT_OK_POSITIVE(sd_event_run(e, /* timeout= */ 0)); - ASSERT_OK(sd_event_exit(e, 0)); + ASSERT_OK(sd_event_exit(e, /* code= */ 0)); ASSERT_OK(sd_event_loop(e)); ASSERT_OK_ZERO(sd_future_result(f)); @@ -359,9 +359,9 @@ TEST(fiber_io_exit_order) { ASSERT_OK(sd_event_add_exit(e, &s, exit_order_handler, &o)); /* The fiber suspends in sd_fiber_recv(). */ - ASSERT_OK_POSITIVE(sd_event_run(e, 0)); + ASSERT_OK_POSITIVE(sd_event_run(e, /* timeout= */ 0)); - ASSERT_OK(sd_event_exit(e, 0)); + ASSERT_OK(sd_event_exit(e, /* code= */ 0)); ASSERT_OK(sd_event_loop(e)); ASSERT_OK_ZERO(sd_future_result(f)); @@ -444,7 +444,7 @@ TEST(fiber_io_pty) { /* The read fiber has the higher priority, so it waits on the pty before the write fiber runs. */ _cleanup_(sd_future_unrefp) sd_future *fr = NULL, *fw = NULL; ASSERT_OK(sd_fiber_new(e, "pty-read", pty_read_fiber, INT_TO_PTR(slave), &fr)); - ASSERT_OK(sd_future_set_priority(fr, 0)); + ASSERT_OK(sd_future_set_priority(fr, /* priority= */ 0)); ASSERT_OK(sd_fiber_new(e, "pty-write", pty_write_fiber, INT_TO_PTR(master), &fw)); ASSERT_OK(sd_future_set_priority(fw, 1)); @@ -748,7 +748,7 @@ TEST(fiber_io_recv_dontwait) { * runs. */ _cleanup_(sd_future_unrefp) sd_future *fr = NULL, *fs = NULL; ASSERT_OK(sd_fiber_new(e, "socket-recv-dontwait", socket_recv_dontwait_fiber, INT_TO_PTR(sockfd[1]), &fr)); - ASSERT_OK(sd_future_set_priority(fr, 0)); + ASSERT_OK(sd_future_set_priority(fr, /* priority= */ 0)); ASSERT_OK(sd_fiber_new(e, "socket-send-dontwait", socket_send_dontwait_fiber, INT_TO_PTR(sockfd[0]), &fs)); ASSERT_OK(sd_future_set_priority(fs, 1)); @@ -1828,7 +1828,7 @@ static int shutdown_reader_fiber(void *userdata) { } static int on_shutdown_timer(sd_event_source *s, uint64_t usec, void *userdata) { - return sd_event_exit(sd_event_source_get_event(s), 0); + return sd_event_exit(sd_event_source_get_event(s), /* code= */ 0); } /* Fibers doing I/O into stack-allocated buffers while a timer fires sd_event_exit(). Every fiber @@ -1853,8 +1853,8 @@ TEST(fiber_io_shutdown_during_work) { /* Short delay so the fibers actually transfer something before the teardown. */ _cleanup_(sd_event_source_unrefp) sd_event_source *timer = NULL; - ASSERT_OK(sd_event_add_time_relative(e, &timer, CLOCK_MONOTONIC, 50 * USEC_PER_MSEC, 0, - on_shutdown_timer, NULL)); + ASSERT_OK(sd_event_add_time_relative(e, &timer, CLOCK_MONOTONIC, 50 * USEC_PER_MSEC, /* accuracy= */ 0, + on_shutdown_timer, /* userdata= */ NULL)); ASSERT_OK(sd_event_loop(e)); diff --git a/src/libsystemd/sd-id128/sd-id128.c b/src/libsystemd/sd-id128/sd-id128.c index 46aa7ac5438..a4d62635dce 100644 --- a/src/libsystemd/sd-id128/sd-id128.c +++ b/src/libsystemd/sd-id128/sd-id128.c @@ -214,7 +214,7 @@ static int get_invocation_from_keyring(sd_id128_t *ret) { assert(ret); - key = request_key("user", "invocation_id", NULL, 0); + key = request_key("user", "invocation_id", /* callout_info= */ NULL, /* destringid= */ 0); if (key == -1) { /* Keyring support not available? Keyring access locked down? No invocation key stored? */ if (ERRNO_IS_NOT_SUPPORTED(errno) || diff --git a/src/libsystemd/sd-login/test-login.c b/src/libsystemd/sd-login/test-login.c index 4c7b2776ad2..f625055e1fe 100644 --- a/src/libsystemd/sd-login/test-login.c +++ b/src/libsystemd/sd-login/test-login.c @@ -77,7 +77,7 @@ TEST(login) { log_info("sd_pid_get_cgroup(0, …) → %s / \"%s\"", e(r), strnull(cgroup)); ASSERT_TRUE(IN_SET(r, 0, -ENOMEDIUM)); - pidfd = pidfd_open(getpid_cached(), 0); + pidfd = pidfd_open(getpid_cached(), /* flags= */ 0); if (pidfd >= 0) { _cleanup_free_ char *cgroup2 = NULL, *session2 = NULL, *unit2 = NULL, *user_unit2 = NULL, *slice2 = NULL; diff --git a/src/libsystemd/sd-varlink/test-varlink-idl.c b/src/libsystemd/sd-varlink/test-varlink-idl.c index 52908e0c8f3..0050ad10a97 100644 --- a/src/libsystemd/sd-varlink/test-varlink-idl.c +++ b/src/libsystemd/sd-varlink/test-varlink-idl.c @@ -694,13 +694,13 @@ TEST(split_qualified_symbol_name) { &name)); ASSERT_STREQ(name, "GetInfo"); - ASSERT_ERROR(varlink_idl_split_qualified_symbol_name(/* symbol= */ NULL, NULL, NULL), EINVAL); - ASSERT_ERROR(varlink_idl_split_qualified_symbol_name("", NULL, NULL), EINVAL); - ASSERT_ERROR(varlink_idl_split_qualified_symbol_name("Foo", NULL, NULL), EINVAL); - ASSERT_ERROR(varlink_idl_split_qualified_symbol_name("xxx.xxx", NULL, NULL), EINVAL); - ASSERT_ERROR(varlink_idl_split_qualified_symbol_name(".Foo", NULL, NULL), EINVAL); - ASSERT_ERROR(varlink_idl_split_qualified_symbol_name("xxx.xxx.0foo", NULL, NULL), EINVAL); - ASSERT_ERROR(varlink_idl_split_qualified_symbol_name("xxx..Foo", NULL, NULL), EINVAL); + ASSERT_ERROR(varlink_idl_split_qualified_symbol_name(/* symbol= */ NULL, /* ret_interface= */ NULL, /* ret_name= */ NULL), EINVAL); + ASSERT_ERROR(varlink_idl_split_qualified_symbol_name("", /* ret_interface= */ NULL, /* ret_name= */ NULL), EINVAL); + ASSERT_ERROR(varlink_idl_split_qualified_symbol_name("Foo", /* ret_interface= */ NULL, /* ret_name= */ NULL), EINVAL); + ASSERT_ERROR(varlink_idl_split_qualified_symbol_name("xxx.xxx", /* ret_interface= */ NULL, /* ret_name= */ NULL), EINVAL); + ASSERT_ERROR(varlink_idl_split_qualified_symbol_name(".Foo", /* ret_interface= */ NULL, /* ret_name= */ NULL), EINVAL); + ASSERT_ERROR(varlink_idl_split_qualified_symbol_name("xxx.xxx.0foo", /* ret_interface= */ NULL, /* ret_name= */ NULL), EINVAL); + ASSERT_ERROR(varlink_idl_split_qualified_symbol_name("xxx..Foo", /* ret_interface= */ NULL, /* ret_name= */ NULL), EINVAL); } TEST(validate_json) { diff --git a/src/login/pam_systemd.c b/src/login/pam_systemd.c index 5908fcba13e..d63d3bf1dbb 100644 --- a/src/login/pam_systemd.c +++ b/src/login/pam_systemd.c @@ -865,7 +865,7 @@ static int create_session_message( assert(ret); if (!avoid_pidfd) { - pidfd = pidfd_open(getpid_cached(), 0); + pidfd = pidfd_open(getpid_cached(), /* flags= */ 0); if (pidfd < 0) return -errno; } diff --git a/src/login/pam_systemd_loadkey.c b/src/login/pam_systemd_loadkey.c index 41afb8ba239..6d095bf0ae8 100644 --- a/src/login/pam_systemd_loadkey.c +++ b/src/login/pam_systemd_loadkey.c @@ -50,7 +50,7 @@ _public_ PAM_EXTERN int pam_sm_authenticate( /* Retrieve the key. */ key_serial_t serial; - serial = request_key("user", keyname, NULL, 0); + serial = request_key("user", keyname, /* callout_info= */ NULL, /* destringid= */ 0); if (serial < 0) { if (errno == ENOKEY) { pam_debug_syslog(pamh, debug, "Key not found: %s", keyname); diff --git a/src/network/networkd-link-varlink.c b/src/network/networkd-link-varlink.c index 666a4ef2d38..ce5d7a17d6d 100644 --- a/src/network/networkd-link-varlink.c +++ b/src/network/networkd-link-varlink.c @@ -255,10 +255,10 @@ static int dispatch_resolve_link(sd_varlink *vlink, sd_json_variant *parameters, return sd_varlink_error_invalid_parameter(vlink, JSON_VARIANT_STRING_CONST("ifindex")); if (FLAGS_SET(link->flags, IFF_LOOPBACK)) - return sd_varlink_error(vlink, "io.systemd.Resolve.Link.IsLoopback", NULL); + return sd_varlink_error(vlink, "io.systemd.Resolve.Link.IsLoopback", /* parameters= */ NULL); if (link->state == LINK_STATE_UNMANAGED) - return sd_varlink_error(vlink, "io.systemd.Network.Link.InterfaceUnmanaged", NULL); + return sd_varlink_error(vlink, "io.systemd.Network.Link.InterfaceUnmanaged", /* parameters= */ NULL); *ret = link; return 0; @@ -327,5 +327,5 @@ int vl_method_link_set_dns(sd_varlink *vlink, sd_json_variant *parameters, sd_va if (r < 0) return r; - return sd_varlink_reply(vlink, NULL); + return sd_varlink_reply(vlink, /* parameters= */ NULL); } diff --git a/src/network/networkd-wwan.c b/src/network/networkd-wwan.c index f5266cf3fac..54f420685e7 100644 --- a/src/network/networkd-wwan.c +++ b/src/network/networkd-wwan.c @@ -681,7 +681,7 @@ int config_parse_mm_allowed_auth( for (const char *p = rvalue;;) { _cleanup_free_ char *auth = NULL; - r = extract_first_word(&p, &auth, /* separators */ NULL, /* flags */ 0); + r = extract_first_word(&p, &auth, /* separators= */ NULL, /* flags= */ 0); if (r < 0) return log_syntax_parse_error(unit, filename, line, r, lvalue, rvalue); if (r == 0) diff --git a/src/nsresourced/test-userns-restrict.c b/src/nsresourced/test-userns-restrict.c index bd64d777255..64921501cdf 100644 --- a/src/nsresourced/test-userns-restrict.c +++ b/src/nsresourced/test-userns-restrict.c @@ -43,9 +43,9 @@ static int make_tmpfs_fsmount(void) { _cleanup_close_ int fsfd = -EBADF, mntfd = -EBADF; fsfd = ASSERT_OK_ERRNO(fsopen("tmpfs", FSOPEN_CLOEXEC)); - ASSERT_OK_ERRNO(fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0)); + ASSERT_OK_ERRNO(fsconfig(fsfd, FSCONFIG_CMD_CREATE, /* key= */ NULL, /* value= */ NULL, /* aux= */ 0)); - mntfd = ASSERT_OK_ERRNO(fsmount(fsfd, FSMOUNT_CLOEXEC, 0)); + mntfd = ASSERT_OK_ERRNO(fsmount(fsfd, FSMOUNT_CLOEXEC, /* attr_flags= */ 0)); return TAKE_FD(mntfd); } @@ -984,7 +984,7 @@ TEST_RET(userns_mqueue_lifetime) { rb = sym_ring_buffer__new(sym_bpf_map__fd(bpf_obj->maps.userns_ringbuf), ringbuf_userns_freed, &inode, /* opts= */ NULL); ASSERT_NOT_NULL(rb); - ASSERT_OK(sym_ring_buffer__poll(rb, 0)); + ASSERT_OK(sym_ring_buffer__poll(rb, /* timeout_ms= */ 0)); inode = (unsigned) userns_st.st_ino; ASSERT_NE(inode, 0U); diff --git a/src/oom/test-oomd-util.c b/src/oom/test-oomd-util.c index 397ce3ad3fd..6eed136019d 100644 --- a/src/oom/test-oomd-util.c +++ b/src/oom/test-oomd-util.c @@ -172,7 +172,7 @@ TEST(oomd_cgroup_kill) { * by the test so that pid1 doesn't delete it before we can read the xattrs. */ ASSERT_NOT_NULL(subcgroup = path_join(cgroup, "oomdkilltest")); /* Always start clean, in case of repeated runs and failures */ - ASSERT_OK(cg_trim(subcgroup, /* delete_root */ true)); + ASSERT_OK(cg_trim(subcgroup, /* delete_root= */ true)); ASSERT_OK(cg_create(subcgroup)); /* If we don't have permissions to set xattrs we're likely in a userns or missing capabilities */ @@ -216,7 +216,7 @@ TEST(oomd_cgroup_kill) { ASSERT_STREQ(v, i == 0 ? "2" : "4"); } - ASSERT_OK(cg_trim(subcgroup, /* delete_root */ true)); + ASSERT_OK(cg_trim(subcgroup, /* delete_root= */ true)); } TEST(oomd_cgroup_kill_signal_reason) { diff --git a/src/repart/repart.c b/src/repart/repart.c index 081385ccbe2..7e1fa67189a 100644 --- a/src/repart/repart.c +++ b/src/repart/repart.c @@ -9050,7 +9050,7 @@ static int context_manifest_gpt(Context *context) { /* Then an object for the whole image, i.e. the complete disk including the partition table */ uint8_t image_hash[SHA256_DIGEST_SIZE]; - r = hash_range(fd, 0, context->total, image_hash); + r = hash_range(fd, /* offset= */ 0, context->total, image_hash); if (r < 0) return r; diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 055d0db7fcb..1af81096387 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -1360,7 +1360,7 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p, bool encrypt dns_server_feature_level_to_string(t->clamp_feature_level_servfail)); } - dns_transaction_retry(t, false /* use the same server */); + dns_transaction_retry(t, /* next_server= */ false /* use the same server */); return; } diff --git a/src/resolve/resolved-varlink.c b/src/resolve/resolved-varlink.c index d115e4e485f..596fbc86e74 100644 --- a/src/resolve/resolved-varlink.c +++ b/src/resolve/resolved-varlink.c @@ -1489,10 +1489,10 @@ static int vl_get_link(sd_varlink *vlink, int ifindex, Link **ret_link) { return sd_varlink_error_invalid_parameter(vlink, JSON_VARIANT_STRING_CONST("ifindex")); if (FLAGS_SET(link->flags, IFF_LOOPBACK)) - return sd_varlink_error(vlink, "io.systemd.Resolve.Link.IsLoopback", NULL); + return sd_varlink_error(vlink, "io.systemd.Resolve.Link.IsLoopback", /* parameters= */ NULL); if (link->is_managed) - return sd_varlink_error(vlink, "io.systemd.Resolve.Link.ManagedByNetworkd", NULL); + return sd_varlink_error(vlink, "io.systemd.Resolve.Link.ManagedByNetworkd", /* parameters= */ NULL); *ret_link = link; return 0; @@ -1555,7 +1555,7 @@ static int vl_method_link_set_dns(sd_varlink *link, sd_json_variant *parameters, if (r < 0) return r; - return sd_varlink_reply(link, NULL); + return sd_varlink_reply(link, /* parameters= */ NULL); } static int varlink_monitor_server_init(Manager *m) { diff --git a/src/resolve/test-dns-synthesize.c b/src/resolve/test-dns-synthesize.c index 2f8a427389d..ec8626970cf 100644 --- a/src/resolve/test-dns-synthesize.c +++ b/src/resolve/test-dns-synthesize.c @@ -316,9 +316,9 @@ TEST(dns_synthesize_answer_onion) { key = dns_resource_key_new(DNS_CLASS_IN, type, "mysecret.onion"); ASSERT_NOT_NULL(key); - ASSERT_OK(dns_question_add(question, key, 0)); + ASSERT_OK(dns_question_add(question, key, /* flags= */ 0)); - ASSERT_ERROR(dns_synthesize_answer(&manager, question, 0, &answer), ENXIO); + ASSERT_ERROR(dns_synthesize_answer(&manager, question, /* ifindex= */ 0, &answer), ENXIO); ASSERT_NULL(answer); } } diff --git a/src/run/run-polkit.c b/src/run/run-polkit.c index 0b400f0bc75..79996ab40fc 100644 --- a/src/run/run-polkit.c +++ b/src/run/run-polkit.c @@ -36,7 +36,7 @@ int polkit_check_authorization(sd_bus *bus, PolkitFlags flags, char **ret_tmpaut pid = getpid_cached(); /* Polkit requires pidfd to honor temporary authorizations */ - pidfd = pidfd_open(pid, 0); + pidfd = pidfd_open(pid, /* flags= */ 0); if (pidfd < 0) return log_debug_errno(errno, "pidfd_open failed: %m"); diff --git a/src/shared/bpf-util.h b/src/shared/bpf-util.h index 88457483f90..9a212a2e53d 100644 --- a/src/shared/bpf-util.h +++ b/src/shared/bpf-util.h @@ -96,7 +96,7 @@ static inline int compat_bpf_map_create( value_size, max_entries, opts); return sym_bpf_create_map(map_type, key_size, value_size, max_entries, - 0 /* opts->map_flags, but opts is always NULL for us so skip build dependency on the type */); + /* map_flags= */ 0 /* opts->map_flags, but opts is always NULL for us so skip build dependency on the type */); } /* libbpf sometimes returns error codes that make sense only in the kernel, like 524 for EOPNOTSUPP. Use diff --git a/src/shared/bus-wait-for-units.c b/src/shared/bus-wait-for-units.c index 7293bf90653..ac65e027a58 100644 --- a/src/shared/bus-wait-for-units.c +++ b/src/shared/bus-wait-for-units.c @@ -246,7 +246,7 @@ static void bus_wait_for_units_check_ready(BusWaitForUnits *d) { d->state = d->has_failed ? BUS_WAIT_FAILURE : BUS_WAIT_SUCCESS; if (d->event) - (void) sd_event_exit(d->event, 0); + (void) sd_event_exit(d->event, /* code= */ 0); } static void wait_for_item_fail(WaitForItem *item) { diff --git a/src/shared/data-fd-util.c b/src/shared/data-fd-util.c index 4405bf3c047..6b1041e0f3c 100644 --- a/src/shared/data-fd-util.c +++ b/src/shared/data-fd-util.c @@ -85,7 +85,7 @@ int copy_data_fd_full(int fd, uint64_t max_size) { /* If we have reason to believe this will fit fine in /tmp, then use that as first fallback. */ if ((!S_ISREG(st.st_mode) || (uint64_t) st.st_size < DATA_FD_TMP_LIMIT)) { - tmp_fd = open_tmpfile_unlinkable(NULL /* NULL as directory means /tmp */, O_RDWR); + tmp_fd = open_tmpfile_unlinkable(/* directory= */ NULL /* NULL as directory means /tmp */, O_RDWR); if (tmp_fd < 0) return tmp_fd; diff --git a/src/shared/dns-packet.c b/src/shared/dns-packet.c index 74b72c081f4..12a67f7405d 100644 --- a/src/shared/dns-packet.c +++ b/src/shared/dns-packet.c @@ -2399,7 +2399,7 @@ int dns_packet_read_rr( if (r < 0) return r; - r = dns_packet_read_name(p, &rr->svcb.target_name, false /* uncompressed */, /* start= */ NULL); + r = dns_packet_read_name(p, &rr->svcb.target_name, /* allow_compression= */ false, /* start= */ NULL); if (r < 0) return r; diff --git a/src/shared/image-policy.c b/src/shared/image-policy.c index cfabd3042ce..436a5616a77 100644 --- a/src/shared/image-policy.c +++ b/src/shared/image-policy.c @@ -799,7 +799,7 @@ int config_parse_image_policy( return 0; } - r = image_policy_from_string(rvalue, /* graceful */ true, &np); + r = image_policy_from_string(rvalue, /* graceful= */ true, &np); if (r == -ENOTUNIQ) return log_syntax(unit, LOG_ERR, filename, line, r, "Duplicate rule in image policy, refusing: %s", rvalue); if (r == -EBADSLT) diff --git a/src/shared/mount-util.c b/src/shared/mount-util.c index 821f190a528..61db151f399 100644 --- a/src/shared/mount-util.c +++ b/src/shared/mount-util.c @@ -2021,14 +2021,14 @@ int fsmount_credentials_fs(int *ret_fsfd) { if (fs_fd < 0) return -errno; - if (fsconfig(fs_fd, FSCONFIG_SET_STRING, "nr_inodes", "1024", 0) < 0) + if (fsconfig(fs_fd, FSCONFIG_SET_STRING, "nr_inodes", "1024", /* aux= */ 0) < 0) return -errno; xsprintf(size_str, "%" PRIu64, CREDENTIALS_TOTAL_SIZE_MAX); - if (fsconfig(fs_fd, FSCONFIG_SET_STRING, "size", size_str, 0) < 0) + if (fsconfig(fs_fd, FSCONFIG_SET_STRING, "size", size_str, /* aux= */ 0) < 0) return -errno; - if (fsconfig(fs_fd, FSCONFIG_SET_FLAG, "noswap", NULL, 0) < 0) { + if (fsconfig(fs_fd, FSCONFIG_SET_FLAG, "noswap", /* value= */ NULL, /* aux= */ 0) < 0) { if (errno != EINVAL) return -errno; @@ -2037,10 +2037,10 @@ int fsmount_credentials_fs(int *ret_fsfd) { close_and_replace(fs_fd, ramfs_fd); } - if (fsconfig(fs_fd, FSCONFIG_SET_STRING, "mode", "0700", 0) < 0) + if (fsconfig(fs_fd, FSCONFIG_SET_STRING, "mode", "0700", /* aux= */ 0) < 0) return -errno; - if (fsconfig(fs_fd, FSCONFIG_CMD_CREATE, NULL, NULL, 0) < 0) + if (fsconfig(fs_fd, FSCONFIG_CMD_CREATE, /* key= */ NULL, /* value= */ NULL, /* aux= */ 0) < 0) return -errno; unsigned mount_attrs = ms_flags_to_mount_attr(credentials_fs_mount_flags(/* ro = */ false)); @@ -2109,11 +2109,11 @@ int make_fsmount( if (fs_fd < 0) return log_full_errno(error_log_level, errno, "Failed to open superblock for \"%s\": %m", type); - if (fsconfig(fs_fd, FSCONFIG_SET_STRING, "source", what, 0) < 0) + if (fsconfig(fs_fd, FSCONFIG_SET_STRING, "source", what, /* aux= */ 0) < 0) return log_full_errno(error_log_level, errno, "Failed to set mount source for \"%s\" to \"%s\": %m", type, what); if (FLAGS_SET(f, MS_RDONLY)) - if (fsconfig(fs_fd, FSCONFIG_SET_FLAG, "ro", NULL, 0) < 0) + if (fsconfig(fs_fd, FSCONFIG_SET_FLAG, "ro", /* value= */ NULL, /* aux= */ 0) < 0) return log_full_errno(error_log_level, errno, "Failed to set read only mount flag for \"%s\": %m", type); for (const char *p = o;;) { @@ -2131,18 +2131,18 @@ int make_fsmount( *eq = 0; eq++; - if (fsconfig(fs_fd, FSCONFIG_SET_STRING, word, eq, 0) < 0) + if (fsconfig(fs_fd, FSCONFIG_SET_STRING, word, eq, /* aux= */ 0) < 0) return log_full_errno(error_log_level, errno, "Failed to set mount option \"%s=%s\" for \"%s\": %m", word, eq, type); } else { - if (fsconfig(fs_fd, FSCONFIG_SET_FLAG, word, NULL, 0) < 0) + if (fsconfig(fs_fd, FSCONFIG_SET_FLAG, word, /* value= */ NULL, /* aux= */ 0) < 0) return log_full_errno(error_log_level, errno, "Failed to set mount flag \"%s\" for \"%s\": %m", word, type); } } - if (fsconfig(fs_fd, FSCONFIG_CMD_CREATE, NULL, NULL, 0) < 0) + if (fsconfig(fs_fd, FSCONFIG_CMD_CREATE, /* key= */ NULL, /* value= */ NULL, /* aux= */ 0) < 0) return log_full_errno(error_log_level, errno, "Failed to realize fs fd for \"%s\" (\"%s\"): %m", what, type); - mnt_fd = fsmount(fs_fd, FSMOUNT_CLOEXEC, 0); + mnt_fd = fsmount(fs_fd, FSMOUNT_CLOEXEC, /* attr_flags= */ 0); if (mnt_fd < 0) return log_full_errno(error_log_level, errno, "Failed to create mount fd for \"%s\" (\"%s\"): %m", what, type); diff --git a/src/shared/mstack.c b/src/shared/mstack.c index 179c158870a..0cb0462a943 100644 --- a/src/shared/mstack.c +++ b/src/shared/mstack.c @@ -904,7 +904,7 @@ static int mstack_make_overlayfs( report_errno_and_exit(errno_pipe_fds[1], /* error= */ 0); } - _cleanup_close_ int overlayfs_mnt_fd = fsmount(sb_fd, FSMOUNT_CLOEXEC, 0); + _cleanup_close_ int overlayfs_mnt_fd = fsmount(sb_fd, FSMOUNT_CLOEXEC, /* attr_flags= */ 0); if (overlayfs_mnt_fd < 0) return log_debug_errno(errno, "Failed to create mount fd: %m"); @@ -951,16 +951,16 @@ int mstack_make_mounts( if (sb_fd < 0) return log_debug_errno(errno, "Failed to create tmpfs: %m"); - if (fsconfig(sb_fd, FSCONFIG_SET_STRING, "source", mstack->path, 0) < 0) + if (fsconfig(sb_fd, FSCONFIG_SET_STRING, "source", mstack->path, /* aux= */ 0) < 0) return log_debug_errno(errno, "Failed to set mount source: %m"); - if (fsconfig(sb_fd, FSCONFIG_SET_STRING, "mode", "0755", 0) < 0) + if (fsconfig(sb_fd, FSCONFIG_SET_STRING, "mode", "0755", /* aux= */ 0) < 0) return log_debug_errno(errno, "Failed to set mount source: %m"); - if (fsconfig(sb_fd, FSCONFIG_CMD_CREATE, NULL, NULL, 0) < 0) + if (fsconfig(sb_fd, FSCONFIG_CMD_CREATE, /* key= */ NULL, /* value= */ NULL, /* aux= */ 0) < 0) return log_debug_errno(errno, "Failed to realize tmpfs: %m"); - mstack->root_mount_fd = fsmount(sb_fd, FSMOUNT_CLOEXEC, 0); + mstack->root_mount_fd = fsmount(sb_fd, FSMOUNT_CLOEXEC, /* attr_flags= */ 0); if (mstack->root_mount_fd < 0) return log_debug_errno(errno, "Failed to create mount fd: %m"); diff --git a/src/shared/numa-util.c b/src/shared/numa-util.c index af8770106e3..53e026d362e 100644 --- a/src/shared/numa-util.c +++ b/src/shared/numa-util.c @@ -72,7 +72,8 @@ int apply_numa_policy(const NUMAPolicy *policy) { assert(policy); - if (get_mempolicy(NULL, NULL, 0, NULL, 0) < 0 && errno == ENOSYS) + if (get_mempolicy(/* mode= */ NULL, /* nodemask= */ NULL, /* maxnode= */ 0, + /* addr= */ NULL, /* flags= */ 0) < 0 && errno == ENOSYS) /* NUMA syscall interface not available (kernel compiled without NUMA support). * Return -ENOSYS so callers can distinguish this from -EOPNOTSUPP, which we * return below when the syscall interface exists but the requested policy is diff --git a/src/shared/seccomp-util.c b/src/shared/seccomp-util.c index 9bd5d8392fd..45ae95d4188 100644 --- a/src/shared/seccomp-util.c +++ b/src/shared/seccomp-util.c @@ -2228,7 +2228,7 @@ static int add_personality_rules(scmp_filter_ctx seccomp, uint32_t arch, uint32_ assert((value & ~mask) == 0); BIT_FOREACH(bit, value) { - r = add_personality_rule(seccomp, arch, INDEX_TO_MASK(uint32_t, bit), 0); + r = add_personality_rule(seccomp, arch, INDEX_TO_MASK(uint32_t, bit), /* value= */ 0); if (r < 0) return r; } @@ -2281,7 +2281,7 @@ int seccomp_restrict_read_implies_exec(void) { if (r < 0) return r; - r = add_personality_rules(seccomp, arch, READ_IMPLIES_EXEC, 0); + r = add_personality_rules(seccomp, arch, READ_IMPLIES_EXEC, /* value= */ 0); if (r < 0) return r; diff --git a/src/shared/uapi16-manifest.c b/src/shared/uapi16-manifest.c index b1bdfe073e2..bff48145756 100644 --- a/src/shared/uapi16-manifest.c +++ b/src/shared/uapi16-manifest.c @@ -115,7 +115,7 @@ bool uapi16_name_is_valid(const char *s) { if (!path_is_normalized(s)) return false; - if (string_has_cc(s, NULL)) + if (string_has_cc(s, /* ok= */ NULL)) return false; if (uapi16_manifest_filename_is_reserved(s)) @@ -130,7 +130,7 @@ bool uapi16_xattr_name_is_valid(const char *s) { return !isempty(s) && utf8_is_valid(s) && - !string_has_cc(s, NULL); + !string_has_cc(s, /* ok= */ NULL); } static int json_dispatch_uapi16_name(const char *name, sd_json_variant *variant, sd_json_dispatch_flags_t flags, void *userdata) { @@ -426,7 +426,7 @@ int uapi16_xattrs_build_json(const Uapi16Xattr *x, size_t n, sd_json_variant **r assert(ret); _cleanup_(sd_json_variant_unrefp) sd_json_variant *ns = NULL; - r = sd_json_variant_new_object(&ns, NULL, 0); + r = sd_json_variant_new_object(&ns, /* array= */ NULL, 0); if (r < 0) return r; diff --git a/src/sysext/sysext.c b/src/sysext/sysext.c index eed8ddac335..6ef3a46d037 100644 --- a/src/sysext/sysext.c +++ b/src/sysext/sysext.c @@ -995,7 +995,7 @@ static int resolve_mutable_directory( /* This also creates, e.g., /var/lib/extensions.mutable/usr if needed and all parent * directories plus it also works when the last part is a symlink to the real /usr but we * can't use chase_and_open here because it does not behave the same. */ - r = chase(path, root, CHASE_MKDIR_0755|CHASE_MUST_BE_DIRECTORY|CHASE_PREFIX_ROOT, /* ret_path */ NULL, &path_fd); + r = chase(path, root, CHASE_MKDIR_0755|CHASE_MUST_BE_DIRECTORY|CHASE_PREFIX_ROOT, /* ret_path= */ NULL, &path_fd); if (r < 0) return log_error_errno(r, "Failed to chase/create base directory '%s/%s': %m", strempty(root), skip_leading_slash(path)); @@ -2351,7 +2351,7 @@ static int merge_subprocess( if (old_origin_content) continue; - r = read_full_file(f, &buf, /* ret_size */ NULL); + r = read_full_file(f, &buf, /* ret_size= */ NULL); if (r < 0) { log_debug_errno(r, "Failed to open '%s', continuing search: %m", f); continue; diff --git a/src/test/test-btrfs.c b/src/test/test-btrfs.c index ebf897214df..468374cd186 100644 --- a/src/test/test-btrfs.c +++ b/src/test/test-btrfs.c @@ -102,7 +102,7 @@ TEST(subvol) { ASSERT_OK_POSITIVE(btrfs_subvol_get_read_only_fd(test1_fd)); ASSERT_OK(btrfs_subvol_set_read_only_fd(test1_fd, false)); ASSERT_OK_ZERO(btrfs_subvol_get_read_only_fd(test1_fd)); - ASSERT_OK(btrfs_subvol_snapshot_at(test1_fd, /* from= */ NULL, dir_fd, "test5", 0)); + ASSERT_OK(btrfs_subvol_snapshot_at(test1_fd, /* from= */ NULL, dir_fd, "test5", /* flags= */ 0)); dev_t devnum_fd = 0, devnum_path = 0; int r = btrfs_get_block_device_fd(dir_fd, &devnum_fd); diff --git a/src/test/test-chase.c b/src/test/test-chase.c index b6383cbce3d..dc335400141 100644 --- a/src/test/test-chase.c +++ b/src/test/test-chase.c @@ -1298,7 +1298,7 @@ TEST(chase_and_open_cloexec) { _cleanup_free_ char *p = NULL; int fl; - ASSERT_OK(tfd = mkdtemp_open(NULL, 0, &t)); + ASSERT_OK(tfd = mkdtemp_open(/* template= */ NULL, /* flags= */ 0, &t)); ASSERT_OK_ERRNO(mkdirat(tfd, "dir", 0755)); /* O_CLOEXEC is implied, whether requested or not, on the shortcut and on the full chase */ diff --git a/src/test/test-core-unit.c b/src/test/test-core-unit.c index 7ed2b18253f..b5aefdfe8ee 100644 --- a/src/test/test-core-unit.c +++ b/src/test/test-core-unit.c @@ -122,8 +122,8 @@ TEST(unit_concat_strv) { TEST(service_restart_backoff_enabled) { ASSERT_TRUE(service_restart_backoff_enabled(10 * USEC_PER_SEC, 160 * USEC_PER_SEC, 4)); - ASSERT_FALSE(service_restart_backoff_enabled(10 * USEC_PER_SEC, 160 * USEC_PER_SEC, 0)); - ASSERT_FALSE(service_restart_backoff_enabled(0, 160 * USEC_PER_SEC, 4)); + ASSERT_FALSE(service_restart_backoff_enabled(10 * USEC_PER_SEC, 160 * USEC_PER_SEC, /* restart_steps= */ 0)); + ASSERT_FALSE(service_restart_backoff_enabled(/* restart_usec= */ 0, 160 * USEC_PER_SEC, 4)); ASSERT_FALSE(service_restart_backoff_enabled(10 * USEC_PER_SEC, USEC_INFINITY, 4)); ASSERT_FALSE(service_restart_backoff_enabled(160 * USEC_PER_SEC, 160 * USEC_PER_SEC, 4)); } @@ -138,9 +138,9 @@ TEST(service_restart_usec_nth) { ASSERT_EQ(service_restart_usec_nth(1 * USEC_PER_SEC, 64 * USEC_PER_SEC, 6, 6), 32 * USEC_PER_SEC); ASSERT_EQ(service_restart_usec_nth(5 * USEC_PER_SEC, 60 * USEC_PER_SEC, 3, 2), 11447142u); - ASSERT_EQ(service_restart_usec_nth(10 * USEC_PER_SEC, 160 * USEC_PER_SEC, 0, 3), 10 * USEC_PER_SEC); + ASSERT_EQ(service_restart_usec_nth(10 * USEC_PER_SEC, 160 * USEC_PER_SEC, /* restart_steps= */ 0, 3), 10 * USEC_PER_SEC); ASSERT_EQ(service_restart_usec_nth(10 * USEC_PER_SEC, USEC_INFINITY, 4, 3), 10 * USEC_PER_SEC); - ASSERT_EQ(service_restart_usec_nth(0, 160 * USEC_PER_SEC, 4, 3), 0u); + ASSERT_EQ(service_restart_usec_nth(/* restart_usec= */ 0, 160 * USEC_PER_SEC, 4, 3), 0u); ASSERT_EQ(service_restart_usec_nth(160 * USEC_PER_SEC, 160 * USEC_PER_SEC, 4, 3), 160 * USEC_PER_SEC); } diff --git a/src/test/test-daemon.c b/src/test/test-daemon.c index ab22cf302c6..0164086acf7 100644 --- a/src/test/test-daemon.c +++ b/src/test/test-daemon.c @@ -66,7 +66,7 @@ int main(int argc, char *argv[]) { ASSERT_TRUE(IN_SET(r, -EOPNOTSUPP, -EBADF)); if (r == -EBADF) { safe_close(fd); - ASSERT_OK_ERRNO(fd = pidfd_open(getpid_cached(), 0)); + ASSERT_OK_ERRNO(fd = pidfd_open(getpid_cached(), /* flags= */ 0)); ASSERT_OK(sd_pidfd_get_inode_id(fd, /* ret= */ NULL)); } diff --git a/src/test/test-device-nodes.c b/src/test/test-device-nodes.c index eea866c49b6..f986f6a4b86 100644 --- a/src/test/test-device-nodes.c +++ b/src/test/test-device-nodes.c @@ -60,7 +60,7 @@ TEST(encode_devnode_name_truncated) { /* A zero-sized buffer is rejected without being touched, a one-byte buffer only holds the NUL */ memset(buf, 'X', sizeof(buf)); - ASSERT_ERROR(encode_devnode_name("abc", buf, 0), EINVAL); + ASSERT_ERROR(encode_devnode_name("abc", buf, /* len= */ 0), EINVAL); ASSERT_EQ(buf[0], 'X'); memset(buf, 'X', sizeof(buf)); diff --git a/src/test/test-extension-util.c b/src/test/test-extension-util.c index 3a7b5a81d0b..9f4dcbf21d4 100644 --- a/src/test/test-extension-util.c +++ b/src/test/test-extension-util.c @@ -40,7 +40,7 @@ TEST(extension_match_host) { host_init(&h); - ASSERT_OK(tfd = mkdtemp_open(NULL, O_DIRECTORY, &t)); + ASSERT_OK(tfd = mkdtemp_open(/* template= */ NULL, O_DIRECTORY, &t)); ASSERT_OK(write_string_file_at(tfd, "ext.raw", "x", WRITE_STRING_FILE_CREATE)); _cleanup_free_ char *p = ASSERT_NOT_NULL(path_join(t, "ext.raw")); diff --git a/src/test/test-fd-util.c b/src/test/test-fd-util.c index 13625f6e732..9acbb40504c 100644 --- a/src/test/test-fd-util.c +++ b/src/test/test-fd-util.c @@ -182,7 +182,7 @@ TEST(fd_move_above_stdio) { ASSERT_OK(close_nointr(/* fd= */ 0)); new_fd = ASSERT_OK_ZERO(xopenat(AT_FDCWD, "/dev/null", O_RDONLY)); - ASSERT_OK(fd_cloexec(new_fd, false)); + ASSERT_OK(fd_cloexec(new_fd, /* cloexec= */ false)); new_fd = fd_move_above_stdio(new_fd); ASSERT_GE(new_fd, 3); @@ -257,7 +257,7 @@ TEST(rearrange_stdio) { ASSERT_OK_EQ_ERRNO(read(0, buffer, sizeof(buffer)), 6); ASSERT_EQ(memcmp(buffer, "foobar", 6), 0); - ASSERT_OK(fd_cloexec(1, true)); /* stays in place, but the flag must still be cleared */ + ASSERT_OK(fd_cloexec(1, /* cloexec= */ true)); /* stays in place, but the flag must still be cleared */ ASSERT_OK(rearrange_stdio(-EBADF, 1, 2)); assert_stdio_not_cloexec(); ASSERT_ERROR_ERRNO(write(1, "a", 1), ENOSPC); diff --git a/src/test/test-fdset.c b/src/test/test-fdset.c index 8bdffb485e8..2d8b453a298 100644 --- a/src/test/test-fdset.c +++ b/src/test/test-fdset.c @@ -35,7 +35,7 @@ TEST(fdset_new_fill) { ASSERT_ERROR(fd_validate(fd), EBADF); fd = ASSERT_OK(xopenat(AT_FDCWD, "/dev/null", O_RDONLY)); - ASSERT_OK(fd_cloexec(fd, false)); + ASSERT_OK(fd_cloexec(fd, /* cloexec= */ false)); ASSERT_OK(fdset_new_fill(/* filter_cloexec= */ 1, &fdset)); ASSERT_FALSE(fdset_contains(fdset, fd)); diff --git a/src/test/test-fileio.c b/src/test/test-fileio.c index 337a9be32e2..0efe3b4976b 100644 --- a/src/test/test-fileio.c +++ b/src/test/test-fileio.c @@ -622,25 +622,25 @@ TEST(xfopenat_empty_path) { _cleanup_fclose_ FILE *f = NULL; _cleanup_free_ char *line = NULL; - ASSERT_OK(tfd = mkdtemp_open(NULL, 0, &t)); + ASSERT_OK(tfd = mkdtemp_open(/* template= */ NULL, /* flags= */ 0, &t)); ASSERT_OK(write_string_file_at(tfd, "file", "hello", WRITE_STRING_FILE_CREATE)); ASSERT_OK(fd = xopenat(tfd, "file", O_RDONLY)); /* A NULL or empty path reopens the fd for reading... */ - ASSERT_OK(xfopenat(fd, /* path= */ NULL, "re", 0, &f)); + ASSERT_OK(xfopenat(fd, /* path= */ NULL, "re", /* open_flags= */ 0, &f)); ASSERT_OK(read_line(f, SIZE_MAX, &line)); ASSERT_STREQ(line, "hello"); f = safe_fclose(f); - ASSERT_OK(xfopenat(fd, "", "re", 0, &f)); + ASSERT_OK(xfopenat(fd, "", "re", /* open_flags= */ 0, &f)); f = safe_fclose(f); /* ...but creating something needs a name */ - ASSERT_ERROR(xfopenat(tfd, "", "we", 0, &f), ENOENT); - ASSERT_ERROR(xfopenat(tfd, /* path= */ NULL, "ae", 0, &f), ENOENT); + ASSERT_ERROR(xfopenat(tfd, "", "we", /* open_flags= */ 0, &f), ENOENT); + ASSERT_ERROR(xfopenat(tfd, /* path= */ NULL, "ae", /* open_flags= */ 0, &f), ENOENT); /* AT_FDCWD has no fd to reopen, "" fails like with fopen() */ - ASSERT_ERROR(xfopenat(AT_FDCWD, "", "re", 0, &f), ENOENT); - ASSERT_ERROR(xfopenat(AT_FDCWD, /* path= */ NULL, "re", 0, &f), EBADF); + ASSERT_ERROR(xfopenat(AT_FDCWD, "", "re", /* open_flags= */ 0, &f), ENOENT); + ASSERT_ERROR(xfopenat(AT_FDCWD, /* path= */ NULL, "re", /* open_flags= */ 0, &f), EBADF); ASSERT_ERROR(fopen_unlocked("", "re", &f), ENOENT); ASSERT_ERROR(read_one_line_file("", &line), ENOENT); } @@ -932,7 +932,7 @@ TEST(xfopenat_dangling_symlink) { _cleanup_fclose_ FILE *f = NULL; _cleanup_close_ int tfd = -EBADF; - ASSERT_OK(tfd = mkdtemp_open(NULL, 0, &t)); + ASSERT_OK(tfd = mkdtemp_open(/* template= */ NULL, /* flags= */ 0, &t)); ASSERT_OK_ERRNO(symlinkat("target", tfd, "link")); /* The creating modes go through openat_report_new() now, which refuses to create through a dangling symlink */ diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c index 4ddce39b2ab..84daef7af05 100644 --- a/src/test/test-fs-util.c +++ b/src/test/test-fs-util.c @@ -748,7 +748,7 @@ TEST(xopenat_tmpfile) { _cleanup_close_ int tfd = -EBADF, fd = -EBADF; struct stat st; - ASSERT_OK(tfd = mkdtemp_open(NULL, 0, &t)); + ASSERT_OK(tfd = mkdtemp_open(/* template= */ NULL, /* flags= */ 0, &t)); BLOCK_WITH_UMASK(0022); @@ -803,7 +803,7 @@ TEST(xopenat_empty_path) { _cleanup_(rm_rf_physical_and_freep) char *t = NULL; _cleanup_close_ int tfd = -EBADF, fd = -EBADF; - ASSERT_OK(tfd = mkdtemp_open(NULL, 0, &t)); + ASSERT_OK(tfd = mkdtemp_open(/* template= */ NULL, /* flags= */ 0, &t)); /* Without XO_EMPTY_PATH an empty path is a missing name, as for open("") */ ASSERT_ERROR(xopenat(tfd, "", O_RDONLY|O_DIRECTORY), ENOENT); @@ -818,20 +818,20 @@ TEST(xopenat_empty_path) { /* With the flag it reopens the directory itself, like AT_EMPTY_PATH... */ ASSERT_OK(fd = xopenat_full(tfd, "", O_RDONLY|O_DIRECTORY, XO_EMPTY_PATH, MODE_INVALID)); - ASSERT_OK_POSITIVE(inode_same_at(tfd, NULL, fd, NULL, AT_EMPTY_PATH)); + ASSERT_OK_POSITIVE(inode_same_at(tfd, /* filea= */ NULL, fd, /* fileb= */ NULL, AT_EMPTY_PATH)); fd = safe_close(fd); ASSERT_OK(fd = xopenat_full(tfd, /* path= */ NULL, O_RDONLY|O_DIRECTORY, XO_EMPTY_PATH, MODE_INVALID)); - ASSERT_OK_POSITIVE(inode_same_at(tfd, NULL, fd, NULL, AT_EMPTY_PATH)); + ASSERT_OK_POSITIVE(inode_same_at(tfd, /* filea= */ NULL, fd, /* fileb= */ NULL, AT_EMPTY_PATH)); fd = safe_close(fd); ASSERT_OK(fd = xopenat_full(XAT_FDROOT, "", O_RDONLY|O_DIRECTORY, XO_EMPTY_PATH, MODE_INVALID)); - ASSERT_OK_POSITIVE(inode_same_at(AT_FDCWD, "/", fd, NULL, AT_EMPTY_PATH)); + ASSERT_OK_POSITIVE(inode_same_at(AT_FDCWD, "/", fd, /* fileb= */ NULL, AT_EMPTY_PATH)); fd = safe_close(fd); /* ...and a non-empty path is opened as usual */ ASSERT_OK(fd = xopenat_full(tfd, "file", O_RDWR|O_CREAT|O_EXCL, XO_EMPTY_PATH, 0644)); - ASSERT_OK_POSITIVE(inode_same_at(tfd, "file", fd, NULL, AT_EMPTY_PATH)); + ASSERT_OK_POSITIVE(inode_same_at(tfd, "file", fd, /* fileb= */ NULL, AT_EMPTY_PATH)); } TEST(xopenat_nocow) { @@ -1157,7 +1157,7 @@ TEST(xopenat_cloexec) { _cleanup_close_ int tfd = -EBADF, fd = -EBADF; int fl; - ASSERT_OK(tfd = mkdtemp_open(NULL, 0, &t)); + ASSERT_OK(tfd = mkdtemp_open(/* template= */ NULL, /* flags= */ 0, &t)); /* O_CLOEXEC is implied, whether requested or not */ ASSERT_OK(fd = xopenat(tfd, "regular", O_RDWR|O_CREAT|O_EXCL)); @@ -1170,7 +1170,7 @@ TEST(xopenat_cloexec) { ASSERT_TRUE(FLAGS_SET(fl, FD_CLOEXEC)); fd = safe_close(fd); - ASSERT_OK(fd = openat_report_new(tfd, "regular", O_RDWR|O_CREAT, 0644, NULL)); + ASSERT_OK(fd = openat_report_new(tfd, "regular", O_RDWR|O_CREAT, 0644, /* ret_newly_created= */ NULL)); ASSERT_OK_ERRNO(fl = fcntl(fd, F_GETFD)); ASSERT_TRUE(FLAGS_SET(fl, FD_CLOEXEC)); fd = safe_close(fd); diff --git a/src/test/test-install-root.c b/src/test/test-install-root.c index b182f49f299..935f1485f4f 100644 --- a/src/test/test-install-root.c +++ b/src/test/test-install-root.c @@ -1624,7 +1624,7 @@ TEST(dependency_mask) { write_vendor_file("system/masked-dep.service", WANTED_BY_MULTI_USER); - do_enable(0, STRV_MAKE("masked-dep.service")); + do_enable(/* flags= */ 0, STRV_MAKE("masked-dep.service")); assert_state("masked-dep.service", UNIT_FILE_ENABLED); entry = strjoina(vendor_root, ETC_WANTS"masked-dep.service"); @@ -1634,7 +1634,7 @@ TEST(dependency_mask) { ASSERT_OK_ERRNO(symlink("/dev/null", entry)); assert_state("masked-dep.service", UNIT_FILE_DISABLED); - do_disable(0, STRV_MAKE("masked-dep.service")); + do_disable(/* flags= */ 0, STRV_MAKE("masked-dep.service")); ASSERT_TRUE(is_dependency_mask(entry)); assert_state("masked-dep.service", UNIT_FILE_DISABLED); } @@ -1650,14 +1650,14 @@ TEST(dependency_mask_empty_file) { empty = strjoina(vendor_root, "/etc/empty-mask"); ASSERT_OK(touch(empty)); - do_enable(0, STRV_MAKE("empty-mask-dep.service")); + do_enable(/* flags= */ 0, STRV_MAKE("empty-mask-dep.service")); entry = strjoina(vendor_root, ETC_WANTS"empty-mask-dep.service"); ASSERT_OK_ERRNO(unlink(entry)); ASSERT_OK_ERRNO(symlink("/etc/empty-mask", entry)); assert_state("empty-mask-dep.service", UNIT_FILE_DISABLED); - do_disable(0, STRV_MAKE("empty-mask-dep.service")); + do_disable(/* flags= */ 0, STRV_MAKE("empty-mask-dep.service")); ASSERT_OK(readlink_malloc(entry, &dest)); ASSERT_STREQ(dest, "/etc/empty-mask"); assert_state("empty-mask-dep.service", UNIT_FILE_DISABLED); @@ -1675,7 +1675,7 @@ TEST(disable_template_keeps_instance_mask) { ASSERT_OK_ERRNO(symlink("/dev/null", mask)); assert_state("masked-tmpl@inst.service", UNIT_FILE_MASKED); - do_disable(0, STRV_MAKE("masked-tmpl@.service")); + do_disable(/* flags= */ 0, STRV_MAKE("masked-tmpl@.service")); ASSERT_TRUE(symlink_exists(mask)); assert_state("masked-tmpl@inst.service", UNIT_FILE_MASKED); } @@ -1689,7 +1689,7 @@ TEST(dependency_shadow) { write_vendor_file("system/shadowed-dep.service", WANTED_BY_MULTI_USER); - do_enable(0, STRV_MAKE("shadowed-dep.service")); + do_enable(/* flags= */ 0, STRV_MAKE("shadowed-dep.service")); assert_state("shadowed-dep.service", UNIT_FILE_ENABLED); shadow = strjoina(vendor_root, SYSTEM_CONFIG_UNIT_DIR".control/multi-user.target.wants/", @@ -1707,7 +1707,7 @@ TEST(dependency_shadow_symlink) { write_vendor_file("system/shadow-linked-dep.service", WANTED_BY_MULTI_USER); - do_enable(0, STRV_MAKE("shadow-linked-dep.service")); + do_enable(/* flags= */ 0, STRV_MAKE("shadow-linked-dep.service")); link = strjoina(vendor_root, "/run/systemd/system.control/multi-user.target.wants/", "shadow-linked-dep.service"); @@ -1726,7 +1726,7 @@ TEST(dependency_dir_not_directory) { "[Install]\n" "WantedBy=file-wants.target\n"); - do_enable(0, STRV_MAKE("file-wants-dep.service")); + do_enable(/* flags= */ 0, STRV_MAKE("file-wants-dep.service")); file = strjoina(vendor_root, "/run/systemd/system.control/file-wants.target.wants"); ASSERT_OK(mkdir_parents(file, 0755)); @@ -1756,7 +1756,7 @@ TEST(dependency_unit_dir_symlink) { unit = strjoina(linked_root, "/usr/lib/systemd/system/linked-dir-dep.service"); ASSERT_OK(write_string_file(unit, WANTED_BY_MULTI_USER, WRITE_STRING_FILE_CREATE)); - ASSERT_OK(unit_file_enable(RUNTIME_SCOPE_SYSTEM, 0, linked_root, STRV_MAKE("linked-dir-dep.service"), + ASSERT_OK(unit_file_enable(RUNTIME_SCOPE_SYSTEM, /* flags= */ 0, linked_root, STRV_MAKE("linked-dir-dep.service"), &changes, &n_changes)); ASSERT_OK(unit_file_get_state(RUNTIME_SCOPE_SYSTEM, linked_root, "linked-dir-dep.service", &state)); ASSERT_EQ(state, UNIT_FILE_ENABLED); @@ -1770,7 +1770,7 @@ TEST(dependency_mask_shadowed_by_symlink) { write_vendor_file("system/mask-shadowed-dep.service", WANTED_BY_MULTI_USER); - do_enable(0, STRV_MAKE("mask-shadowed-dep.service")); + do_enable(/* flags= */ 0, STRV_MAKE("mask-shadowed-dep.service")); entry = strjoina(vendor_root, ETC_WANTS"mask-shadowed-dep.service"); ASSERT_OK_ERRNO(unlink(entry)); @@ -1788,7 +1788,7 @@ TEST(dependency_shadow_mask) { write_vendor_file("system/shadow-masked-dep.service", WANTED_BY_MULTI_USER); - do_enable(0, STRV_MAKE("shadow-masked-dep.service")); + do_enable(/* flags= */ 0, STRV_MAKE("shadow-masked-dep.service")); assert_state("shadow-masked-dep.service", UNIT_FILE_ENABLED); /* If the mask in /run/ counted as an enablement symlink, the unit would be enabled-runtime. */ @@ -1818,7 +1818,7 @@ TEST(vendor_enable) { /* Disabling it cannot remove the vendor symlink, so it has to shadow it with a symlink to /dev/null * in the configuration directory. */ - ASSERT_OK(unit_file_disable(RUNTIME_SCOPE_SYSTEM, 0, vendor_root, + ASSERT_OK(unit_file_disable(RUNTIME_SCOPE_SYSTEM, /* flags= */ 0, vendor_root, STRV_MAKE("vendor-enabled.service"), &changes, &n_changes)); ASSERT_EQ(n_changes, 1u); ASSERT_EQ(changes[0].type, INSTALL_CHANGE_MASK_DEPENDENCY); @@ -1833,12 +1833,12 @@ TEST(vendor_enable) { assert_state("vendor-enabled.service", UNIT_FILE_DISABLED); /* Disabling twice must not undo the mask. */ - do_disable(0, STRV_MAKE("vendor-enabled.service")); + do_disable(/* flags= */ 0, STRV_MAKE("vendor-enabled.service")); ASSERT_TRUE(is_dependency_mask(mask)); assert_state("vendor-enabled.service", UNIT_FILE_DISABLED); /* Enabling drops the mask again. */ - do_enable(0, STRV_MAKE("vendor-enabled.service")); + do_enable(/* flags= */ 0, STRV_MAKE("vendor-enabled.service")); ASSERT_TRUE(symlink_exists(mask)); ASSERT_FALSE(is_dependency_mask(mask)); assert_state("vendor-enabled.service", UNIT_FILE_ENABLED); @@ -1861,10 +1861,10 @@ TEST(vendor_mask_in_unrelated_target) { assert_state("vendor-two-targets.service", UNIT_FILE_ENABLED); - do_disable(0, STRV_MAKE("vendor-two-targets.service")); + do_disable(/* flags= */ 0, STRV_MAKE("vendor-two-targets.service")); assert_state("vendor-two-targets.service", UNIT_FILE_DISABLED); - do_enable(0, STRV_MAKE("vendor-two-targets.service")); + do_enable(/* flags= */ 0, STRV_MAKE("vendor-two-targets.service")); ASSERT_TRUE(is_dependency_mask(mask)); assert_state("vendor-two-targets.service", UNIT_FILE_ENABLED); } @@ -1880,7 +1880,7 @@ TEST(vendor_preset_leaves_vendor_enablement_alone) { write_vendor_symlink("multi-user.target.wants/vendor-preset-off.service", "../vendor-preset-off.service"); write_vendor_file("system-preset/13-vendor-off.preset", "disable vendor-preset-off.service\n"); - do_preset(0, STRV_MAKE("vendor-preset-off.service")); + do_preset(/* flags= */ 0, STRV_MAKE("vendor-preset-off.service")); vendor_link = strjoina(vendor_root, VENDOR_WANTS"vendor-preset-off.service"); mask = strjoina(vendor_root, ETC_WANTS"vendor-preset-off.service"); @@ -1889,13 +1889,13 @@ TEST(vendor_preset_leaves_vendor_enablement_alone) { assert_state("vendor-preset-off.service", UNIT_FILE_ENABLED); /* An explicit disable is a different matter. */ - do_disable(0, STRV_MAKE("vendor-preset-off.service")); + do_disable(/* flags= */ 0, STRV_MAKE("vendor-preset-off.service")); ASSERT_TRUE(is_dependency_mask(mask)); assert_state("vendor-preset-off.service", UNIT_FILE_DISABLED); /* And presetting it back to "enable" clears that mask again. */ write_vendor_file("system-preset/13-vendor-off.preset", "enable vendor-preset-off.service\n"); - do_preset(0, STRV_MAKE("vendor-preset-off.service")); + do_preset(/* flags= */ 0, STRV_MAKE("vendor-preset-off.service")); ASSERT_FALSE(is_dependency_mask(mask)); assert_state("vendor-preset-off.service", UNIT_FILE_ENABLED); @@ -1915,7 +1915,7 @@ TEST(vendor_symlinked_dependency_dir_is_not_followed) { assert_state("vendor-escape.service", UNIT_FILE_DISABLED); - do_disable(0, STRV_MAKE("vendor-escape.service")); + do_disable(/* flags= */ 0, STRV_MAKE("vendor-escape.service")); ASSERT_FALSE(symlink_exists(strjoina(vendor_root, SYSTEM_CONFIG_UNIT_DIR"/escape.target.wants/vendor-escape.service"))); } @@ -1937,11 +1937,11 @@ TEST(vendor_requires_and_upholds) { write_vendor_symlink(rel, strjoina("../", name)); assert_state(name, UNIT_FILE_ENABLED); - do_disable(0, STRV_MAKE(name)); + do_disable(/* flags= */ 0, STRV_MAKE(name)); ASSERT_TRUE(is_dependency_mask(mask)); assert_state(name, UNIT_FILE_DISABLED); - do_enable(0, STRV_MAKE(name)); + do_enable(/* flags= */ 0, STRV_MAKE(name)); ASSERT_FALSE(is_dependency_mask(mask)); assert_state(name, UNIT_FILE_ENABLED); } @@ -1963,14 +1963,14 @@ TEST(vendor_template_instance) { assert_state("vendor-tmpl@inst.service", UNIT_FILE_ENABLED); /* Disabling the template must reach the instance the vendor wired up. */ - do_disable(0, STRV_MAKE("vendor-tmpl@.service")); + do_disable(/* flags= */ 0, STRV_MAKE("vendor-tmpl@.service")); mask = strjoina(vendor_root, ETC_WANTS"vendor-tmpl@inst.service"); ASSERT_TRUE(is_dependency_mask(mask)); ASSERT_TRUE(symlink_exists(strjoina(vendor_root, VENDOR_WANTS"vendor-tmpl@inst.service"))); assert_state("vendor-tmpl@inst.service", UNIT_FILE_DISABLED); - do_enable(0, STRV_MAKE("vendor-tmpl@.service")); + do_enable(/* flags= */ 0, STRV_MAKE("vendor-tmpl@.service")); ASSERT_TRUE(symlink_exists(strjoina(vendor_root, ETC_WANTS"vendor-tmpl@dflt.service"))); ASSERT_FALSE(symlink_exists(mask)); assert_state("vendor-tmpl@inst.service", UNIT_FILE_ENABLED); @@ -1991,13 +1991,13 @@ TEST(vendor_also) { "WantedBy=sockets.target\n"); write_vendor_symlink("sockets.target.wants/vendor-also.socket", "../vendor-also.socket"); - do_disable(0, STRV_MAKE("vendor-also.service")); + do_disable(/* flags= */ 0, STRV_MAKE("vendor-also.service")); mask = strjoina(vendor_root, SYSTEM_CONFIG_UNIT_DIR"/sockets.target.wants/vendor-also.socket"); ASSERT_TRUE(is_dependency_mask(mask)); assert_state("vendor-also.socket", UNIT_FILE_DISABLED); - do_enable(0, STRV_MAKE("vendor-also.service")); + do_enable(/* flags= */ 0, STRV_MAKE("vendor-also.service")); ASSERT_FALSE(is_dependency_mask(mask)); assert_state("vendor-also.socket", UNIT_FILE_ENABLED); } @@ -2017,7 +2017,7 @@ TEST(vendor_mask_shadows_lower_priority_dir) { assert_state("vendor-shadow.service", UNIT_FILE_DISABLED); - do_disable(0, STRV_MAKE("vendor-shadow.service")); + do_disable(/* flags= */ 0, STRV_MAKE("vendor-shadow.service")); mask = strjoina(vendor_root, ETC_WANTS"vendor-shadow.service"); ASSERT_FALSE(symlink_exists(mask)); @@ -2036,14 +2036,14 @@ TEST(vendor_multiple_units) { write_vendor_symlink(strjoina(target, ".wants/", name), strjoina("../", name)); } - do_disable(0, STRV_MAKE("vendor-multi-a.service", "vendor-multi-b.service", "vendor-multi-c.service")); + do_disable(/* flags= */ 0, STRV_MAKE("vendor-multi-a.service", "vendor-multi-b.service", "vendor-multi-c.service")); FOREACH_STRING(name, "vendor-multi-a.service", "vendor-multi-b.service", "vendor-multi-c.service") FOREACH_STRING(target, "multi-user.target", "graphical.target") ASSERT_TRUE(is_dependency_mask(strjoina(vendor_root, SYSTEM_CONFIG_UNIT_DIR"/", target, ".wants/", name))); - do_enable(0, STRV_MAKE("vendor-multi-a.service", "vendor-multi-b.service", "vendor-multi-c.service")); + do_enable(/* flags= */ 0, STRV_MAKE("vendor-multi-a.service", "vendor-multi-b.service", "vendor-multi-c.service")); FOREACH_STRING(name, "vendor-multi-a.service", "vendor-multi-b.service", "vendor-multi-c.service") FOREACH_STRING(target, "multi-user.target", "graphical.target") @@ -2086,7 +2086,7 @@ TEST(vendor_entry_kinds) { /* Whatever is sitting there already shadows the vendor symlink, so disabling has nothing * left to do and must not replace it: that would destroy it. */ - do_disable(0, STRV_MAKE("vendor-kinds.service")); + do_disable(/* flags= */ 0, STRV_MAKE("vendor-kinds.service")); ASSERT_OK(read_full_file(entry, &back, /* ret_size= */ NULL)); ASSERT_STREQ(back, contents); @@ -2097,7 +2097,7 @@ TEST(vendor_entry_kinds) { assert_state("vendor-kinds.service", UNIT_FILE_DISABLED); /* Replacing this one would fail outright rather than quietly. */ - do_disable(0, STRV_MAKE("vendor-kinds.service")); + do_disable(/* flags= */ 0, STRV_MAKE("vendor-kinds.service")); ASSERT_OK_ERRNO(rmdir(entry)); } @@ -2116,7 +2116,7 @@ TEST(vendor_static_dependency) { assert_state("vendor-static-dep.service", UNIT_FILE_ENABLED); /* Disabling masks the symlink that enables the unit and leaves the static dependency symlinks alone. */ - do_disable(0, STRV_MAKE("vendor-static-dep.service")); + do_disable(/* flags= */ 0, STRV_MAKE("vendor-static-dep.service")); ASSERT_TRUE(is_dependency_mask(strjoina(vendor_root, ETC_WANTS"vendor-static-dep.service"))); ASSERT_FALSE(symlink_exists(strjoina(vendor_root, SYSTEM_CONFIG_UNIT_DIR"/initrd-root-device.target.wants/vendor-static-dep.service"))); ASSERT_FALSE(symlink_exists(strjoina(vendor_root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.requires/vendor-static-dep.service"))); @@ -2131,7 +2131,7 @@ TEST(vendor_static_dependency) { write_vendor_symlink("multi-user.target.wants/vendor-static-dep@b.service", "../vendor-static-dep@.service"); assert_state("vendor-static-dep@b.service", UNIT_FILE_ENABLED); - do_disable(0, STRV_MAKE("vendor-static-dep@.service")); + do_disable(/* flags= */ 0, STRV_MAKE("vendor-static-dep@.service")); ASSERT_TRUE(is_dependency_mask(strjoina(vendor_root, ETC_WANTS"vendor-static-dep@b.service"))); ASSERT_FALSE(symlink_exists(strjoina(vendor_root, SYSTEM_CONFIG_UNIT_DIR"/initrd-root-device.target.wants/vendor-static-dep@a.service"))); assert_state("vendor-static-dep@a.service", UNIT_FILE_STATIC); @@ -2175,14 +2175,14 @@ TEST(vendor_also_keeps_sibling_preset_policy) { "disable vendor-main.service\n" "enable vendor-aux.socket\n"); - do_enable(0, STRV_MAKE("vendor-aux.socket")); + do_enable(/* flags= */ 0, STRV_MAKE("vendor-aux.socket")); assert_state("vendor-aux.socket", UNIT_FILE_ENABLED); - do_preset(0, STRV_MAKE("vendor-main.service")); + do_preset(/* flags= */ 0, STRV_MAKE("vendor-main.service")); assert_state("vendor-main.service", UNIT_FILE_DISABLED); assert_state("vendor-aux.socket", UNIT_FILE_ENABLED); - do_preset_all(0); + do_preset_all(/* flags= */ 0); assert_state("vendor-main.service", UNIT_FILE_DISABLED); assert_state("vendor-aux.socket", UNIT_FILE_ENABLED); } diff --git a/src/test/test-io-uring-util.c b/src/test/test-io-uring-util.c index 24019d61053..e10e80df36b 100644 --- a/src/test/test-io-uring-util.c +++ b/src/test/test-io-uring-util.c @@ -48,7 +48,7 @@ TEST(sq_full) { if (!io_uring_init_or_skip(&ring, 4)) return; - queue_nops(&ring, 4, 0); + queue_nops(&ring, 4, /* first_id= */ 0); ASSERT_NULL(io_uring_get_sqe(&ring)); ASSERT_OK_EQ(io_uring_submit(&ring), 4); @@ -110,7 +110,7 @@ TEST(submit_and_wait) { ASSERT_ERROR(io_uring_submit_and_wait(&ring, 10 * USEC_PER_MSEC), ETIME); - queue_nops(&ring, 1, 0); + queue_nops(&ring, 1, /* first_id= */ 0); ASSERT_OK_EQ(io_uring_submit_and_wait(&ring, USEC_INFINITY), 1); ASSERT_EQ(reap(&ring, seen, ELEMENTSOF(seen)), 1u); ASSERT_TRUE(seen[0]); diff --git a/src/test/test-keyring-util.c b/src/test/test-keyring-util.c index 95ed3e259dc..3851ae01a22 100644 --- a/src/test/test-keyring-util.c +++ b/src/test/test-keyring-util.c @@ -130,7 +130,7 @@ TEST(keyring_find_by_name) { /* Keyrings the process possesses show up in /proc/keys. A leftover of a crashed run may linger. */ xsprintf(name, "test-keyring-util-find-" PID_FMT, getpid()); - ring = add_key("keyring", name, NULL, 0, KEY_SPEC_THREAD_KEYRING); + ring = add_key("keyring", name, /* payload= */ NULL, /* plen= */ 0, KEY_SPEC_THREAD_KEYRING); if (ring < 0) { log_tests_skipped_errno(errno, "Cannot create keyring"); return; @@ -159,7 +159,7 @@ TEST(keyring_find_by_name) { ASSERT_EQ(serial, ring); /* Two keyrings of the same name are ambiguous */ - other = add_key("keyring", name, NULL, 0, KEY_SPEC_PROCESS_KEYRING); + other = add_key("keyring", name, /* payload= */ NULL, /* plen= */ 0, KEY_SPEC_PROCESS_KEYRING); ASSERT_OK_ERRNO(other); ASSERT_ERROR(keyring_find_by_name(name, getuid(), &serial), ENOTUNIQ); @@ -192,7 +192,7 @@ TEST(keyring_find_by_name_at) { ASSERT_ERROR(r, ENOKEY); xsprintf(name, "test-keyring-util-find-at-" PID_FMT, getpid()); - ring = add_key("keyring", name, NULL, 0, KEY_SPEC_THREAD_KEYRING); + ring = add_key("keyring", name, /* payload= */ NULL, /* plen= */ 0, KEY_SPEC_THREAD_KEYRING); if (ring < 0) { log_tests_skipped_errno(errno, "Cannot create keyring"); return; @@ -242,7 +242,7 @@ TEST(keyring_ops) { int r; /* A private keyring in the thread keyring suffices, no privileges needed */ - ring = add_key("keyring", "test-keyring-util", NULL, 0, KEY_SPEC_THREAD_KEYRING); + ring = add_key("keyring", "test-keyring-util", /* payload= */ NULL, /* plen= */ 0, KEY_SPEC_THREAD_KEYRING); if (ring < 0) { log_tests_skipped_errno(errno, "Cannot create keyring"); return; @@ -407,7 +407,7 @@ TEST(keyring_chain) { key_serial_t ring, serial, serial2; int r; - ring = add_key("keyring", "test-keyring-util-chain", NULL, 0, KEY_SPEC_THREAD_KEYRING); + ring = add_key("keyring", "test-keyring-util-chain", /* payload= */ NULL, /* plen= */ 0, KEY_SPEC_THREAD_KEYRING); if (ring < 0) return (void) log_tests_skipped_errno(errno, "Cannot create keyring"); diff --git a/src/test/test-machine-tags.c b/src/test/test-machine-tags.c index 833d2595676..4760fe17740 100644 --- a/src/test/test-machine-tags.c +++ b/src/test/test-machine-tags.c @@ -59,40 +59,40 @@ TEST(machine_tag_is_valid) { TEST(machine_tag_list_is_valid) { ASSERT_OK_POSITIVE(machine_tag_list_is_valid(NULL, /* to_add= */ true)); /* empty list is valid */ - ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("a"), true)); - ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo", "bar", "c-d.e"), true)); - ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo=uuu", "bar=qqqq", "c-d.e"), true)); - ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo", "foo=aa"), true)); /* bare key and assignment coexist */ - ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo=1", "foobar=2"), true)); /* one key is a prefix of the other */ - ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("ab=1", "a=2"), true)); /* ... and the other way around */ + ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("a"), /* to_add= */ true)); + ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo", "bar", "c-d.e"), /* to_add= */ true)); + ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo=uuu", "bar=qqqq", "c-d.e"), /* to_add= */ true)); + ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo", "foo=aa"), /* to_add= */ true)); /* bare key and assignment coexist */ + ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo=1", "foobar=2"), /* to_add= */ true)); /* one key is a prefix of the other */ + ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("ab=1", "a=2"), /* to_add= */ true)); /* ... and the other way around */ - ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo=aa", "foo=aa"), true)); /* same key + same value is OK */ - ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("foo", "b:c"), true)); - ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("foo", ""), true)); - ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("foo=aa", "foo=b"), true)); /* same key, different value */ - ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("a=1", "b=2", "a=3"), true)); /* ... also when not adjacent */ - ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("foo=aa", "bar", "foo=aa", "foo=b"), true)); - ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("=aa"), true)); - ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("ab=1*"), true)); /* globs */ - ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("ab=2[345]"), true)); + ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo=aa", "foo=aa"), /* to_add= */ true)); /* same key + same value is OK */ + ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("foo", "b:c"), /* to_add= */ true)); + ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("foo", ""), /* to_add= */ true)); + ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("foo=aa", "foo=b"), /* to_add= */ true)); /* same key, different value */ + ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("a=1", "b=2", "a=3"), /* to_add= */ true)); /* ... also when not adjacent */ + ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("foo=aa", "bar", "foo=aa", "foo=b"), /* to_add= */ true)); + ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("=aa"), /* to_add= */ true)); + ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("ab=1*"), /* to_add= */ true)); /* globs */ + ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("ab=2[345]"), /* to_add= */ true)); ASSERT_OK_POSITIVE(machine_tag_list_is_valid(NULL, /* to_add= */ false)); /* empty list is valid */ - ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("a"), false)); - ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo", "bar", "c-d.e"), false)); - ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo=uuu", "bar=qqqq", "c-d.e"), false)); - ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo", "foo=aa"), false)); /* bare key and assignment coexist */ - ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo=1", "foobar=2"), false)); /* one key is a prefix of the other */ - ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("ab=1", "a=2"), false)); /* ... and the other way around */ + ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("a"), /* to_add= */ false)); + ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo", "bar", "c-d.e"), /* to_add= */ false)); + ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo=uuu", "bar=qqqq", "c-d.e"), /* to_add= */ false)); + ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo", "foo=aa"), /* to_add= */ false)); /* bare key and assignment coexist */ + ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo=1", "foobar=2"), /* to_add= */ false)); /* one key is a prefix of the other */ + ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("ab=1", "a=2"), /* to_add= */ false)); /* ... and the other way around */ - ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo=aa", "foo=aa"), false)); /* same key + same value is OK */ - ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("foo", "b:c"), false)); - ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("foo", ""), false)); - ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo=aa", "foo=b"), false)); /* same key, different value */ - ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("a=1", "b=2", "a=3"), false)); /* ... also when not adjacent */ - ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo=aa", "bar", "foo=aa", "foo=b"), false)); - ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("=aa"), false)); /* this is not a valid pattern, but we don't check */ - ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("ab=1*"), false)); /* globs */ - ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("ab=2[345]"), false)); + ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo=aa", "foo=aa"), /* to_add= */ false)); /* same key + same value is OK */ + ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("foo", "b:c"), /* to_add= */ false)); + ASSERT_OK_ZERO(machine_tag_list_is_valid(STRV_MAKE("foo", ""), /* to_add= */ false)); + ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo=aa", "foo=b"), /* to_add= */ false)); /* same key, different value */ + ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("a=1", "b=2", "a=3"), /* to_add= */ false)); /* ... also when not adjacent */ + ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("foo=aa", "bar", "foo=aa", "foo=b"), /* to_add= */ false)); + ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("=aa"), /* to_add= */ false)); /* this is not a valid pattern, but we don't check */ + ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("ab=1*"), /* to_add= */ false)); /* globs */ + ASSERT_OK_POSITIVE(machine_tag_list_is_valid(STRV_MAKE("ab=2[345]"), /* to_add= */ false)); } TEST(machine_tags_from_string) { @@ -217,10 +217,10 @@ TEST(machine_tags_apply_config) { ASSERT_OK(mkdtemp_malloc("/tmp/test-machine-tags-XXXXXX", &root)); /* No configuration at all: the base list is returned, sorted. An empty result is NULL. */ - ASSERT_OK_ZERO(machine_tags_config_files(root, NULL, NULL)); + ASSERT_OK_ZERO(machine_tags_config_files(root, /* ret_files= */ NULL, /* ret_n_files= */ NULL)); check(root, STRV_MAKE("foo", "bar"), STRV_MAKE("bar", "foo")); - check(root, NULL, NULL); + check(root, /* base= */ NULL, /* expected= */ NULL); /* Comments, empty lines, several entries per line, CRLF line endings, an explicit '+' prefix, and * files from all directories applied in order of their names. The base tag 'vendor.env=old' is @@ -238,7 +238,7 @@ TEST(machine_tags_apply_config) { "-vendor.env=* -vendor.region=*\n" "local.rack=7\n"); - ASSERT_OK_POSITIVE(machine_tags_config_files(root, NULL, NULL)); + ASSERT_OK_POSITIVE(machine_tags_config_files(root, /* ret_files= */ NULL, /* ret_n_files= */ NULL)); check(root, STRV_MAKE("manual", "vendor.env=old"), STRV_MAKE("local.rack=7", "manual", "plus", "runtime", "vendor.after-comment", "vendor.extra", @@ -283,14 +283,14 @@ TEST(machine_tags_apply_config) { /* "-*" resets the list */ write_tags_file(root, "etc/machine-tags.d/90-reset.tags", "-* fresh\n"); check(root, STRV_MAKE("manual", "vendor.env=old"), STRV_MAKE("fresh")); - check(root, NULL, STRV_MAKE("fresh")); + check(root, /* base= */ NULL, STRV_MAKE("fresh")); /* Too many tags */ _cleanup_free_ char *many = NULL; for (unsigned i = 0; i <= MACHINE_TAGS_MAX; i++) ASSERT_OK(strextendf_with_separator(&many, " ", "t%u", i)); write_tags_file(root, "etc/machine-tags.d/95-many.tags", many); - ASSERT_ERROR(machine_tags_apply_config(root, NULL, &l), E2BIG); + ASSERT_ERROR(machine_tags_apply_config(root, /* base= */ NULL, &l), E2BIG); ASSERT_NULL(l); } diff --git a/src/test/test-manifest-util.c b/src/test/test-manifest-util.c index 53ce61788a2..30c7fa2b22c 100644 --- a/src/test/test-manifest-util.c +++ b/src/test/test-manifest-util.c @@ -156,7 +156,7 @@ static int make_tree(char **ret_path) { /* Populates a temporary directory with one inode of each type we can create unprivileged, including * a pair of hardlinks. */ - ASSERT_OK(tfd = mkdtemp_open(NULL, O_DIRECTORY, &t)); + ASSERT_OK(tfd = mkdtemp_open(/* template= */ NULL, O_DIRECTORY, &t)); ASSERT_OK(write_string_file_at(tfd, "regular.txt", "hello world", WRITE_STRING_FILE_CREATE|WRITE_STRING_FILE_AVOID_NEWLINE)); ASSERT_OK(write_string_file_at(tfd, "empty.txt", "", WRITE_STRING_FILE_CREATE|WRITE_STRING_FILE_AVOID_NEWLINE)); @@ -461,8 +461,8 @@ TEST(manifest_mtree_names) { _cleanup_free_ char *user_name = NULL, *group_name = NULL; ASSERT_NOT_NULL(user_name = uid_to_name(getuid())); ASSERT_NOT_NULL(group_name = gid_to_name(getgid())); - const char *expected_user_name = parse_uid(user_name, NULL) < 0 ? user_name : NULL, - *expected_group_name = parse_gid(group_name, NULL) < 0 ? group_name : NULL; + const char *expected_user_name = parse_uid(user_name, /* ret_uid= */ NULL) < 0 ? user_name : NULL, + *expected_group_name = parse_gid(group_name, /* ret_gid= */ NULL) < 0 ? group_name : NULL; /* Names are not included by default */ _cleanup_free_ char *buf = NULL; @@ -470,8 +470,8 @@ TEST(manifest_mtree_names) { _cleanup_(manifest_context_done) ManifestContext c; context_init(&c, MANIFEST_FORMAT_MTREE); ASSERT_OK(generate(&c, tfd, &buf, &size)); - assert_mtree_keyword(buf, "./regular.txt ", "uname", NULL); - assert_mtree_keyword(buf, "./regular.txt ", "gname", NULL); + assert_mtree_keyword(buf, "./regular.txt ", "uname", /* value= */ NULL); + assert_mtree_keyword(buf, "./regular.txt ", "gname", /* value= */ NULL); /* Ask for the names only: they must be resolved even though the numeric IDs are not requested, and * the latter must not be included. */ @@ -489,8 +489,8 @@ TEST(manifest_mtree_names) { FOREACH_STRING(prefix, "./ ", "./regular.txt ", "./subdir/ ", "./subdir/nested.txt ", "./symlink ", "./fifo ", "./hardlink_b ") { assert_mtree_keyword(buf2, prefix, "uname", expected_user_name); assert_mtree_keyword(buf2, prefix, "gname", expected_group_name); - assert_mtree_keyword(buf2, prefix, "uid", NULL); - assert_mtree_keyword(buf2, prefix, "gid", NULL); + assert_mtree_keyword(buf2, prefix, "uid", /* value= */ NULL); + assert_mtree_keyword(buf2, prefix, "gid", /* value= */ NULL); } /* Each name may be requested individually, and is combined with the numeric ID if that is requested @@ -506,7 +506,7 @@ TEST(manifest_mtree_names) { assert_mtree_keyword(buf3, "./regular.txt ", "uid", uid_str); assert_mtree_keyword(buf3, "./regular.txt ", "gid", gid_str); assert_mtree_keyword(buf3, "./regular.txt ", "uname", expected_user_name); - assert_mtree_keyword(buf3, "./regular.txt ", "gname", NULL); + assert_mtree_keyword(buf3, "./regular.txt ", "gname", /* value= */ NULL); _cleanup_free_ char *buf4 = NULL; size_t size4; @@ -514,9 +514,9 @@ TEST(manifest_mtree_names) { context_init(&c, MANIFEST_FORMAT_MTREE); c.fields = MANIFEST_FIELD_GROUP_NAME; ASSERT_OK(generate(&c, tfd, &buf4, &size4)); - assert_mtree_keyword(buf4, "./regular.txt ", "uid", NULL); - assert_mtree_keyword(buf4, "./regular.txt ", "gid", NULL); - assert_mtree_keyword(buf4, "./regular.txt ", "uname", NULL); + assert_mtree_keyword(buf4, "./regular.txt ", "uid", /* value= */ NULL); + assert_mtree_keyword(buf4, "./regular.txt ", "gid", /* value= */ NULL); + assert_mtree_keyword(buf4, "./regular.txt ", "uname", /* value= */ NULL); assert_mtree_keyword(buf4, "./regular.txt ", "gname", expected_group_name); /* Verification of mtree manifests is not supported, regardless of the fields */ @@ -564,7 +564,7 @@ TEST(manifest_uapi16) { /* Each record starts with an RS byte */ ASSERT_EQ(buf[0], 0x1e); - _cleanup_(sd_json_variant_unrefp) sd_json_variant *root = find_object(buf, NULL); + _cleanup_(sd_json_variant_unrefp) sd_json_variant *root = find_object(buf, /* name= */ NULL); ASSERT_NOT_NULL(root); ASSERT_STREQ(sd_json_variant_string(sd_json_variant_by_key(root, "mediaType")), UAPI16_MANIFEST_MEDIA_TYPE); @@ -635,11 +635,11 @@ TEST(manifest_uapi16) { _cleanup_(sd_json_variant_unrefp) sd_json_variant *reg3 = find_object(buf3, "regular.txt"); ASSERT_NOT_NULL(reg3); ASSERT_NULL(sd_json_variant_by_key(reg3, "uid")); - if (parse_uid(user_name, NULL) < 0) /* uid_to_name() falls back to the numeric UID if unresolvable */ + if (parse_uid(user_name, /* ret_uid= */ NULL) < 0) /* uid_to_name() falls back to the numeric UID if unresolvable */ ASSERT_STREQ(sd_json_variant_string(sd_json_variant_by_key(reg3, "userName")), user_name); else ASSERT_NULL(sd_json_variant_by_key(reg3, "userName")); - if (parse_gid(group_name, NULL) < 0) + if (parse_gid(group_name, /* ret_gid= */ NULL) < 0) ASSERT_STREQ(sd_json_variant_string(sd_json_variant_by_key(reg3, "groupName")), group_name); else ASSERT_NULL(sd_json_variant_by_key(reg3, "groupName")); @@ -1147,10 +1147,10 @@ static void load_key_pair( ASSERT_OK(write_string_file(key_path, private_key_pem, WRITE_STRING_FILE_CREATE|WRITE_STRING_FILE_MODE_0600)); ASSERT_OK(write_string_file(cert_path, certificate_pem, WRITE_STRING_FILE_CREATE)); - ASSERT_OK(openssl_load_x509_certificate(OPENSSL_CERTIFICATE_SOURCE_FILE, NULL, cert_path, ret_certificate)); + ASSERT_OK(openssl_load_x509_certificate(OPENSSL_CERTIFICATE_SOURCE_FILE, /* certificate_source= */ NULL, cert_path, ret_certificate)); ASSERT_OK(openssl_load_private_key( OPENSSL_KEY_SOURCE_FILE, - NULL, + /* private_key_source= */ NULL, key_path, &(AskPasswordRequest) { .tty_fd = -EBADF, diff --git a/src/test/test-mount-util.c b/src/test/test-mount-util.c index 67ea614ed80..ad0b56871c4 100644 --- a/src/test/test-mount-util.c +++ b/src/test/test-mount-util.c @@ -388,11 +388,11 @@ TEST(umount_recursive_stale_path) { CHECK_PRIV; - ASSERT_OK(mkdtemp_malloc(NULL, &t)); + ASSERT_OK(mkdtemp_malloc(/* template= */ NULL, &t)); - r = ASSERT_OK(pidref_safe_fork("(umount-rec-stale)", FORK_COMMON_FLAGS, NULL)); + r = ASSERT_OK(pidref_safe_fork("(umount-rec-stale)", FORK_COMMON_FLAGS, /* ret= */ NULL)); if (r == 0) { /* child */ - ASSERT_OK(mount_nofollow_verbose(LOG_INFO, "tmpfs", t, "tmpfs", 0, NULL)); + ASSERT_OK(mount_nofollow_verbose(LOG_INFO, "tmpfs", t, "tmpfs", /* flags= */ 0, /* options= */ NULL)); _cleanup_free_ char *src = ASSERT_NOT_NULL(path_join(t, "src")), @@ -411,11 +411,11 @@ TEST(umount_recursive_stale_path) { /* The mount that later ends up below the parent has to be older than the parent, so that the * parent is reached first when the table is iterated backwards. Once the parent is detached * the path of the child resolves via the symlink it covered, to a mount outside of the prefix. */ - ASSERT_OK(mount_nofollow_verbose(LOG_INFO, "tmpfs", src, "tmpfs", 0, NULL)); - ASSERT_OK(mount_nofollow_verbose(LOG_INFO, "tmpfs", victim, "tmpfs", 0, NULL)); - ASSERT_OK(mount_nofollow_verbose(LOG_INFO, "tmpfs", parent, "tmpfs", 0, NULL)); + ASSERT_OK(mount_nofollow_verbose(LOG_INFO, "tmpfs", src, "tmpfs", /* flags= */ 0, /* options= */ NULL)); + ASSERT_OK(mount_nofollow_verbose(LOG_INFO, "tmpfs", victim, "tmpfs", /* flags= */ 0, /* options= */ NULL)); + ASSERT_OK(mount_nofollow_verbose(LOG_INFO, "tmpfs", parent, "tmpfs", /* flags= */ 0, /* options= */ NULL)); ASSERT_OK(mkdir_p(child, 0755)); - ASSERT_OK(mount_nofollow_verbose(LOG_INFO, src, child, NULL, MS_MOVE, NULL)); + ASSERT_OK(mount_nofollow_verbose(LOG_INFO, src, child, /* fstype= */ NULL, MS_MOVE, /* options= */ NULL)); ASSERT_OK_POSITIVE(umount_recursive(prefix, MNT_DETACH)); @@ -432,11 +432,11 @@ TEST(umount_recursive_busy) { CHECK_PRIV; - ASSERT_OK(mkdtemp_malloc(NULL, &t)); + ASSERT_OK(mkdtemp_malloc(/* template= */ NULL, &t)); - r = ASSERT_OK(pidref_safe_fork("(umount-rec-busy)", FORK_COMMON_FLAGS, NULL)); + r = ASSERT_OK(pidref_safe_fork("(umount-rec-busy)", FORK_COMMON_FLAGS, /* ret= */ NULL)); if (r == 0) { /* child */ - ASSERT_OK(mount_nofollow_verbose(LOG_INFO, "tmpfs", t, "tmpfs", 0, NULL)); + ASSERT_OK(mount_nofollow_verbose(LOG_INFO, "tmpfs", t, "tmpfs", /* flags= */ 0, /* options= */ NULL)); _cleanup_free_ char *prefix = ASSERT_NOT_NULL(path_join(t, "prefix")), @@ -447,12 +447,12 @@ TEST(umount_recursive_busy) { ASSERT_OK(mkdir_p(busy, 0755)); /* The busy mount is newer, hence reached first. It must not stop the walk. */ - ASSERT_OK(mount_nofollow_verbose(LOG_INFO, "tmpfs", older, "tmpfs", 0, NULL)); - ASSERT_OK(mount_nofollow_verbose(LOG_INFO, "tmpfs", busy, "tmpfs", 0, NULL)); + ASSERT_OK(mount_nofollow_verbose(LOG_INFO, "tmpfs", older, "tmpfs", /* flags= */ 0, /* options= */ NULL)); + ASSERT_OK(mount_nofollow_verbose(LOG_INFO, "tmpfs", busy, "tmpfs", /* flags= */ 0, /* options= */ NULL)); _unused_ _cleanup_close_ int fd = ASSERT_OK(xopenat(AT_FDCWD, busy, O_DIRECTORY)); - ASSERT_OK_EQ(umount_recursive(prefix, 0), 1); + ASSERT_OK_EQ(umount_recursive(prefix, /* flags= */ 0), 1); ASSERT_OK_ZERO(path_is_mount_point(older)); ASSERT_OK_POSITIVE(path_is_mount_point(busy)); @@ -663,7 +663,7 @@ TEST(umountat) { CHECK_PRIV; _cleanup_(rm_rf_physical_and_freep) char *p = NULL; - _cleanup_close_ int dfd = mkdtemp_open(/* template= */ NULL, 0, &p); + _cleanup_close_ int dfd = mkdtemp_open(/* template= */ NULL, /* flags= */ 0, &p); ASSERT_OK(dfd); ASSERT_OK(mkdirat(dfd, "foo", 0777)); diff --git a/src/test/test-process-util.c b/src/test/test-process-util.c index 48ebdca654f..bc997d81430 100644 --- a/src/test/test-process-util.c +++ b/src/test/test-process-util.c @@ -1164,7 +1164,7 @@ TEST(execvp_safe) { /* An empty executable file must make execvp_safe() fail with ENOEXEC, and must not be run as an * empty shell script, which would exit successfully. */ - ASSERT_OK(mkdtemp_malloc(NULL, &d)); + ASSERT_OK(mkdtemp_malloc(/* template= */ NULL, &d)); ASSERT_NOT_NULL(p = path_join(d, "test-process-util-empty")); ASSERT_OK(touch_file(p, /* parents= */ false, USEC_INFINITY, UID_INVALID, GID_INVALID, 0755)); if (access(p, X_OK) < 0) /* e.g. /tmp/ mounted noexec */ @@ -1173,7 +1173,7 @@ TEST(execvp_safe) { test_execvp_safe_one(p, /* search_path= */ NULL, -ENOEXEC); test_execvp_safe_one("test-process-util-empty", d, -ENOEXEC); test_execvp_safe_one("test-process-util-missing", d, -ENOENT); - test_execvp_safe_one("true", /* search_path= */ NULL, 0); + test_execvp_safe_one("true", /* search_path= */ NULL, /* expected_error= */ 0); } static int intro(void) { diff --git a/src/test/test-seccomp.c b/src/test/test-seccomp.c index 389442f758f..e88e9938380 100644 --- a/src/test/test-seccomp.c +++ b/src/test/test-seccomp.c @@ -558,7 +558,7 @@ TEST(restrict_realtime) { .sched_policy = SCHED_FIFO, .sched_priority = 1, }; - ASSERT_ERROR_ERRNO(sched_setattr(0, &attr, 0), ENOANO); + ASSERT_ERROR_ERRNO(sched_setattr(/* pid= */ 0, &attr, /* flags= */ 0), ENOANO); attr = (struct sched_attr) { .size = sizeof(attr), @@ -567,13 +567,13 @@ TEST(restrict_realtime) { .sched_deadline = 10000000, .sched_period = 10000000, }; - ASSERT_ERROR_ERRNO(sched_setattr(0, &attr, 0), ENOANO); + ASSERT_ERROR_ERRNO(sched_setattr(/* pid= */ 0, &attr, /* flags= */ 0), ENOANO); attr = (struct sched_attr) { .size = sizeof(attr), .sched_policy = SCHED_OTHER, }; - ASSERT_ERROR_ERRNO(sched_setattr(0, &attr, 0), ENOANO); + ASSERT_ERROR_ERRNO(sched_setattr(/* pid= */ 0, &attr, /* flags= */ 0), ENOANO); _exit(EXIT_SUCCESS); } @@ -1024,7 +1024,7 @@ TEST(lock_personality) { } /* Each bit set in the locked personality takes a rule of its own. */ - r = ASSERT_OK(pidref_safe_fork("(lock-personality-flags)", FORK_LOG|FORK_WAIT, NULL)); + r = ASSERT_OK(pidref_safe_fork("(lock-personality-flags)", FORK_LOG|FORK_WAIT, /* ret= */ NULL)); if (r == 0) { unsigned long personality = current_opinionated | ADDR_NO_RANDOMIZE | ADDR_COMPAT_LAYOUT; diff --git a/src/test/test-socket-netlink.c b/src/test/test-socket-netlink.c index adb1e0427ae..ebedeb2c8b0 100644 --- a/src/test/test-socket-netlink.c +++ b/src/test/test-socket-netlink.c @@ -217,7 +217,7 @@ TEST(socket_address_is) { SocketAddress a; ASSERT_OK(socket_address_parse(&a, "192.168.1.1:8888")); - ASSERT_TRUE(socket_address_is(&a, "192.168.1.1:8888", 0 /* unspecified yet */)); + ASSERT_TRUE(socket_address_is(&a, "192.168.1.1:8888", /* type= */ 0 /* unspecified yet */)); ASSERT_FALSE(socket_address_is(&a, "route", /* type= */ 0)); ASSERT_FALSE(socket_address_is(&a, "route", SOCK_STREAM)); ASSERT_FALSE(socket_address_is(&a, "192.168.1.1:8888", SOCK_RAW)); diff --git a/src/test/test-tpm2.c b/src/test/test-tpm2.c index 05cebbcd67f..78af409038a 100644 --- a/src/test/test-tpm2.c +++ b/src/test/test-tpm2.c @@ -1338,7 +1338,7 @@ TEST(calculate_policy_or) { /* The empty input must be rejected before the flat/tree split. */ actual = (TPM2B_DIGEST) { .size = SHA256_DIGEST_SIZE }; - ASSERT_ERROR(tpm2_calculate_policy_or(leaves, 0, &actual), EINVAL); + ASSERT_ERROR(tpm2_calculate_policy_or(leaves, /* n_branches= */ 0, &actual), EINVAL); /* For n<=8 the flat form must be used unchanged; for n>8 the result must match the balanced tree * reference implementation. */ diff --git a/src/test/test-xattr-match.c b/src/test/test-xattr-match.c index 4b39b0958e3..26f08405faa 100644 --- a/src/test/test-xattr-match.c +++ b/src/test/test-xattr-match.c @@ -125,7 +125,7 @@ TEST(xattr_match_filter_test_uapi16_object) { _cleanup_close_ int tfd = -EBADF; /* A root to load the host properties from */ - ASSERT_OK(tfd = mkdtemp_open(NULL, O_PATH, &t)); + ASSERT_OK(tfd = mkdtemp_open(/* template= */ NULL, O_PATH, &t)); ASSERT_OK_ERRNO(mkdirat(tfd, "etc", 0755)); ASSERT_OK(write_string_file_at(tfd, "etc/hostname", "foo.example.com", WRITE_STRING_FILE_CREATE)); ASSERT_OK(write_string_file_at(tfd, "etc/machine-id", "4b1f7c950d4f4e1a9e8f3d22576180a3", WRITE_STRING_FILE_CREATE)); @@ -200,7 +200,7 @@ TEST(host_properties_load_symlinks) { _cleanup_close_ int tfd = -EBADF; /* Absolute and relative symlinks are resolved below the root, not on the host */ - ASSERT_OK(tfd = mkdtemp_open(NULL, O_PATH, &t)); + ASSERT_OK(tfd = mkdtemp_open(/* template= */ NULL, O_PATH, &t)); ASSERT_OK_ERRNO(mkdirat(tfd, "etc", 0755)); ASSERT_OK(write_string_file_at(tfd, "etc/hostname.real", "inroot.example.com", WRITE_STRING_FILE_CREATE)); ASSERT_OK_ERRNO(symlinkat("/etc/hostname.real", tfd, "etc/hostname")); @@ -220,7 +220,7 @@ TEST(xattr_match_filter_test_inode) { _cleanup_close_ int tfd = -EBADF; int r; - ASSERT_OK(tfd = mkdtemp_open(NULL, O_DIRECTORY, &t)); + ASSERT_OK(tfd = mkdtemp_open(/* template= */ NULL, O_DIRECTORY, &t)); ASSERT_OK(write_string_file_at(tfd, "plain", "x", WRITE_STRING_FILE_CREATE)); ASSERT_OK(write_string_file_at(tfd, "mine", "x", WRITE_STRING_FILE_CREATE)); ASSERT_OK(write_string_file_at(tfd, "other", "x", WRITE_STRING_FILE_CREATE)); diff --git a/tools/check-argument-comments.py b/tools/check-argument-comments.py new file mode 100755 index 00000000000..1e100e4fd5c --- /dev/null +++ b/tools/check-argument-comments.py @@ -0,0 +1,1001 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: LGPL-2.1-or-later +"""Report function arguments passed as a bare NULL, 0, false or true without a /* name= */ comment. + +docs/CODING_STYLE.md asks for the argument name in a comment whenever NULL or another value meaning +"unset" is passed to a function. clang-tidy's bugprone-argument-comment cannot check that for C code: +it never sees NULL behind its cast, and asking it to comment integer literals flags every size and +mode as well. This walks the AST with libclang instead, takes the parameter names from the actual +prototypes and looks only at the literals the rule is about. A comment that names a different +parameter is reported too, those go stale when parameters are renamed. As for bugprone-argument-comment +only a /* name= */ comment names a parameter, case and surrounding underscores aside, a /* name */ that +lost its '=' is reported as well, and any other comment, in front of the literal or after it, leaves it +bare. + +The names come from the first declaration the caller sees, usually the header, which is also the one +bugprone-argument-comment compares against. Parameters whose name says nothing, a single letter, argN +or a reserved __name copied from a libc declaration, are left alone. Calls to functions declared outside +the source tree and the build directory, e.g. libc, are skipped unless --system is given; calls through +the sym_* pointers of the dlopen() wrappers are ours and are checked against the function the pointer +was declared from, except that a comment naming none of its parameters is only warned about, library +headers rename parameters between versions. Calls produced by a macro body are skipped, the caller never +spelled the argument, while a call written inside a macro argument, assert_se(f(NULL)), is checked, and +so is a call through a macro that only renames the function, #define f f_shim. The in-tree headers a +file includes are checked from its translation unit, which covers their static inline functions, and a +file built more than once, for userspace and for the boot loader say, is checked with each set of flags. +With --diff REV only lines that differ from REV, and files git does not track, are reported, which is +what a review wants. Rust code is not checked. + +Usage: tools/check-argument-comments.py [-p BUILDDIR] [--root DIR] [--diff REV] [--fix] [-j JOBS] [--system] PATH... + +PATH is a C file or a directory to search for them, BPF programs are skipped. The compile flags come +from compile_commands.json in the build directory, the generated headers must exist ("meson compile -C +BUILDDIR gensources"); a file that is not in there, or does not parse, is an error and fails the run, +except that --diff skips files the build does not compile. --fix leaves the comments that name the +wrong parameter and files whose translation unit did not parse to a human. libclang is the one the +bindings find, else the one LIBCLANG_PATH names, the library or its directory, else the newest one +installed, and has to be 19 or newer. Exits with 1 when something is left to fix, 2 on errors and 77 +when the libclang python bindings are missing. +""" + +import argparse +import bisect +import glob +import json +import os +import re +import shlex +import shutil +import signal +import subprocess +import sys +from collections.abc import Iterator +from concurrent.futures import BrokenExecutor, ProcessPoolExecutor +from ctypes import POINTER, byref, c_uint, c_void_p, cast +from typing import Any, NamedTuple, Optional + +try: + import clang.cindex as ci # type: ignore +except ImportError: + print('python3-clang is not installed', file=sys.stderr) + sys.exit(77) + +# The argument lexer steps over comments, string and character literals and preprocessor lines, which can +# hold anything, and splits on the commas outside of nested brackets. +TOKEN_RE = re.compile( + rb'/\*.*?\*/|//(?:\\\n|[^\n])*|"(?:\\.|[^"\\\n])*"|\'(?:\\.|[^\'\\\n])*\'|^[ \t]*#(?:\\\n|[^\n])*|[()\[\]{},]', + re.DOTALL | re.MULTILINE, +) +# What may surround an argument, a newline on its own so that a preprocessor line can start after it +GAP_RE = re.compile( + rb'^[ \t]*#(?:\\\n|[^\n])*|[ \t\r\f\v]+|\n|/\*.*?\*/|//(?:\\\n|[^\n])*', re.DOTALL | re.MULTILINE +) +LITERAL_RE = re.compile(rb'(?:NULL|0|false|true)(?![\w.])') +# [] is how an array parameter is declared +NAME_COMMENT_RE = re.compile(rb'/\*\s*(\w+)(?:\[\])?\s*(=)?\s*\*/') +# The callee, maybe in parentheses, up to the parenthesis that opens the arguments +CALL_RE = re.compile(rb'((?:\(\s*)*)([A-Za-z_]\w*)\s*((?:\)\s*)*)\(') +ALIAS_RE = re.compile( + rb'[ \t]*#[ \t]*define[ \t]+(\w+)[ \t]+(\w+)[ \t]*(?:/\*[^\n]*?\*/[ \t]*)*(?://[^\n]*)?(?=\n|\Z)' +) +INCLUDE_RE = re.compile(rb'^[ \t]*#[ \t]*include[ \t]*[<"]([^>"\n]+)[>"]', re.MULTILINE) +HUNK_RE = re.compile(r'@@ -\S+ \+(\d+)(?:,(\d+))? @@') +UNINFORMATIVE_RE = re.compile(r'^(?:[a-z]|arg\d*|__.*)$', re.IGNORECASE) +# gcc-only flags of the EFI build that libclang rejects, .clang-tidy drops the same ones through RemovedArgs +DROPPED_FLAGS = frozenset({'-fwide-exec-charset=UCS2', '-maccumulate-outgoing-args'}) +LAUNCHERS = frozenset({'ccache', 'sccache', 'distcc', 'icecc'}) +# A cross compiler named after its target, which clang's tooling turns into --target= as well +CROSS_COMPILER_RE = re.compile( + r'((?:aarch64|alpha|arc|arm|csky|hppa|i[3-6]86|ia64|loongarch|m68k|microblaze|mips|or1k|powerpc|ppc|riscv' + r'|s390|sh|sparc|x86_64|xtensa)\w*(?:-\w+)+)-(?:gcc|cc|clang)(?:-[\d.]+)?$' +) + +CALL_EXPR = ci.CursorKind.CALL_EXPR.value +DECL_REF_EXPR = ci.CursorKind.DECL_REF_EXPR.value + +Command = tuple[str, tuple[str, ...]] +Edit = tuple[int, int, str] + + +class Finding(NamedTuple): + file: str + line: int + column: int + offset: int + callee: str + param: str + literal: str + comment: Optional[str] # None for a bare literal, else the text of the comment in front of it + fix: Optional[Edit] # what adds or completes the comment, None when a human has to look + + +class Result(NamedTuple): + path: str + findings: list[Finding] + errors: list[str] + warnings: list[str] + + +class Item(NamedTuple): + path: str + root: str + build_dir: str + command: Command + include_system: bool + only: Optional[dict[str, list[int]]] + resources: Optional[str] + + +class GitError(Exception): + pass + + +def newest_first(libraries: list[str]) -> list[str]: + return sorted( + libraries, key=lambda p: [int(x) if x.isdigit() else x for x in re.split(r'(\d+)', p)], reverse=True + ) + + +def setup_libclang(library_file: Optional[str] = None) -> Optional[str]: + """Loads libclang and returns the library file the workers are to load, None when the bindings find it + on their own. A worker loads the one the parent passes.""" + if ci.Config.loaded: + return library_file # a forked worker inherits the parent's library + if library_file is not None: + ci.Config.set_library_file(library_file) + ci.conf.lib + return library_file + try: + ci.conf.lib + return None + except ci.LibclangError as e: + error = e + candidates = [] + path = os.environ.get('LIBCLANG_PATH') + if path: + path = os.path.realpath(path) # a worker resolves relative paths elsewhere + if os.path.isdir(path): # the bindgen convention + candidates += newest_first(glob.glob(os.path.join(glob.escape(path), 'libclang*.so*'))) + else: + candidates.append(path) + candidates += newest_first( + glob.glob('/usr/lib/llvm-*/lib/libclang-*.so.1') + + glob.glob('/usr/lib*/libclang.so*') + + glob.glob('/usr/lib/*/libclang*.so*') + ) + for candidate in candidates: + ci.Config.set_library_file(candidate) # refused only once a library has loaded + try: + ci.conf.lib + return candidate + except ci.LibclangError: + continue + print(f'error: cannot load libclang: {error}', file=sys.stderr) + sys.exit(2) + + +def setup_worker(library_file: Optional[str]) -> None: + signal.signal(signal.SIGINT, signal.SIG_IGN) # Ctrl-C is for the parent, which cancels the run + setup_libclang(library_file) + + +# The libclang 19 bindings only have the callbacks dictionary +VISIT = getattr(ci, 'cursor_visit_callback', None) or ci.callbacks['cursor_visit'] + + +def _children(child: Any, parent: Any, out: list[Any]) -> int: + out.append(child) + return 1 # CXChildVisit_Continue + + +def _calls(child: Any, parent: Any, out: list[Any]) -> int: + if child._kind_id == CALL_EXPR: + out.append(child) + return 2 # CXChildVisit_Recurse + + +def _decl_refs(child: Any, parent: Any, out: list[Any]) -> int: + if child._kind_id == DECL_REF_EXPR: + out.append(child) + return 2 + + +CHILDREN, CALLS, DECL_REFS = VISIT(_children), VISIT(_calls), VISIT(_decl_refs) + + +def visit(cursor: Any, callback: Any) -> list[Any]: + """The cursors below cursor the callback picks. libclang does the walking, which is a lot cheaper than + walk_preorder() calling into it for every node, and does not recurse in python.""" + out: list[Any] = [] + ci.conf.lib.clang_visitChildren(cursor, callback, out) + for c in out: + c._tu = cursor._tu # keeps the translation unit alive, as the bindings do + return out + + +LOCATIONS: dict[str, Any] = {} + + +def location_function(name: str) -> Any: + """clang_getFileLocation() gives where a token was spelled if it came from a macro argument and the macro + invocation if it came from a macro body, clang_getSpellingLocation() the #define for the latter and + clang_getExpansionLocation() always the invocation. The bindings only wrap the expansion location.""" + fn = LOCATIONS.get(name) + if fn is None: + fn = LOCATIONS[name] = getattr(ci.conf.lib, name) + fn.argtypes = [ + ci.SourceLocation, + POINTER(ci.c_object_p), + POINTER(c_uint), + POINTER(c_uint), + POINTER(c_uint), + ] + fn.restype = None + return fn + + +SOURCES: dict[str, Optional[bytes]] = {} +LINES: dict[str, list[int]] = {} +REAL_PATHS: dict[tuple[str, str], str] = {} +ARGUMENTS: dict[tuple[str, int], dict[int, tuple[bytes, list[tuple[int, int]]]]] = {} + + +def read(path: str) -> Optional[bytes]: + if path not in SOURCES: + try: + with open(path, 'rb') as f: + SOURCES[path] = f.read() # libclang offsets are byte offsets + except OSError: + SOURCES[path] = None + return SOURCES[path] + + +def position(path: str, offset: int) -> tuple[int, int]: + """(line, column) of a byte offset.""" + starts = LINES.get(path) + if starts is None: + starts = LINES[path] = [0] + [m.end() for m in re.finditer(rb'\n', read(path) or b'')] + line = bisect.bisect_right(starts, offset) + return line, offset - starts[line - 1] + 1 + + +def touches(lines: list[int], first: int, last: int) -> bool: + """Whether one of the sorted lines lies in [first, last].""" + i = bisect.bisect_left(lines, first) + return i < len(lines) and lines[i] <= last + + +def real_path(directory: str, name: str) -> str: + key = (directory, name) + path = REAL_PATHS.get(key) + if path is None: + path = REAL_PATHS[key] = os.path.realpath(os.path.join(directory, name)) + return path + + +def source_path(path: str) -> str: + """A file to check, its directories resolved but not the file itself. test-bus-vtable-cc.cc is a + symlink to test-bus-vtable.c and a C++ translation unit of its own.""" + path = os.path.abspath(path) + return os.path.join(os.path.realpath(os.path.dirname(path)), os.path.basename(path)) + + +def display(path: str, root: str) -> str: + return os.path.relpath(path, root) if path.startswith(root + os.sep) else path + + +def split_arguments(source: bytes, open_paren: int) -> Optional[list[tuple[int, int]]]: + """[start, end) of each argument of the call whose argument list opens at open_paren, None when the + brackets do not match up.""" + ranges: list[tuple[int, int]] = [] + depth, start = 0, open_paren + 1 + for m in TOKEN_RE.finditer(source, open_paren): + token = m.group() + if len(token) != 1: + continue # a comment, a literal or a preprocessor line + if token in b'([{': + depth += 1 + elif token in b')]}': + depth -= 1 + if depth == 0: + ranges.append((start, m.start())) + return ranges if token == b')' else None + elif token == b',' and depth == 1: + ranges.append((start, m.start())) + start = m.end() + return None + + +def skip_gap(source: bytes, pos: int, end: int, comments: Optional[list[tuple[int, int]]] = None) -> int: + """Steps over whitespace, comments and preprocessor lines and collects the block comments.""" + while pos < end: + m = GAP_RE.match(source, pos, end) + if not m: + break + if comments is not None and source.startswith(b'/*', pos): + comments.append((pos, m.end())) + pos = m.end() + return pos + + +def bare_literal(source: bytes, start: int, end: int) -> Optional[tuple[int, bytes, list[tuple[int, int]]]]: + """(offset, literal, block comments in front) when the argument in [start, end) is nothing but NULL, 0, + false or true, in parentheses or not.""" + comments: list[tuple[int, int]] = [] + pos = arg = skip_gap(source, start, end, comments) + depth = 0 + while pos < end and source[pos] == ord('('): + depth += 1 + pos = skip_gap(source, pos + 1, end) + m = LITERAL_RE.match(source, pos, end) + if not m: + return None + pos = skip_gap(source, m.end(), end) + while depth and pos < end and source[pos] == ord(')'): + depth -= 1 + pos = skip_gap(source, pos + 1, end) + return (arg, m.group(), comments) if depth == 0 and pos == end else None + + +def literal_arguments( + path: str, source: bytes, open_paren: int +) -> dict[int, tuple[bytes, list[tuple[int, int]]]]: + """The arguments of a call that are written as a bare literal, by offset. Headers come up in every file + that includes them, so this is kept for the whole run of a worker.""" + key = (path, open_paren) + found = ARGUMENTS.get(key) + if found is None: + found = ARGUMENTS[key] = {} + for start, end in split_arguments(source, open_paren) or []: + literal = bare_literal(source, start, end) + if literal: + found[literal[0]] = (literal[1], literal[2]) + return found + + +def name_comment( + source: bytes, comments: list[tuple[int, int]], arg: int +) -> Optional[tuple[str, bool, int, int]]: + """The parameter the comments in front of an argument name: (name, whether the '=' is there, start and end + of the comment). A /* name= */ counts anywhere after the previous argument, a /* name */ only right in + front of this one and any other comment names nothing.""" + for start, end in reversed(comments): + m = NAME_COMMENT_RE.fullmatch(source, start, end) + if m and m.group(2): + return m.group(1).decode(), True, start, end + if comments: + start, end = comments[-1] + m = NAME_COMMENT_RE.fullmatch(source, start, end) + if m and not source[end:arg].strip(b' \t'): + return m.group(1).decode(), False, start, end + return None + + +def same_name(a: str, b: str) -> bool: + """The comparison of bugprone-argument-comment without StrictMode.""" + return a.strip('_').lower() == b.strip('_').lower() + + +class Checker: + """The calls of one translation unit.""" + + def __init__(self, item: Item, tu: Any): + self.item = item + self.tu = tu + self.directory = item.command[0] + self.root = item.root + os.sep + self.build_dir = item.build_dir + os.sep + self.main = os.path.realpath(item.path) + self.paths: dict[Optional[int], str] = {} + self.ours_by_name: dict[str, bool] = {} + self.params_by_name: dict[str, list[str]] = {} + self.syms: dict[str, Optional[Any]] = {} + self.functions: Optional[dict[str, Any]] = None + self.done: set[tuple[str, int, str]] = set() + self.findings: list[Finding] = [] + self.warnings: list[str] = [] + + def locate(self, function: str, location: Any) -> tuple[Optional[str], int]: + """(real path, offset) of a location, location_function() tells which one.""" + f, offset = ci.c_object_p(), c_uint() + location_function(function)(location, byref(f), None, None, byref(offset)) + if not f: + return None, 0 + key = cast(f, c_void_p).value + path = self.paths.get(key) + if path is None: + path = self.paths[key] = real_path(self.directory, ci.File(f).name) + return path, offset.value + + def ours(self, path: str) -> bool: + """Whether a file is the one checked or one of the in-tree headers it includes.""" + return path == self.main or (path.startswith(self.root) and not path.startswith(self.build_dir)) + + def top_level(self) -> Iterator[Any]: + """The definitions in the file and in the in-tree headers it includes. Under --diff only those that + span a changed line.""" + only = self.item.only + for c in visit(self.tu.cursor, CHILDREN): + path, _ = self.locate('clang_getExpansionLocation', c.location) + if path is None or not self.ours(path): + continue + if only is not None: + lines = only.get(path) + if not lines: + continue + extent = c.extent + start = self.locate('clang_getExpansionLocation', extent.start)[1] + end = self.locate('clang_getExpansionLocation', extent.end)[1] + if not touches(lines, position(path, start)[0], position(path, end)[0]): + continue + yield c + + def declared_by_us(self, function: Any) -> bool: + """Whether a function's first declaration is in the source tree, or in a header generated into the + build directory, which need not be below it.""" + ours = self.ours_by_name.get(function.spelling) + if ours is None: + path, _ = self.locate('clang_getExpansionLocation', function.canonical.location) + ours = path is not None and (path.startswith(self.root) or path.startswith(self.build_dir)) + self.ours_by_name[function.spelling] = ours + return ours + + def parameters(self, function: Any) -> list[str]: + params = self.params_by_name.get(function.spelling) + if params is None: + params = self.params_by_name[function.spelling] = [ + p.spelling for p in function.canonical.get_arguments() + ] + return params + + def sym_function(self, var: Any) -> Optional[Any]: + """The function a sym_* pointer was declared from with typeof(&function), which also sees through a + library renaming its functions with macros, pcre2 does.""" + if var.spelling not in self.syms: + function = None + for ref in visit(var, DECL_REFS): + target = ref.referenced + if target is not None and target.kind == ci.CursorKind.FUNCTION_DECL: + function = target + break + if function is None: + if self.functions is None: + self.functions = { + c.spelling: c + for c in visit(self.tu.cursor, CHILDREN) + if c.kind == ci.CursorKind.FUNCTION_DECL + } + function = self.functions.get(var.spelling[4:]) + self.syms[var.spelling] = function + return self.syms[var.spelling] + + def alias(self, path: Optional[str], offset: int, name: bytes, target: str) -> bool: + """Whether the token at offset is the body of '#define name target', a macro that only renames.""" + source = read(path) if path else None + if source is None: + return False + m = ALIAS_RE.match(source, source.rfind(b'\n', 0, offset) + 1) + return ( + m is not None and m.group(1) == name and m.group(2) == target.encode() and m.start(2) == offset + ) + + def callee(self, call: Any, name: bytes) -> Optional[tuple[Any, bool]]: + """The function a call names and whether it does so through a sym_* pointer. None for calls through + an expression, calls a macro body produced and functions that are not ours.""" + children = visit(call, CHILDREN) + expr = children[0] if children else None + while expr is not None and expr.kind in (ci.CursorKind.UNEXPOSED_EXPR, ci.CursorKind.PAREN_EXPR): + inner = visit(expr, CHILDREN) + expr = inner[0] if len(inner) == 1 else None + if expr is None or expr._kind_id != DECL_REF_EXPR: + return None # the pointer some expression yields, lookup(x)(NULL) say + decl = expr.referenced + if decl is None: + return None + spelled = self.locate('clang_getSpellingLocation', expr.location) + if spelled != self.locate('clang_getFileLocation', expr.location) and not self.alias( + *spelled, name, decl.spelling + ): + return None # the product of a macro body, the caller never spelled the arguments + if decl.kind == ci.CursorKind.FUNCTION_DECL: + if not self.item.include_system and not self.declared_by_us(decl): + return None + return decl, False + if decl.kind == ci.CursorKind.VAR_DECL and decl.spelling.startswith('sym_'): + function = self.sym_function(decl) + return (function, True) if function is not None else None + return None + + def check(self, call: Any) -> None: + path, offset = self.locate('clang_getFileLocation', call.location) + if path is None or not self.ours(path): + return + only_lines = None + if self.item.only is not None: + only_lines = self.item.only.get(path) + if not only_lines: + return + source = read(path) + if source is None: + return + m = CALL_RE.match(source, offset) + if not m or m.group(1).count(b'(') != m.group(3).count(b')'): + return # the call is not spelled out here, a macro body produced it + open_paren = m.end() - 1 + literals = literal_arguments(path, source, open_paren) + if not literals: + return + callee = self.callee(call, m.group(2)) + if callee is None: + return + function, via_sym = callee + key = (path, open_paren, function.spelling) + if key in self.done: + return # a macro such as ASSERT_EQ() has its argument more than once + self.done.add(key) + name = m.group(2).decode() + params = self.parameters(function) + for i, arg in enumerate(call.get_arguments()): + if i >= len(params): + break # the rest is variadic + param = params[i] + if arg is None or not param or UNINFORMATIVE_RE.match(param): + continue + arg_path, arg_offset = self.locate('clang_getFileLocation', arg.location) + found = literals.get(arg_offset) if arg_path == path else None + if found is None: + continue # not written as a bare literal, or not at this position of the call + line, column = position(path, arg_offset) + if only_lines is not None and not touches(only_lines, line, line): + continue + literal, comments = found + named = name_comment(source, comments, arg_offset) + comment: Optional[str] = None + fix: Optional[Edit] = None + if named is None: + fix = (arg_offset, arg_offset, f'/* {param}= */ ') + else: + text, has_equals, start, end = named + if has_equals and same_name(text, param): + continue + comment = source[start + 2 : end - 2].strip().decode(errors='replace') + if via_sym and not any(p and same_name(text, p) for p in params): + self.warnings.append( + f'{display(path, self.item.root)}:{line}:{column}: {name}(): ' + f"'{comment}' names no parameter of {function.spelling}(), " + f"its header calls argument {i + 1} '{param}'" + ) + continue + if not has_equals and same_name(text, param): + fix = (start, end, f'/* {param}= */') + self.findings.append( + Finding(path, line, column, arg_offset, name, param, literal.decode(), comment, fix) + ) + + +def describe(diagnostic: Any, root: str) -> str: + location = diagnostic.location + if not location.file: + return str(diagnostic.spelling) + return f'{display(os.path.realpath(location.file.name), root)}:{location.line}: {diagnostic.spelling}' + + +def check_file(item: Item) -> Result: + directory, flags = item.command + os.chdir(directory) + args = ([f'-resource-dir={item.resources}'] if item.resources else []) + list(flags) + try: + tu = ci.Index.create().parse(item.path, args=args) + except ci.TranslationUnitLoadError as e: + return Result(item.path, [], [str(e)], []) + errors = [describe(d, item.root) for d in tu.diagnostics if d.severity >= ci.Diagnostic.Error] + checker = Checker(item, tu) + for top in checker.top_level(): + for call in visit(top, CALLS): + checker.check(call) + return Result(item.path, checker.findings, errors, checker.warnings) + + +def worker(item: Item) -> Result: + try: + return check_file(item) + except Exception as e: # a compile directory that is gone, say, fails this file and not the run + return Result(item.path, [], [f'{type(e).__name__}: {e}'], []) + + +def expand_response_files(argv: list[str], directory: str, depth: int = 0) -> list[str]: + out: list[str] = [] + for a in argv: + if a.startswith('@') and depth < 8: + try: + with open(os.path.join(directory, a[1:])) as f: + out += expand_response_files(shlex.split(f.read()), directory, depth + 1) + continue + except OSError: + pass + out.append(a) + return out + + +def compile_flags(argv: list[str], directory: str, path: str) -> tuple[str, ...]: + """The flags of a compile command minus what only matters to a real compile, with the target a cross + compiler implies made explicit, as clang's tooling does.""" + while len(argv) > 1 and os.path.basename(argv[0]) in LAUNCHERS: + argv = argv[1:] + argv = expand_response_files(argv, directory) + flags: list[str] = [] + skip = False + for a in argv[1:]: + if skip: + skip = False + elif a in ('-o', '-MF', '-MQ', '-MT'): + skip = True + elif a in ('-c', '-MD', '-MMD') or a in DROPPED_FLAGS or a.startswith('-fdiagnostics'): + continue + elif a.startswith('-Wp,'): + if not any(p.startswith('-M') for p in a[4:].split(',')): + flags.append(a) # -D, -U and -include for the preprocessor, but no dependency files + elif a.startswith('-W'): + continue + elif ( + a.startswith('-') + or os.path.basename(a) != os.path.basename(path) + or source_path(os.path.join(directory, a)) != path + ): + flags.append(a) + m = CROSS_COMPILER_RE.match(os.path.basename(argv[0])) + if m and not any(f.startswith('--target=') or f == '-target' for f in flags): + flags.insert(0, f'--target={m.group(1)}') + return tuple(flags) + + +def compile_commands(build_dir: str, wanted: set[str]) -> dict[str, list[Command]]: + """file -> each distinct (directory, flags) it is compiled with, for the files asked for.""" + with open(os.path.join(build_dir, 'compile_commands.json')) as f: + entries = json.load(f) + commands: dict[str, list[Command]] = {} + for e in entries: + path = source_path(os.path.join(e['directory'], e['file'])) + if path not in wanted: + continue + argv = e['arguments'] if 'arguments' in e else shlex.split(e['command']) + command = (e['directory'], compile_flags(argv, e['directory'], path)) + if command not in commands.setdefault(path, []): + commands[path].append(command) + return commands + + +def libclang_major() -> Optional[int]: + try: + fn = ci.conf.lib.clang_getClangVersion # the bindings do not declare its return type + fn.restype = ci._CXString + version = str(ci._CXString.from_result(fn())) + except AttributeError: + return None + m = re.search(r'clang version (\d+)', version) + return int(m.group(1)) if m else None + + +def resource_dir() -> Optional[str]: + """Where clang's builtin headers, stddef.h and friends, live. libclang derives that from its own location, + which goes wrong when the library does not sit next to them, so ask the clang binary of the same major + version instead. An unversioned clang only counts when it is that version, the headers of another one + need not parse with this libclang.""" + major = libclang_major() + if major is None: + return None + for name in (f'clang-{major}', 'clang'): + clang = shutil.which(name) + if not clang: + continue + r = subprocess.run([clang, '-print-resource-dir'], capture_output=True, text=True) + path = r.stdout.strip() + if ( + r.returncode == 0 + and os.path.basename(path).split('.')[0] == str(major) + and os.path.exists(os.path.join(path, 'include', 'stddef.h')) + ): + return path + return None + + +def source_root(path: str) -> Optional[str]: + """The tree the file belongs to, which need not be the one this script lives in.""" + try: + r = subprocess.run( + ['git', '-C', os.path.dirname(path), 'rev-parse', '--show-toplevel'], + capture_output=True, + text=True, + ) + except FileNotFoundError: + return None + return os.path.realpath(r.stdout.strip()) if r.returncode == 0 else None + + +def twin_header(path: str) -> Optional[str]: + """The header a .c file is paired with.""" + header = path[:-2] + '.h' + return header if os.path.exists(header) else None + + +def includes(path: str, header: str) -> bool: + name = os.path.basename(header).encode() + return any(os.path.basename(m.group(1)) == name for m in INCLUDE_RE.finditer(read(path) or b'')) + + +def git(cwd: str, *args: str) -> str: + try: + r = subprocess.run( + ['git', '-c', 'core.quotePath=false', *args], + cwd=cwd, + capture_output=True, + text=True, + errors='surrogateescape', + ) + except OSError as e: + raise GitError(f'git: {e}') from e + if r.returncode != 0: + raise GitError(r.stderr.strip() or f'git {args[0]} failed') + return r.stdout + + +def unquote(name: str) -> str: + """Undoes git's C-style quoting of unusual file names.""" + if not name.startswith('"'): + return name + escapes = {'a': 7, 'b': 8, 't': 9, 'n': 10, 'v': 11, 'f': 12, 'r': 13} + raw, out, i = name[1:-1].encode(errors='surrogateescape'), bytearray(), 0 + while i < len(raw): + if raw[i] == ord('\\') and i + 1 < len(raw): + c = chr(raw[i + 1]) + if c in '01234567': + out.append(int(raw[i + 1 : i + 4], 8)) + i += 4 + else: + out.append(escapes.get(c, raw[i + 1])) + i += 2 + else: + out.append(raw[i]) + i += 1 + return out.decode(errors='surrogateescape') + + +def changed_lines(root: str, rev: str, paths: list[str]) -> dict[str, list[int]]: + """The lines of the files below paths that differ from rev, all of them for files git does not track. + One diff over all of them pairs up renames, and git's settings must not widen the hunks.""" + if rev.startswith('-'): + raise GitError(f'{rev}: not a revision') + try: + git(root, 'rev-parse', '--verify', rev + '^{commit}') + except GitError as e: + raise GitError(f'{rev}: {e}') from e + top = os.path.realpath(git(root, 'rev-parse', '--show-toplevel').strip()) + diff = git( + root, + 'diff', + '-U0', + '--inter-hunk-context=0', + '--no-color', + '--no-ext-diff', + '--no-textconv', + '--no-relative', + '--src-prefix=a/', + '--dst-prefix=b/', + '-M', + rev, + '--', + *paths, + ) + lines: dict[str, set[int]] = {} + current: Optional[set[int]] = None + header = False + for line in diff.split('\n'): + if line.startswith('diff '): + header, current = True, None + elif header and line.startswith('+++ '): + name = unquote(line[4:]) + current = None + if name.startswith('b/'): + current = lines.setdefault(os.path.realpath(os.path.join(top, name[2:])), set()) + elif line.startswith('@@'): + header = False + m = HUNK_RE.match(line) + if m and current is not None: + start = int(m.group(1)) + current.update(range(start, start + (int(m.group(2)) if m.group(2) is not None else 1))) + untracked = git(root, 'ls-files', '-z', '--full-name', '--others', '--exclude-standard', '--', *paths) + for name in untracked.split('\0'): + if name: + path = os.path.realpath(os.path.join(top, name)) + lines[path] = set(range(1, (read(path) or b'').count(b'\n') + 2)) + return {path: sorted(numbers) for path, numbers in lines.items() if numbers} + + +def diff_selection( + files: list[str], only: dict[str, list[int]], commands: dict[str, list[Command]] +) -> list[str]: + """The files --diff parses: the changed ones and those paired with a changed header. A changed header + that no compiled file of its own includes is checked from whichever file includes it, so then all.""" + paired = set() + for f in files: + header = twin_header(f) + if header and f in commands and includes(f, header): + paired.add(os.path.realpath(header)) + if any(path.endswith('.h') and path not in paired for path in only): + return files + return [f for f in files if os.path.realpath(f) in only or os.path.realpath(twin_header(f) or f) in only] + + +def fix_files(edits: list[tuple[str, Edit]]) -> None: + by_file: dict[str, list[Edit]] = {} + for path, edit in edits: + by_file.setdefault(path, []).append(edit) + for path, items in by_file.items(): + with open(path, 'rb') as f: + source = f.read() + for start, end, text in sorted(items, reverse=True): + source = source[:start] + text.encode() + source[end:] + with open(path, 'wb') as f: + f.write(source) + + +def default_jobs() -> int: + try: + return len(os.sched_getaffinity(0)) + except (AttributeError, OSError): + return os.cpu_count() or 1 + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__.split('\n\n')[0] if __doc__ else None) + parser.add_argument( + '-p', '--build-dir', default='build', help='build directory with compile_commands.json' + ) + parser.add_argument( + '--root', + help='source tree whose functions are checked, the git toplevel of the first PATH by default', + ) + parser.add_argument('-j', '--jobs', type=int, default=default_jobs(), help='parallel jobs') + parser.add_argument('--diff', metavar='REV', help='only report lines that differ from this git revision') + parser.add_argument('--fix', action='store_true', help='insert the missing comments in place') + parser.add_argument( + '--system', action='store_true', help='also check calls to functions declared outside the tree' + ) + parser.add_argument( + 'paths', nargs='+', metavar='PATH', help='C files, or directories to search for them' + ) + args = parser.parse_args() + if args.diff == '': + print('error: --diff needs a revision', file=sys.stderr) + return 2 + + library = setup_libclang() + major = libclang_major() + if major is not None and major < 19: + # Before 19 clang_getSpellingLocation() returns the file location, callee() needs the real one + print(f'error: libclang {major} is too old, the checker needs 19 or newer', file=sys.stderr) + return 2 + build_dir = os.path.realpath(args.build_dir) + files: list[str] = [] + for p in args.paths: + real = os.path.realpath(p) + if os.path.isdir(real): + found = glob.glob(os.path.join(glob.escape(real), '**', '*.c'), recursive=True) + files += [source_path(f) for f in sorted(found)] + elif os.path.isfile(real) and p.endswith('.c'): + files.append(source_path(p)) + else: + problem = 'not a .c file or a directory' if os.path.exists(real) else 'no such file or directory' + print(f'{p}: error: {problem}', file=sys.stderr) + return 2 + files = list( + dict.fromkeys(f for f in files if not f.endswith('.bpf.c')) + ) # BPF programs are not ours to style, and libclang cannot parse them + if not files: + print('nothing to check', file=sys.stderr) + return 2 + root = os.path.realpath(args.root) if args.root else source_root(files[0]) + if root is None: + print(f'cannot tell which source tree {files[0]} belongs to, use --root', file=sys.stderr) + return 2 + try: + commands = compile_commands(build_dir, set(files)) + except (OSError, ValueError, KeyError) as e: + print(f'error: cannot read {build_dir}/compile_commands.json: {e!r}', file=sys.stderr) + return 2 + only = None + if args.diff is not None: + dirs = [os.path.realpath(p) for p in args.paths if os.path.isdir(p)] + twins = [h for h in map(twin_header, files) if h and not any(h.startswith(d + os.sep) for d in dirs)] + try: + only = changed_lines(root, args.diff, [os.path.realpath(p) for p in args.paths] + twins) + except GitError as e: + print(f'error: {e}', file=sys.stderr) + return 2 + files = diff_selection(files, only, commands) + resources = resource_dir() + n_errors = 0 + checked = [] + items = [] + for f in files: + if f in commands: + checked.append(f) + items += [Item(f, root, build_dir, c, args.system, only, resources) for c in commands[f]] + elif args.diff is not None: + print( + f'{os.path.relpath(f, root)}: not in {build_dir}/compile_commands.json, skipping', + file=sys.stderr, + ) + else: + print( + f'{os.path.relpath(f, root)}: error: not in {build_dir}/compile_commands.json', + file=sys.stderr, + ) + n_errors += 1 + + # A header is checked from every file that includes it, so collect first and act on each argument once. + # A finding only counts as fixable when a translation unit that parsed cleanly produced it. + findings: dict[tuple[str, int], tuple[Finding, bool]] = {} + warnings: set[str] = set() + if items: + items.sort(key=lambda item: -os.path.getsize(item.path)) # the big ones first keep every worker busy + broken: Optional[BaseException] = None + try: + with ProcessPoolExecutor( + max(1, min(args.jobs, len(items))), initializer=setup_worker, initargs=(library,) + ) as pool: + try: + for future in [pool.submit(worker, item) for item in items]: + try: + result = future.result() + except BrokenExecutor as e: + broken = e + continue + for error in result.errors: + print(f'{os.path.relpath(result.path, root)}: error: {error}', file=sys.stderr) + n_errors += len(result.errors) + warnings.update(result.warnings) + for finding in result.findings: + key = (finding.file, finding.offset) + if key not in findings or (findings[key][1] and not result.errors): + findings[key] = (finding, bool(result.errors)) + except KeyboardInterrupt: + # Wait here, the with's shutdown resets cancel_futures before the executor acts on it + pool.shutdown(cancel_futures=True) + raise + except BrokenExecutor as e: + broken = e + if broken is not None: + # A worker that dies, libclang crashing on a file say, ends the run instead of hanging it. + print(f'error: {broken}', file=sys.stderr) + n_errors += 1 + + for w in sorted(warnings): + print(f'warning: {w}', file=sys.stderr) + + fixable = [x for x, dirty in findings.values() if x.fix is not None and not dirty] + if args.fix: + fix_files([(x.file, x.fix) for x in fixable if x.fix is not None]) + left = [x for x, dirty in findings.values() if x.fix is None or dirty] + else: + left = [x for x, _ in findings.values()] + for x in sorted(left, key=lambda x: (x.file, x.offset)): + prefix = f"{os.path.relpath(x.file, root)}:{x.line}:{x.column}: {x.callee}(): argument '{x.param}'" + if x.comment is None: + print(f'{prefix} passed as bare {x.literal}, comment it as /* {x.param}= */ {x.literal}') + elif x.fix is not None: + print(f"{prefix} is commented as '{x.comment}', comment it as /* {x.param}= */ {x.literal}") + else: + print(f"{prefix} is commented as '{x.comment}'") + + print( + (f'fixed {len(fixable)} argument(s), ' if args.fix else '') + + f'{len(left)} argument(s) without a matching comment in {len(checked)} file(s)' + + (f', {n_errors} error(s)' if n_errors else ''), + file=sys.stderr, + ) + return 2 if n_errors else 1 if left else 0 + + +if __name__ == '__main__': + sys.exit(main()) diff --git a/tools/meson.build b/tools/meson.build index e22ff11f983..da77368f644 100644 --- a/tools/meson.build +++ b/tools/meson.build @@ -1,6 +1,7 @@ # SPDX-License-Identifier: LGPL-2.1-or-later check_api_docs_sh = files('check-api-docs.sh') +check_argument_comments_py = files('check-argument-comments.py') check_coccinelle_sh = files('check-coccinelle.sh') check_efi_alignment_py = files('check-efi-alignment.py') check_help_sh = files('check-help.sh')