cppcheck: Work across translation units

And while we're here, remove some suppressions that do nothing right
now, and document why we have the remaining ones.

There are also a couple of ones we can do inline, or do what it
suggests, so just do that.

Author: cdown

Makefile

@@ -55,16 +55,16 @@ clean:
 
 clang_supports_unsafe_buffer_usage := $(shell clang -x c -c /dev/null -o /dev/null -Werror -Wunsafe-buffer-usage > /dev/null 2>&1; echo $$?)
 ifeq ($(clang_supports_unsafe_buffer_usage),0)
-    extra_clang_flags := -Wno-unsafe-buffer-usage
+    extra_clang_flags := -Wno-unsafe-buffer-usage -Wno-missing-include-dirs
 else
-    extra_clang_flags :=
+    extra_clang_flags := -Wno-missing-include-dirs
 endif
 
 c_analyse_targets := $(c_files:%=%-analyse)
 h_analyse_targets := $(h_files:%=%-analyse)
 
 analyse: CFLAGS+=$(debug_cflags)
-analyse: $(c_analyse_targets) $(h_analyse_targets)
+analyse: cppcheck $(c_analyse_targets) $(h_analyse_targets)
 
 $(c_analyse_targets): %-analyse:
 	# -W options here are not clang compatible, so out of generic CFLAGS
@@ -90,20 +90,39 @@ $(h_analyse_targets): %-analyse:
 	$(MAKE) $*-shared-analyse
 
 %-shared-analyse: %
-	# cppcheck is a bit dim about unused functions/variables, leave that to
-	# clang/GCC
-	cppcheck $< --std=c99 --quiet --inline-suppr --force \
-		--enable=all --suppress=missingIncludeSystem \
-		--suppress=unusedFunction --suppress=unmatchedSuppression \
-		--suppress=unreadVariable \
-		--suppress=checkersReport \
-		--suppress=normalCheckLevelMaxBranches \
-		--suppress=unusedStructMember \
-		--max-ctu-depth=32 --error-exitcode=1
 	# clang-analyzer-unix.Malloc does not understand _drop_()
 	clang-tidy $< --quiet -checks=-clang-analyzer-unix.Malloc -- -std=gnu99
 	clang-format --dry-run --Werror $<
 
+# --suppress=missingIncludeSystem:
+#
+# Without this there's a bunch of noise from cppcheck from the system headers
+# themselves.
+#
+# --suppress=unusedFunction:
+#
+# cppcheck does not understand _drop_ and marks those as unused. This is
+# already well checked by Clang/GCC, just leave it to them.
+#
+# --suppress=unmatchedSuppression:
+#
+# We run both locally and on CI, so there may be some suppressions that
+# depending on version do not match in one version but do on another.
+#
+# --suppress=unusedStructMember
+#
+# Structs are used across translation units and cppcheck gets this wrong.
+cppcheck: $(c_files) $(h_files)
+	cppcheck $(c_files) $(h_files) --std=c99 --quiet --inline-suppr --force \
+		--enable=all \
+		--suppress=missingIncludeSystem \
+		--suppress=unusedFunction \
+		--suppress=unmatchedSuppression \
+		--suppress=checkersReport \
+		--suppress=unusedStructMember \
+		--check-level=exhaustive \
+		--max-ctu-depth=10 --error-exitcode=1
+
 tests: tests/test_store
 	tests/test_store
 
@@ -113,4 +132,5 @@ integration_tests:
 tests/test_store: tests/test_store.c src/store.o src/util.o
 	$(CC) $(CFLAGS) $(CPPFLAGS) -I./src -o $@ $^ $(LDLIBS)
 
-.PHONY: all debug install uninstall clean analyse tests integration_tests
+.PHONY: all debug install uninstall clean analyse tests integration_tests \
+	cppcheck

src/clipserve.c

@@ -147,6 +147,8 @@ static void _nonnull_ serve_clipboard(uint64_t hash,
                                        .target = req->target,
                                        .property = req->property};
 
+                // cppcheck-suppress constVariablePointer
+                // _drop_(XFree) passes this to XFree, so keep it non-const.
                 _drop_(XFree) char *window_title =
                     get_window_title(dpy, req->requestor);
                 dbg("Servicing request to window '%s' (0x%lX) for clip " PRI_HASH

src/config.c

@@ -92,7 +92,7 @@ static int resolve_path(const char *input, void *output) {
  * the config file to the type we expect for `struct Config`.
  */
 
-int convert_bool(const char *str, void *output) {
+static int convert_bool(const char *str, void *output) {
     const char *const truthy[] = {"1", "y", "yes", "true", "on"};
     const char *const falsy[] = {"0", "n", "no", "false", "off"};
 
@@ -113,7 +113,7 @@ int convert_bool(const char *str, void *output) {
     return -EINVAL;
 }
 
-int convert_positive_int(const char *str, void *output) {
+static int convert_positive_int(const char *str, void *output) {
     char *end;
     long val = strtol(str, &end, 10);
     if (*end != '\0' || end == str || val < 0 || val > INT_MAX) {
@@ -123,7 +123,7 @@ int convert_positive_int(const char *str, void *output) {
     return 0;
 }
 
-int convert_ignore_window(const char *str, void *output) {
+static int convert_ignore_window(const char *str, void *output) {
     struct ignore_window *iw = output;
     iw->set = (bool)str;
     if (!iw->set) {
@@ -326,7 +326,7 @@ static int config_apply_default_values(struct config_entry entries[],
  * This is generally not expected to be called by applications -- call
  * config_setup() instead, which provides the right file for you.
  */
-int config_setup_internal(FILE *file, struct config *cfg) {
+static int config_setup_internal(FILE *file, struct config *cfg) {
     struct config_entry entries[] = {
         {"max_clips", "CM_MAX_CLIPS", &cfg->max_clips, convert_positive_int,
          "1000", 0},

src/config.h

@@ -96,10 +96,6 @@ selection_atom_to_selection_type(Atom atom, const struct cm_selections *sels);
 enum selection_type _nonnull_
 storage_atom_to_selection_type(Atom atom, const struct cm_selections *sels);
 
-int convert_bool(const char *str, void *output);
-int convert_positive_int(const char *str, void *output);
-int convert_ignore_window(const char *str, void *output);
-int config_setup_internal(FILE *file, struct config *cfg);
 void config_free(struct config *cfg);
 DEFINE_DROP_FUNC_PTR(struct config, config_free)
 

src/store.c

@@ -478,7 +478,7 @@ cs_content_add(struct clip_store *cs, uint64_t hash, const char *content,
  *
  * @content: The content to unmap
  */
-int cs_content_unmap(struct cs_content *content) {
+static int cs_content_unmap(struct cs_content *content) {
     if (content && content->data) {
         close(content->fd);
         if (munmap(content->data, (size_t)content->size)) {

src/store.h

@@ -161,7 +161,6 @@ void _nonnull_ drop_cs_unref(struct ref_guard *guard);
 int _must_use_ _nonnull_ cs_destroy(struct clip_store *cs);
 int _must_use_ _nonnull_ cs_init(struct clip_store *cs, int snip_fd,
                                  int content_dir_fd);
-int _must_use_ cs_content_unmap(struct cs_content *content);
 void drop_cs_content_unmap(struct cs_content *content);
 void drop_cs_destroy(struct clip_store *cs);
 int _must_use_ _nonnull_ cs_content_get(struct clip_store *cs, uint64_t hash,