threads / patch / 58640

patch, 12 partsmore unused-parameter fixes / annotations

Subject: [PATCH 0/12] more unused-parameter fixes / annotations

## tl;dr

13 messages between Oct 18, 2022 and Oct 18, 2022. Diffs are folded; open one to read it.

replies: 12people: 1as markdown or json

Jeff King· Oct 18, 2022, 01:00 UTC · lore

Here's another batch of UNUSED markings. I've floated the less-obvious ones to the top. Patches 4-12 are trivially correct in the sense that the compiler would tell us if we were wrong about it being unused. But I tried where appropriate to reason out why ignoring a parameter wasn't an unintentional bug.

  [01/12]: diffstat_consume(): assert non-zero length
  [02/12]: submodule--helper: drop unused argc from module_list_compute()
  [03/12]: update-index: drop unused argc from do_reupdate()
  [04/12]: mark unused parameters in trivial compat functions
  [05/12]: object-file: mark unused parameters in hash_unknown functions
  [06/12]: string-list: mark unused callback parameters
  [07/12]: date: mark unused parameters in handler functions
  [08/12]: apply: mark unused parameters in handlers
  [09/12]: apply: mark unused parameters in noop error/warning routine
  [10/12]: convert: mark unused parameter in null stream filter
  [11/12]: diffcore-pickaxe: mark unused parameters in pickaxe functions
  [12/12]: ll-merge: mark unused parameters in callbacks
 apply.c                     | 18 +++++++++---------
 archive.c                   |  2 +-
 builtin/gc.c                |  2 +-
 builtin/remote.c            |  2 +-
 builtin/submodule--helper.c | 18 +++++++++---------
 builtin/update-index.c      |  6 +++---
 compat/nonblock.c           |  2 +-
 convert.c                   |  4 ++--
 date.c                      |  6 +++---
 diff.c                      |  3 +++
 diffcore-pickaxe.c          |  4 ++--
 exec-cmd.c                  |  2 +-
 git-compat-util.h           | 16 ++++++++++------
 ll-merge.c                  | 18 +++++++++---------
 merge-ort.c                 |  2 +-
 object-file.c               | 15 ++++++++++-----
 reflog-walk.c               |  2 +-
 string-list.c               |  2 +-
 t/helper/test-path-utils.c  |  3 ++-
 19 files changed, 70 insertions(+), 57 deletions(-)
-Peff
Jeff King· Oct 18, 2022, 01:01 UTC · re: Jeff King · lore

[PATCH 01/12] diffstat_consume(): assert non-zero length

The callback interface for xdiff_emit_line_fn gives us a line/len pair, but diffstat_consume() never looks at "len". At first glance this seems like a bug that could cause us to read further than xdiff intends. But in practice, we read only the first character, and xdiff would never pass us an empty line.

Let's add a run-time assertion that this is true, which clarifies our assumption and silences -Wunused-parameter.

Signed-off-by: Jeff King <peff@peff.net>
---
 diff.c | 3 +++
 1 file changed, 3 insertions(+)
Show changes to diff.c +3 −0
diff --git a/diff.c b/diff.c
index 648f6717a5..bba888a34a 100644
--- a/diff.c
+++ b/diff.c
@@ -2488,6 +2488,9 @@ static int diffstat_consume(void *priv, char *line, unsigned long len)
 	struct diffstat_t *diffstat = priv;
 	struct diffstat_file *x = diffstat->files[diffstat->nr - 1];
 
+	if (!len)
+		BUG("xdiff fed us an empty line");
+
 	if (line[0] == '+')
 		x->added++;
 	else if (line[0] == '-')
-- 
2.38.0.371.g300879f34e
Jeff King· Oct 18, 2022, 01:02 UTC · re: Jeff King · lore

[PATCH 02/12] submodule--helper: drop unused argc from module_list_compute()

The module_list_compute() function takes an argc/argv pair, but never looks at argc. This is OK, as the NULL terminator in argv is sufficient for our purposes (we feed it to parse_pathspec(), which takes only the array, not a count).

Note that one of the callers _looks_ like it would be buggy, but isn't: we pass 0/NULL for argc/argv from module_foreach(), so finding the terminating NULL in that argv naively would segfault. However, parse_pathspec() is smart enough to interpret a bare NULL as an empty argv.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/submodule--helper.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)
Show changes to builtin/submodule--helper.c +9 −9
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 0b4acb442b..1e29a3d7dc 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -181,7 +181,7 @@ static void module_list_release(struct module_list *ml)
 	free(ml->entries);
 }
 
-static int module_list_compute(int argc, const char **argv,
+static int module_list_compute(const char **argv,
 			       const char *prefix,
 			       struct pathspec *pathspec,
 			       struct module_list *list)
@@ -405,7 +405,7 @@ static int module_foreach(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, module_foreach_options,
 			     git_submodule_helper_usage, 0);
 
-	if (module_list_compute(0, NULL, prefix, &pathspec, &list) < 0)
+	if (module_list_compute(NULL, prefix, &pathspec, &list) < 0)
 		goto cleanup;
 
 	info.argc = argc;
@@ -545,7 +545,7 @@ static int module_init(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, module_init_options,
 			     git_submodule_helper_usage, 0);
 
-	if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
+	if (module_list_compute(argv, prefix, &pathspec, &list) < 0)
 		goto cleanup;
 
 	/*
@@ -732,7 +732,7 @@ static int module_status(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, module_status_options,
 			     git_submodule_helper_usage, 0);
 
-	if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
+	if (module_list_compute(argv, prefix, &pathspec, &list) < 0)
 		goto cleanup;
 
 	info.prefix = prefix;
@@ -1326,7 +1326,7 @@ static int module_sync(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, module_sync_options,
 			     git_submodule_helper_usage, 0);
 
-	if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
+	if (module_list_compute(argv, prefix, &pathspec, &list) < 0)
 		goto cleanup;
 
 	info.prefix = prefix;
@@ -1479,7 +1479,7 @@ static int module_deinit(int argc, const char **argv, const char *prefix)
 	if (!argc && !all)
 		die(_("Use '--all' if you really want to deinitialize all submodules"));
 
-	if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
+	if (module_list_compute(argv, prefix, &pathspec, &list) < 0)
 		goto cleanup;
 
 	info.prefix = prefix;
@@ -2697,7 +2697,7 @@ static int module_update(int argc, const char **argv, const char *prefix)
 	if (opt.update_default)
 		opt.update_strategy.type = opt.update_default;
 
-	if (module_list_compute(argc, argv, prefix, &pathspec, &opt.list) < 0) {
+	if (module_list_compute(argv, prefix, &pathspec, &opt.list) < 0) {
 		ret = 1;
 		goto cleanup;
 	}
@@ -2709,7 +2709,7 @@ static int module_update(int argc, const char **argv, const char *prefix)
 		struct module_list list = MODULE_LIST_INIT;
 		struct init_cb info = INIT_CB_INIT;
 
-		if (module_list_compute(argc, argv, opt.prefix,
+		if (module_list_compute(argv, opt.prefix,
 					&pathspec2, &list) < 0) {
 			module_list_release(&list);
 			ret = 1;
@@ -2840,7 +2840,7 @@ static int absorb_git_dirs(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, embed_gitdir_options,
 			     git_submodule_helper_usage, 0);
 
-	if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
+	if (module_list_compute(argv, prefix, &pathspec, &list) < 0)
 		goto cleanup;
 
 	for (i = 0; i < list.nr; i++)
-- 
2.38.0.371.g300879f34e
Jeff King· Oct 18, 2022, 01:04 UTC · re: Jeff King · lore

[PATCH 03/12] update-index: drop unused argc from do_reupdate()

The parse-options callback for --again soaks up all remaining options by manipulating the parse_opt_ctx's argc and argv fields. Even though it has to look at both, the actual parsing happens via the do_reupdate() helper, which only looks at the argv half (by passing it along to parse_pathspec). So that helper doesn't need to see argc at all.

Note that the helper does look at "argv + 1" without confirming that argc is greater than 0. We know this is correct because it is skipping past the actual "--again" string, which will always be present. However, to make what's going on more obvious, let's move that "+1" into the caller, which has the matching "-1" when fixing up the ctx's argc/argv.

Signed-off-by: Jeff King <peff@peff.net>
---
Generate with -U5 so you can see the context I mentioned in
reupdate_callback().
 builtin/update-index.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
Show changes to builtin/update-index.c +3 −3
diff --git a/builtin/update-index.c b/builtin/update-index.c
index b62249905f..7b0c924d7d 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -730,23 +730,23 @@ static int do_unresolve(int ac, const char **av,
 		free(p);
 	}
 	return err;
 }
 
-static int do_reupdate(int ac, const char **av,
+static int do_reupdate(const char **paths,
 		       const char *prefix)
 {
 	/* Read HEAD and run update-index on paths that are
 	 * merged and already different between index and HEAD.
 	 */
 	int pos;
 	int has_head = 1;
 	struct pathspec pathspec;
 
 	parse_pathspec(&pathspec, 0,
 		       PATHSPEC_PREFER_CWD,
-		       prefix, av + 1);
+		       prefix, paths);
 
 	if (read_ref("HEAD", &head_oid))
 		/* If there is no HEAD, that means it is an initial
 		 * commit.  Update everything in the index.
 		 */
@@ -968,11 +968,11 @@ static enum parse_opt_result reupdate_callback(
 	BUG_ON_OPT_NEG(unset);
 	BUG_ON_OPT_ARG(arg);
 
 	/* consume remaining arguments. */
 	setup_work_tree();
-	*has_errors = do_reupdate(ctx->argc, ctx->argv, prefix);
+	*has_errors = do_reupdate(ctx->argv + 1, prefix);
 	if (*has_errors)
 		active_cache_changed = 0;
 
 	ctx->argv += ctx->argc - 1;
 	ctx->argc = 1;
-- 
2.38.0.371.g300879f34e
Jeff King· Oct 18, 2022, 01:05 UTC · re: Jeff King · lore

[PATCH 04/12] mark unused parameters in trivial compat functions

When a platform feature isn't available or in use, we sometimes conditionally compile empty or trivial functions to turn these into noops. We need to annotate their parameters so that -Wunused-parameters won't complain about them.

Note that there are many more of these in compat/mingw.h, but we'll leave them for now, as there's some trickery required to get the UNUSED macro available there.

Signed-off-by: Jeff King <peff@peff.net>
---
 compat/nonblock.c |  2 +-
 exec-cmd.c        |  2 +-
 git-compat-util.h | 16 ++++++++++------
 3 files changed, 12 insertions(+), 8 deletions(-)
Show changes to 3 files +12 −8

compat/nonblock.c, exec-cmd.c, git-compat-util.h

diff --git a/compat/nonblock.c b/compat/nonblock.c
index 9694ebdb1d..5b51195c32 100644
--- a/compat/nonblock.c
+++ b/compat/nonblock.c
@@ -41,7 +41,7 @@ int enable_pipe_nonblock(int fd)
 
 #else
 
-int enable_pipe_nonblock(int fd)
+int enable_pipe_nonblock(int fd UNUSED)
 {
 	errno = ENOSYS;
 	return -1;
diff --git a/exec-cmd.c b/exec-cmd.c
index eeb2ee52b8..0232bbc990 100644
--- a/exec-cmd.c
+++ b/exec-cmd.c
@@ -252,7 +252,7 @@ static const char *system_prefix(void)
  * This is called during initialization, but No work needs to be done here when
  * runtime prefix is not being used.
  */
-void git_resolve_executable_dir(const char *argv0)
+void git_resolve_executable_dir(const char *argv0 UNUSED)
 {
 }
 
diff --git a/git-compat-util.h b/git-compat-util.h
index ea53ea4a78..a76d0526f7 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -314,7 +314,9 @@ typedef unsigned long uintptr_t;
 #ifdef PRECOMPOSE_UNICODE
 #include "compat/precompose_utf8.h"
 #else
-static inline const char *precompose_argv_prefix(int argc, const char **argv, const char *prefix)
+static inline const char *precompose_argv_prefix(int argc UNUSED,
+						 const char **argv UNUSED,
+						 const char *prefix)
 {
 	return prefix;
 }
@@ -339,7 +341,9 @@ struct itimerval {
 #endif
 
 #ifdef NO_SETITIMER
-static inline int setitimer(int which, const struct itimerval *value, struct itimerval *newvalue) {
+static inline int setitimer(int which UNUSED,
+			    const struct itimerval *value UNUSED,
+			    struct itimerval *newvalue UNUSED) {
 	return 0; /* pretend success */
 }
 #endif
@@ -424,15 +428,15 @@ int lstat_cache_aware_rmdir(const char *path);
 #endif
 
 #ifndef has_dos_drive_prefix
-static inline int git_has_dos_drive_prefix(const char *path)
+static inline int git_has_dos_drive_prefix(const char *path UNUSED)
 {
 	return 0;
 }
 #define has_dos_drive_prefix git_has_dos_drive_prefix
 #endif
 
 #ifndef skip_dos_drive_prefix
-static inline int git_skip_dos_drive_prefix(char **path)
+static inline int git_skip_dos_drive_prefix(char **path UNUSED)
 {
 	return 0;
 }
@@ -1467,11 +1471,11 @@ int open_nofollow(const char *path, int flags);
 #endif
 
 #ifndef _POSIX_THREAD_SAFE_FUNCTIONS
-static inline void flockfile(FILE *fh)
+static inline void flockfile(FILE *fh UNUSED)
 {
 	; /* nothing */
 }
-static inline void funlockfile(FILE *fh)
+static inline void funlockfile(FILE *fh UNUSED)
 {
 	; /* nothing */
 }
-- 
2.38.0.371.g300879f34e
Jeff King· Oct 18, 2022, 01:05 UTC · re: Jeff King · lore

[PATCH 05/12] object-file: mark unused parameters in hash_unknown functions

The 0'th entry of our hash_algos array fills out the virtual methods with a series of functions which simply BUG(). This is the right thing to do, since the point is to catch use of an invalid algo parameter, but we need to annotate them to appease -Wunused-parameters.

Signed-off-by: Jeff King <peff@peff.net>
---
 object-file.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)
Show changes to object-file.c +10 −5
diff --git a/object-file.c b/object-file.c
index 5e30960234..957790098f 100644
--- a/object-file.c
+++ b/object-file.c
@@ -140,27 +140,32 @@ static void git_hash_sha256_final_oid(struct object_id *oid, git_hash_ctx *ctx)
 	oid->algo = GIT_HASH_SHA256;
 }
 
-static void git_hash_unknown_init(git_hash_ctx *ctx)
+static void git_hash_unknown_init(git_hash_ctx *ctx UNUSED)
 {
 	BUG("trying to init unknown hash");
 }
 
-static void git_hash_unknown_clone(git_hash_ctx *dst, const git_hash_ctx *src)
+static void git_hash_unknown_clone(git_hash_ctx *dst UNUSED,
+				   const git_hash_ctx *src UNUSED)
 {
 	BUG("trying to clone unknown hash");
 }
 
-static void git_hash_unknown_update(git_hash_ctx *ctx, const void *data, size_t len)
+static void git_hash_unknown_update(git_hash_ctx *ctx UNUSED,
+				    const void *data UNUSED,
+				    size_t len UNUSED)
 {
 	BUG("trying to update unknown hash");
 }
 
-static void git_hash_unknown_final(unsigned char *hash, git_hash_ctx *ctx)
+static void git_hash_unknown_final(unsigned char *hash UNUSED,
+				   git_hash_ctx *ctx UNUSED)
 {
 	BUG("trying to finalize unknown hash");
 }
 
-static void git_hash_unknown_final_oid(struct object_id *oid, git_hash_ctx *ctx)
+static void git_hash_unknown_final_oid(struct object_id *oid UNUSED,
+				       git_hash_ctx *ctx UNUSED)
 {
 	BUG("trying to finalize unknown hash");
 }
-- 
2.38.0.371.g300879f34e
Jeff King· Oct 18, 2022, 01:05 UTC · re: Jeff King · lore

[PATCH 06/12] string-list: mark unused callback parameters

String-lists may be used with callbacks for clearing or iteration. These callbacks need to conform to a particular interface, even though not every callback needs all of its parameters. Mark the unused ones to make -Wunused-parameter happy.

Signed-off-by: Jeff King <peff@peff.net>
---
 archive.c                  | 2 +-
 builtin/gc.c               | 2 +-
 builtin/remote.c           | 2 +-
 merge-ort.c                | 2 +-
 reflog-walk.c              | 2 +-
 string-list.c              | 2 +-
 t/helper/test-path-utils.c | 3 ++-
 7 files changed, 8 insertions(+), 7 deletions(-)
Show changes to 7 files +8 −7

archive.c, builtin/gc.c, builtin/remote.c, merge-ort.c, reflog-walk.c, string-list.c, t/helper/test-path-utils.c

diff --git a/archive.c b/archive.c
index 61a79e4a22..16f85e5475 100644
--- a/archive.c
+++ b/archive.c
@@ -500,7 +500,7 @@ static void parse_treeish_arg(const char **argv,
 	ar_args->time = archive_time;
 }
 
-static void extra_file_info_clear(void *util, const char *str)
+static void extra_file_info_clear(void *util, const char *str UNUSED)
 {
 	struct extra_file_info *info = util;
 	free(info->base);
diff --git a/builtin/gc.c b/builtin/gc.c
index 243ee85d28..24ea85c7af 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -322,7 +322,7 @@ static uint64_t estimate_repack_memory(struct packed_git *pack)
 	return os_cache + heap;
 }
 
-static int keep_one_pack(struct string_list_item *item, void *data)
+static int keep_one_pack(struct string_list_item *item, void *data UNUSED)
 {
 	strvec_pushf(&repack, "--keep-pack=%s", basename(item->string));
 	return 0;
diff --git a/builtin/remote.c b/builtin/remote.c
index 910f7b9316..93285fc06e 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -942,7 +942,7 @@ static int rm(int argc, const char **argv, const char *prefix)
 	return result;
 }
 
-static void clear_push_info(void *util, const char *string)
+static void clear_push_info(void *util, const char *string UNUSED)
 {
 	struct push_info *info = util;
 	free(info->dest);
diff --git a/merge-ort.c b/merge-ort.c
index e5f41cce48..7e83ebfaa9 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -397,7 +397,7 @@ struct conflicted_submodule_item {
 	int flag;
 };
 
-static void conflicted_submodule_item_free(void *util, const char *str)
+static void conflicted_submodule_item_free(void *util, const char *str UNUSED)
 {
 	struct conflicted_submodule_item *item = util;
 
diff --git a/reflog-walk.c b/reflog-walk.c
index 7aa6595a51..8a4d8fa3bd 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -55,7 +55,7 @@ static void free_complete_reflog(struct complete_reflogs *array)
 	free(array);
 }
 
-static void complete_reflogs_clear(void *util, const char *str)
+static void complete_reflogs_clear(void *util, const char *str UNUSED)
 {
 	struct complete_reflogs *array = util;
 	free_complete_reflog(array);
diff --git a/string-list.c b/string-list.c
index 549fc416d6..42bacaec55 100644
--- a/string-list.c
+++ b/string-list.c
@@ -156,7 +156,7 @@ void filter_string_list(struct string_list *list, int free_util,
 	list->nr = dst;
 }
 
-static int item_is_not_empty(struct string_list_item *item, void *unused)
+static int item_is_not_empty(struct string_list_item *item, void *data UNUSED)
 {
 	return *item->string != '\0';
 }
diff --git a/t/helper/test-path-utils.c b/t/helper/test-path-utils.c
index d20e1b7a18..f69709d674 100644
--- a/t/helper/test-path-utils.c
+++ b/t/helper/test-path-utils.c
@@ -8,7 +8,8 @@
  * GIT_CEILING_DIRECTORIES.  If the path is unusable for some reason,
  * die with an explanation.
  */
-static int normalize_ceiling_entry(struct string_list_item *item, void *unused)
+static int normalize_ceiling_entry(struct string_list_item *item,
+				   void *data UNUSED)
 {
 	char *ceil = item->string;
 
-- 
2.38.0.371.g300879f34e
Jeff King· Oct 18, 2022, 01:05 UTC · re: Jeff King · lore

[PATCH 07/12] date: mark unused parameters in handler functions

When parsing approxidates, we use a table to map special strings (like "noon") to functions which handle them. Not all functions need the "now" parameter, as they are not relative (e.g., "yesterday" does, but "pm" does not). Let's annotate those to make -Wunused-parameter happy.

Signed-off-by: Jeff King <peff@peff.net>
---
 date.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
Show changes to date.c +3 −3
diff --git a/date.c b/date.c
index 68a260c214..53bd6a7932 100644
--- a/date.c
+++ b/date.c
@@ -1101,7 +1101,7 @@ static void date_tea(struct tm *tm, struct tm *now, int *num)
 	date_time(tm, now, 17);
 }
 
-static void date_pm(struct tm *tm, struct tm *now, int *num)
+static void date_pm(struct tm *tm, struct tm *now UNUSED, int *num)
 {
 	int hour, n = *num;
 	*num = 0;
@@ -1115,7 +1115,7 @@ static void date_pm(struct tm *tm, struct tm *now, int *num)
 	tm->tm_hour = (hour % 12) + 12;
 }
 
-static void date_am(struct tm *tm, struct tm *now, int *num)
+static void date_am(struct tm *tm, struct tm *now UNUSED, int *num)
 {
 	int hour, n = *num;
 	*num = 0;
@@ -1129,7 +1129,7 @@ static void date_am(struct tm *tm, struct tm *now, int *num)
 	tm->tm_hour = (hour % 12);
 }
 
-static void date_never(struct tm *tm, struct tm *now, int *num)
+static void date_never(struct tm *tm, struct tm *now UNUSED, int *num)
 {
 	time_t n = 0;
 	localtime_r(&n, tm);
-- 
2.38.0.371.g300879f34e
Jeff King· Oct 18, 2022, 01:08 UTC · re: Jeff King · lore

[PATCH 08/12] apply: mark unused parameters in handlers

In parse_git_diff_header(), we have a table-driven parser that maps strings to handler functions. Not all handlers need all of the parameters; let's mark the unused ones to appease -Wunused-parameter.

Signed-off-by: Jeff King <peff@peff.net>
---
 apply.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
Show changes to apply.c +8 −8
diff --git a/apply.c b/apply.c
index 2b7cd930ef..fa9a02771c 100644
--- a/apply.c
+++ b/apply.c
@@ -892,9 +892,9 @@ static int parse_traditional_patch(struct apply_state *state,
 	return 0;
 }
 
-static int gitdiff_hdrend(struct gitdiff_data *state,
-			  const char *line,
-			  struct patch *patch)
+static int gitdiff_hdrend(struct gitdiff_data *state UNUSED,
+			  const char *line UNUSED,
+			  struct patch *patch UNUSED)
 {
 	return 1;
 }
@@ -1044,7 +1044,7 @@ static int gitdiff_renamedst(struct gitdiff_data *state,
 	return 0;
 }
 
-static int gitdiff_similarity(struct gitdiff_data *state,
+static int gitdiff_similarity(struct gitdiff_data *state UNUSED,
 			      const char *line,
 			      struct patch *patch)
 {
@@ -1054,7 +1054,7 @@ static int gitdiff_similarity(struct gitdiff_data *state,
 	return 0;
 }
 
-static int gitdiff_dissimilarity(struct gitdiff_data *state,
+static int gitdiff_dissimilarity(struct gitdiff_data *state UNUSED,
 				 const char *line,
 				 struct patch *patch)
 {
@@ -1104,9 +1104,9 @@ static int gitdiff_index(struct gitdiff_data *state,
  * This is normal for a diff that doesn't change anything: we'll fall through
  * into the next diff. Tell the parser to break out.
  */
-static int gitdiff_unrecognized(struct gitdiff_data *state,
-				const char *line,
-				struct patch *patch)
+static int gitdiff_unrecognized(struct gitdiff_data *state UNUSED,
+				const char *line UNUSED,
+				struct patch *patch UNUSED)
 {
 	return 1;
 }
-- 
2.38.0.371.g300879f34e
Jeff King· Oct 18, 2022, 01:08 UTC · re: Jeff King · lore

[PATCH 09/12] apply: mark unused parameters in noop error/warning routine

We squelch error/warning output by passing a noop handler to set_error_routine(). We need to tell the compiler that this is intended so that it doesn't trigger -Wunused-parameter.

Signed-off-by: Jeff King <peff@peff.net>
---
 apply.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to apply.c +1 −1
diff --git a/apply.c b/apply.c
index fa9a02771c..6b4dbe0c88 100644
--- a/apply.c
+++ b/apply.c
@@ -125,7 +125,7 @@ void clear_apply_state(struct apply_state *state)
 	/* &state->fn_table is cleared at the end of apply_patch() */
 }
 
-static void mute_routine(const char *msg, va_list params)
+static void mute_routine(const char *msg UNUSED, va_list params UNUSED)
 {
 	/* do nothing */
 }
-- 
2.38.0.371.g300879f34e
Jeff King· Oct 18, 2022, 01:09 UTC · re: Jeff King · lore

[PATCH 11/12] diffcore-pickaxe: mark unused parameters in pickaxe functions

We have a virtual pickaxe_fn for handling -G versus -S pickaxe options. They need to take the same set of parameters, but of course they care about different ones (e.g., a regex -G will never use a kwset).

Mark the unused ones to appease -Wunused-parameter.
Signed-off-by: Jeff King <peff@peff.net>
---
 diffcore-pickaxe.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to diffcore-pickaxe.c +2 −2
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index c88e50c632..03fcbcb40b 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -38,7 +38,7 @@ static int diffgrep_consume(void *priv, char *line, unsigned long len)
 
 static int diff_grep(mmfile_t *one, mmfile_t *two,
 		     struct diff_options *o,
-		     regex_t *regexp, kwset_t kws)
+		     regex_t *regexp, kwset_t kws UNUSED)
 {
 	struct diffgrep_cb ecbdata;
 	xpparam_t xpp;
@@ -114,7 +114,7 @@ static unsigned int contains(mmfile_t *mf, regex_t *regexp, kwset_t kws,
 }
 
 static int has_changes(mmfile_t *one, mmfile_t *two,
-		       struct diff_options *o,
+		       struct diff_options *o UNUSED,
 		       regex_t *regexp, kwset_t kws)
 {
 	unsigned int c1 = one ? contains(one, regexp, kws, 0) : 0;
-- 
2.38.0.371.g300879f34e
Jeff King· Oct 18, 2022, 01:08 UTC · re: Jeff King · lore

[PATCH 10/12] convert: mark unused parameter in null stream filter

The null stream filter unsurprisingly does not look at its "filter" argument, since it just eats bytes. But we can't drop it, since it has to conform to the same virtual interface that real filters do. Mark the unused parameter to appease -Wunused-parameter.

Signed-off-by: Jeff King <peff@peff.net>
---
 convert.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to convert.c +2 −2
diff --git a/convert.c b/convert.c
index 95e6a5244f..9b67649032 100644
--- a/convert.c
+++ b/convert.c
@@ -1549,7 +1549,7 @@ struct stream_filter {
 	struct stream_filter_vtbl *vtbl;
 };
 
-static int null_filter_fn(struct stream_filter *filter,
+static int null_filter_fn(struct stream_filter *filter UNUSED,
 			  const char *input, size_t *isize_p,
 			  char *output, size_t *osize_p)
 {
@@ -1568,7 +1568,7 @@ static int null_filter_fn(struct stream_filter *filter,
 	return 0;
 }
 
-static void null_free_fn(struct stream_filter *filter)
+static void null_free_fn(struct stream_filter *filter UNUSED)
 {
 	; /* nothing -- null instances are shared */
 }
-- 
2.38.0.371.g300879f34e
Jeff King· Oct 18, 2022, 01:10 UTC · re: Jeff King · lore

[PATCH 12/12] ll-merge: mark unused parameters in callbacks

We have a generic ll_merge_fn, but not every implementation needs every parameter. In particular, neither binary nor ext merges care about names (since they do not generate conflict markers), and most do not need to look at the ll_merge_driver itself.

Ironically, neither ll_xdl_merge() nor ll_union_merge() needs to have their driver parameter annotated (even though both are named drv_unused!). This is because they may fall back to calling ll_binary_merge() directly. And even though that function won't look at it, we still pass it along, and hence it is "used" in the caller.

We could get away with passing NULL, but that's likely more confusing and brittle than just passing along our own driver. And we have to keep the driver parameter in all callbacks, since ll_ext_merge() uses it.

Signed-off-by: Jeff King <peff@peff.net>
---
 ll-merge.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)
Show changes to ll-merge.c +9 −9
diff --git a/ll-merge.c b/ll-merge.c
index 8955d7e1f6..a8e2db9336 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -49,14 +49,14 @@ void reset_merge_attributes(void)
 /*
  * Built-in low-levels
  */
-static enum ll_merge_result ll_binary_merge(const struct ll_merge_driver *drv_unused,
+static enum ll_merge_result ll_binary_merge(const struct ll_merge_driver *drv UNUSED,
 			   mmbuffer_t *result,
-			   const char *path,
-			   mmfile_t *orig, const char *orig_name,
-			   mmfile_t *src1, const char *name1,
-			   mmfile_t *src2, const char *name2,
+			   const char *path UNUSED,
+			   mmfile_t *orig, const char *orig_name UNUSED,
+			   mmfile_t *src1, const char *name1 UNUSED,
+			   mmfile_t *src2, const char *name2 UNUSED,
 			   const struct ll_merge_options *opts,
-			   int marker_size)
+			   int marker_size UNUSED)
 {
 	enum ll_merge_result ret;
 	mmfile_t *stolen;
@@ -183,9 +183,9 @@ static void create_temp(mmfile_t *src, char *path, size_t len)
 static enum ll_merge_result ll_ext_merge(const struct ll_merge_driver *fn,
 			mmbuffer_t *result,
 			const char *path,
-			mmfile_t *orig, const char *orig_name,
-			mmfile_t *src1, const char *name1,
-			mmfile_t *src2, const char *name2,
+			mmfile_t *orig, const char *orig_name UNUSED,
+			mmfile_t *src1, const char *name1 UNUSED,
+			mmfile_t *src2, const char *name2 UNUSED,
 			const struct ll_merge_options *opts,
 			int marker_size)
 {
-- 
2.38.0.371.g300879f34e

← back to recent threads