Git Mailing List Archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/12] more unused-parameter fixes / annotations
@ 2022-10-18  1:00 Jeff King
  2022-10-18  1:01 ` [PATCH 01/12] diffstat_consume(): assert non-zero length Jeff King
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: Jeff King @ 2022-10-18  1:00 UTC (permalink / raw)
  To: git

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 01/12] diffstat_consume(): assert non-zero length
  2022-10-18  1:00 [PATCH 0/12] more unused-parameter fixes / annotations Jeff King
@ 2022-10-18  1:01 ` Jeff King
  2022-10-18  1:02 ` [PATCH 02/12] submodule--helper: drop unused argc from module_list_compute() Jeff King
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Jeff King @ 2022-10-18  1:01 UTC (permalink / raw)
  To: git

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(+)

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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 02/12] submodule--helper: drop unused argc from module_list_compute()
  2022-10-18  1:00 [PATCH 0/12] more unused-parameter fixes / annotations Jeff King
  2022-10-18  1:01 ` [PATCH 01/12] diffstat_consume(): assert non-zero length Jeff King
@ 2022-10-18  1:02 ` Jeff King
  2022-10-18  1:04 ` [PATCH 03/12] update-index: drop unused argc from do_reupdate() Jeff King
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Jeff King @ 2022-10-18  1:02 UTC (permalink / raw)
  To: git

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(-)

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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 03/12] update-index: drop unused argc from do_reupdate()
  2022-10-18  1:00 [PATCH 0/12] more unused-parameter fixes / annotations Jeff King
  2022-10-18  1:01 ` [PATCH 01/12] diffstat_consume(): assert non-zero length Jeff King
  2022-10-18  1:02 ` [PATCH 02/12] submodule--helper: drop unused argc from module_list_compute() Jeff King
@ 2022-10-18  1:04 ` Jeff King
  2022-10-18  1:05 ` [PATCH 04/12] mark unused parameters in trivial compat functions Jeff King
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Jeff King @ 2022-10-18  1:04 UTC (permalink / raw)
  To: git

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(-)

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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 04/12] mark unused parameters in trivial compat functions
  2022-10-18  1:00 [PATCH 0/12] more unused-parameter fixes / annotations Jeff King
                   ` (2 preceding siblings ...)
  2022-10-18  1:04 ` [PATCH 03/12] update-index: drop unused argc from do_reupdate() Jeff King
@ 2022-10-18  1:05 ` Jeff King
  2022-10-18  1:05 ` [PATCH 05/12] object-file: mark unused parameters in hash_unknown functions Jeff King
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Jeff King @ 2022-10-18  1:05 UTC (permalink / raw)
  To: git

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(-)

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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 05/12] object-file: mark unused parameters in hash_unknown functions
  2022-10-18  1:00 [PATCH 0/12] more unused-parameter fixes / annotations Jeff King
                   ` (3 preceding siblings ...)
  2022-10-18  1:05 ` [PATCH 04/12] mark unused parameters in trivial compat functions Jeff King
@ 2022-10-18  1:05 ` Jeff King
  2022-10-18  1:05 ` [PATCH 06/12] string-list: mark unused callback parameters Jeff King
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Jeff King @ 2022-10-18  1:05 UTC (permalink / raw)
  To: git

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(-)

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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 06/12] string-list: mark unused callback parameters
  2022-10-18  1:00 [PATCH 0/12] more unused-parameter fixes / annotations Jeff King
                   ` (4 preceding siblings ...)
  2022-10-18  1:05 ` [PATCH 05/12] object-file: mark unused parameters in hash_unknown functions Jeff King
@ 2022-10-18  1:05 ` Jeff King
  2022-10-18  1:05 ` [PATCH 07/12] date: mark unused parameters in handler functions Jeff King
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Jeff King @ 2022-10-18  1:05 UTC (permalink / raw)
  To: git

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(-)

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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 07/12] date: mark unused parameters in handler functions
  2022-10-18  1:00 [PATCH 0/12] more unused-parameter fixes / annotations Jeff King
                   ` (5 preceding siblings ...)
  2022-10-18  1:05 ` [PATCH 06/12] string-list: mark unused callback parameters Jeff King
@ 2022-10-18  1:05 ` Jeff King
  2022-10-18  1:08 ` [PATCH 08/12] apply: mark unused parameters in handlers Jeff King
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Jeff King @ 2022-10-18  1:05 UTC (permalink / raw)
  To: git

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(-)

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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 08/12] apply: mark unused parameters in handlers
  2022-10-18  1:00 [PATCH 0/12] more unused-parameter fixes / annotations Jeff King
                   ` (6 preceding siblings ...)
  2022-10-18  1:05 ` [PATCH 07/12] date: mark unused parameters in handler functions Jeff King
@ 2022-10-18  1:08 ` Jeff King
  2022-10-18  1:08 ` [PATCH 09/12] apply: mark unused parameters in noop error/warning routine Jeff King
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Jeff King @ 2022-10-18  1:08 UTC (permalink / raw)
  To: git

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(-)

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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 09/12] apply: mark unused parameters in noop error/warning routine
  2022-10-18  1:00 [PATCH 0/12] more unused-parameter fixes / annotations Jeff King
                   ` (7 preceding siblings ...)
  2022-10-18  1:08 ` [PATCH 08/12] apply: mark unused parameters in handlers Jeff King
@ 2022-10-18  1:08 ` Jeff King
  2022-10-18  1:08 ` [PATCH 10/12] convert: mark unused parameter in null stream filter Jeff King
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Jeff King @ 2022-10-18  1:08 UTC (permalink / raw)
  To: git

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(-)

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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 10/12] convert: mark unused parameter in null stream filter
  2022-10-18  1:00 [PATCH 0/12] more unused-parameter fixes / annotations Jeff King
                   ` (8 preceding siblings ...)
  2022-10-18  1:08 ` [PATCH 09/12] apply: mark unused parameters in noop error/warning routine Jeff King
@ 2022-10-18  1:08 ` Jeff King
  2022-10-18  1:09 ` [PATCH 11/12] diffcore-pickaxe: mark unused parameters in pickaxe functions Jeff King
  2022-10-18  1:10 ` [PATCH 12/12] ll-merge: mark unused parameters in callbacks Jeff King
  11 siblings, 0 replies; 13+ messages in thread
From: Jeff King @ 2022-10-18  1:08 UTC (permalink / raw)
  To: git

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(-)

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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 11/12] diffcore-pickaxe: mark unused parameters in pickaxe functions
  2022-10-18  1:00 [PATCH 0/12] more unused-parameter fixes / annotations Jeff King
                   ` (9 preceding siblings ...)
  2022-10-18  1:08 ` [PATCH 10/12] convert: mark unused parameter in null stream filter Jeff King
@ 2022-10-18  1:09 ` Jeff King
  2022-10-18  1:10 ` [PATCH 12/12] ll-merge: mark unused parameters in callbacks Jeff King
  11 siblings, 0 replies; 13+ messages in thread
From: Jeff King @ 2022-10-18  1:09 UTC (permalink / raw)
  To: git

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(-)

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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 12/12] ll-merge: mark unused parameters in callbacks
  2022-10-18  1:00 [PATCH 0/12] more unused-parameter fixes / annotations Jeff King
                   ` (10 preceding siblings ...)
  2022-10-18  1:09 ` [PATCH 11/12] diffcore-pickaxe: mark unused parameters in pickaxe functions Jeff King
@ 2022-10-18  1:10 ` Jeff King
  11 siblings, 0 replies; 13+ messages in thread
From: Jeff King @ 2022-10-18  1:10 UTC (permalink / raw)
  To: git

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(-)

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2022-10-18  1:11 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-18  1:00 [PATCH 0/12] more unused-parameter fixes / annotations Jeff King
2022-10-18  1:01 ` [PATCH 01/12] diffstat_consume(): assert non-zero length Jeff King
2022-10-18  1:02 ` [PATCH 02/12] submodule--helper: drop unused argc from module_list_compute() Jeff King
2022-10-18  1:04 ` [PATCH 03/12] update-index: drop unused argc from do_reupdate() Jeff King
2022-10-18  1:05 ` [PATCH 04/12] mark unused parameters in trivial compat functions Jeff King
2022-10-18  1:05 ` [PATCH 05/12] object-file: mark unused parameters in hash_unknown functions Jeff King
2022-10-18  1:05 ` [PATCH 06/12] string-list: mark unused callback parameters Jeff King
2022-10-18  1:05 ` [PATCH 07/12] date: mark unused parameters in handler functions Jeff King
2022-10-18  1:08 ` [PATCH 08/12] apply: mark unused parameters in handlers Jeff King
2022-10-18  1:08 ` [PATCH 09/12] apply: mark unused parameters in noop error/warning routine Jeff King
2022-10-18  1:08 ` [PATCH 10/12] convert: mark unused parameter in null stream filter Jeff King
2022-10-18  1:09 ` [PATCH 11/12] diffcore-pickaxe: mark unused parameters in pickaxe functions Jeff King
2022-10-18  1:10 ` [PATCH 12/12] ll-merge: mark unused parameters in callbacks Jeff King

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).