From 1d9e9c2ed6f0cb3bf168c0d602ae0a289ee27093 Mon Sep 17 00:00:00 2001 From: Kevin Svetlitski Date: Thu, 6 Jul 2023 16:27:56 -0700 Subject: [PATCH] Fix inconsistent parameter names between definition/declaration pairs For the sake of consistency, function definitions and their corresponding declarations should use the same names for parameters. I've enabled this check in static analysis to prevent this issue from occurring again in the future. --- include/jemalloc/internal/arena_externs.h | 4 ++-- include/jemalloc/internal/bin.h | 2 +- include/jemalloc/internal/hook.h | 2 +- include/jemalloc/internal/hpdata.h | 2 +- include/jemalloc/internal/tcache_externs.h | 14 +++++++------- include/jemalloc/internal/tsd.h | 2 +- scripts/run_static_analysis.sh | 15 ++++++++++++--- 7 files changed, 25 insertions(+), 16 deletions(-) diff --git a/include/jemalloc/internal/arena_externs.h b/include/jemalloc/internal/arena_externs.h index 8e323639..22d7fff7 100644 --- a/include/jemalloc/internal/arena_externs.h +++ b/include/jemalloc/internal/arena_externs.h @@ -47,9 +47,9 @@ edata_t *arena_extent_alloc_large(tsdn_t *tsdn, arena_t *arena, void arena_extent_dalloc_large_prep(tsdn_t *tsdn, arena_t *arena, edata_t *edata); void arena_extent_ralloc_large_shrink(tsdn_t *tsdn, arena_t *arena, - edata_t *edata, size_t oldsize); + edata_t *edata, size_t oldusize); void arena_extent_ralloc_large_expand(tsdn_t *tsdn, arena_t *arena, - edata_t *edata, size_t oldsize); + edata_t *edata, size_t oldusize); bool arena_decay_ms_set(tsdn_t *tsdn, arena_t *arena, extent_state_t state, ssize_t decay_ms); ssize_t arena_decay_ms_get(arena_t *arena, extent_state_t state); diff --git a/include/jemalloc/internal/bin.h b/include/jemalloc/internal/bin.h index 63f97395..027af088 100644 --- a/include/jemalloc/internal/bin.h +++ b/include/jemalloc/internal/bin.h @@ -48,7 +48,7 @@ struct bins_s { bin_t *bin_shards; }; -void bin_shard_sizes_boot(unsigned bin_shards[SC_NBINS]); +void bin_shard_sizes_boot(unsigned bin_shard_sizes[SC_NBINS]); bool bin_update_shard_size(unsigned bin_shards[SC_NBINS], size_t start_size, size_t end_size, size_t nshards); diff --git a/include/jemalloc/internal/hook.h b/include/jemalloc/internal/hook.h index 41157207..27f94841 100644 --- a/include/jemalloc/internal/hook.h +++ b/include/jemalloc/internal/hook.h @@ -146,7 +146,7 @@ struct hook_ralloc_args_s { */ bool hook_boot(void); -void *hook_install(tsdn_t *tsdn, hooks_t *hooks); +void *hook_install(tsdn_t *tsdn, hooks_t *to_install); /* Uninstalls the hook with the handle previously returned from hook_install. */ void hook_remove(tsdn_t *tsdn, void *opaque); diff --git a/include/jemalloc/internal/hpdata.h b/include/jemalloc/internal/hpdata.h index 5bf7aae8..36918258 100644 --- a/include/jemalloc/internal/hpdata.h +++ b/include/jemalloc/internal/hpdata.h @@ -359,7 +359,7 @@ void hpdata_init(hpdata_t *hpdata, void *addr, uint64_t age); * offset within that allocation. */ void *hpdata_reserve_alloc(hpdata_t *hpdata, size_t sz); -void hpdata_unreserve(hpdata_t *hpdata, void *begin, size_t sz); +void hpdata_unreserve(hpdata_t *hpdata, void *addr, size_t sz); /* * The hpdata_purge_prepare_t allows grabbing the metadata required to purge diff --git a/include/jemalloc/internal/tcache_externs.h b/include/jemalloc/internal/tcache_externs.h index a2ab7101..37f61646 100644 --- a/include/jemalloc/internal/tcache_externs.h +++ b/include/jemalloc/internal/tcache_externs.h @@ -36,14 +36,14 @@ extern tcaches_t *tcaches; size_t tcache_salloc(tsdn_t *tsdn, const void *ptr); void *tcache_alloc_small_hard(tsdn_t *tsdn, arena_t *arena, tcache_t *tcache, - cache_bin_t *tbin, szind_t binind, bool *tcache_success); + cache_bin_t *cache_bin, szind_t binind, bool *tcache_success); -void tcache_bin_flush_small(tsd_t *tsd, tcache_t *tcache, cache_bin_t *tbin, - szind_t binind, unsigned rem); -void tcache_bin_flush_large(tsd_t *tsd, tcache_t *tcache, cache_bin_t *tbin, - szind_t binind, unsigned rem); -void tcache_bin_flush_stashed(tsd_t *tsd, tcache_t *tcache, cache_bin_t *bin, - szind_t binind, bool is_small); +void tcache_bin_flush_small(tsd_t *tsd, tcache_t *tcache, + cache_bin_t *cache_bin, szind_t binind, unsigned rem); +void tcache_bin_flush_large(tsd_t *tsd, tcache_t *tcache, + cache_bin_t *cache_bin, szind_t binind, unsigned rem); +void tcache_bin_flush_stashed(tsd_t *tsd, tcache_t *tcache, + cache_bin_t *cache_bin, szind_t binind, bool is_small); void tcache_arena_reassociate(tsdn_t *tsdn, tcache_slow_t *tcache_slow, tcache_t *tcache, arena_t *arena); tcache_t *tcache_create_explicit(tsd_t *tsd); diff --git a/include/jemalloc/internal/tsd.h b/include/jemalloc/internal/tsd.h index 3dd52247..c2f432e8 100644 --- a/include/jemalloc/internal/tsd.h +++ b/include/jemalloc/internal/tsd.h @@ -164,7 +164,7 @@ void malloc_tsd_dalloc(void *wrapper); tsd_t *malloc_tsd_boot0(void); void malloc_tsd_boot1(void); void tsd_cleanup(void *arg); -tsd_t *tsd_fetch_slow(tsd_t *tsd, bool internal); +tsd_t *tsd_fetch_slow(tsd_t *tsd, bool minimal); void tsd_state_set(tsd_t *tsd, uint8_t new_state); void tsd_slow_update(tsd_t *tsd); void tsd_prefork(tsd_t *tsd); diff --git a/scripts/run_static_analysis.sh b/scripts/run_static_analysis.sh index 68ceae55..4994fe64 100755 --- a/scripts/run_static_analysis.sh +++ b/scripts/run_static_analysis.sh @@ -27,7 +27,7 @@ EXTRA_CFLAGS='-Wstrict-prototypes' EXTRA_CXXFLAGS='-Wstrict-prototypes' ./autoge # otherwise you'll get tons of warnings for things # that are already covered by `assert`s. -bear -- make -s -j $(nproc) +bear -- make -s -j "$(nproc)" # We end up with lots of duplicate entries in the compilation database, one for # each output file type (e.g. .o, .d, .sym, etc.). There must be exactly one # entry for each file in the compilation database in order for @@ -35,9 +35,18 @@ bear -- make -s -j $(nproc) jq '[.[] | select(.output | test("/[^./]*\\.o$"))]' compile_commands.json > compile_commands.json.tmp mv compile_commands.json.tmp compile_commands.json -CC_ANALYZERS_FROM_PATH=1 CodeChecker analyze compile_commands.json --jobs $(nproc) \ +# CodeChecker has a bug where it freaks out if you supply the skipfile via process substitution, +# so we resort to manually creating a temporary file +skipfile=$(mktemp) +# The single-quotes are deliberate here, you want `$skipfile` to be evaluated upon exit +trap 'rm -f $skipfile' EXIT +echo '-**/stdlib.h' > "$skipfile" +CC_ANALYZERS_FROM_PATH=1 CodeChecker analyze compile_commands.json --jobs "$(nproc)" \ --ctu --compile-uniqueing strict --output static_analysis_raw_results \ - --analyzers clang-tidy clangsa + --analyzers clangsa clang-tidy --skip "$skipfile" \ + --enable readability-inconsistent-declaration-parameter-name + # `--enable` is additive, the vast majority of the checks we want are + # enabled by default. html_output_dir="${1:-static_analysis_results}" result=${2:-/dev/null}