[PATCH 0/15] marking some more unused parameters

[PATCH 0/15] marking some more unused parameters

[Jeff King]

These are from my pile of -Wunused-parameter fixes. I built this on top
of the "dropping" series I just posted at:

  https://lore.kernel.org/git/20240817072621.GA1535666@coredump.intra.peff.net/

but it should also be fine to apply it in parallel. This should
hopefully be pretty quick to review, as it's only marking things UNUSED,
and never removing them or changing code.

  [01/15]: imap-send: mark unused parameter in ssl_socket_connect() fallback
  [02/15]: update-ref: mark more unused parameters in parser callbacks
  [03/15]: refs: mark unused parameters in ref_store fsck callbacks
  [04/15]: refs: mark unused parameters in do_for_each_reflog_helper()
  [05/15]: oss-fuzz: mark unused argv/argc argument
  [06/15]: t/helper: mark more unused argv/argc arguments
  [07/15]: unit-tests: ignore unused argc/argv
  [08/15]: reftable: ignore unused argc/argv in test functions
  [09/15]: reftable: drop obsolete test function declarations
  [10/15]: reftable: mark unused parameters in virtual functions
  [11/15]: t-hashmap: mark unused parameters in callback function
  [12/15]: test-mergesort: mark unused parameters in trivial callback
  [13/15]: setup: mark unused parameter in config callback
  [14/15]: daemon: mark unused parameters in non-posix fallbacks
  [15/15]: scalar: mark unused parmaeters in dummy function

 builtin/update-ref.c             |  8 ++++----
 daemon.c                         |  6 +++---
 imap-send.c                      |  2 +-
 oss-fuzz/dummy-cmd-main.c        |  2 +-
 refs.c                           |  5 +++--
 refs/packed-backend.c            |  4 ++--
 refs/reftable-backend.c          | 18 +++++++++---------
 reftable/block_test.c            |  2 +-
 reftable/blocksource.c           |  8 ++++----
 reftable/generic.c               |  8 +++++---
 reftable/iter.c                  |  3 ++-
 reftable/readwrite_test.c        |  2 +-
 reftable/record.c                | 31 ++++++++++++++++++-------------
 reftable/reftable-tests.h        |  2 --
 reftable/stack_test.c            |  6 +++---
 reftable/test_framework.c        |  2 +-
 reftable/writer.c                |  2 +-
 scalar.c                         |  3 ++-
 setup.c                          |  2 +-
 t/helper/test-example-tap.c      |  2 +-
 t/helper/test-hashmap.c          |  2 +-
 t/helper/test-mergesort.c        |  2 +-
 t/unit-tests/t-ctype.c           |  2 +-
 t/unit-tests/t-hash.c            |  2 +-
 t/unit-tests/t-hashmap.c         |  3 ++-
 t/unit-tests/t-mem-pool.c        |  2 +-
 t/unit-tests/t-prio-queue.c      |  2 +-
 t/unit-tests/t-reftable-basics.c |  2 +-
 t/unit-tests/t-reftable-merged.c |  4 ++--
 t/unit-tests/t-reftable-pq.c     |  2 +-
 t/unit-tests/t-reftable-record.c |  2 +-
 t/unit-tests/t-reftable-tree.c   |  2 +-
 t/unit-tests/t-strbuf.c          |  2 +-
 t/unit-tests/t-strcmp-offset.c   |  2 +-
 t/unit-tests/t-strvec.c          |  2 +-
 t/unit-tests/t-trailer.c         |  2 +-
 36 files changed, 81 insertions(+), 72 deletions(-)

-Peff

[PATCH 01/15] imap-send: mark unused parameter in ssl_socket_connect() fallback

[Jeff King]

Commit cea1ff7f1f (imap-send: drop global `imap_server_conf` variable,
2024-06-07) added an imap_server_conf parameter to several functions.
But when compiled with NO_OPENSSL, the ssl_socket_connect() fallback
just returns immediately, so its parameters all need to be annotated to
avoid triggering -Wunused-parameter.

Signed-off-by: Jeff King <peff@peff.net>
---
 imap-send.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/imap-send.c b/imap-send.c
index 01404e5047..3551908de6 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -190,7 +190,7 @@ static void socket_perror(const char *func, struct imap_socket *sock, int ret)
 
 #ifdef NO_OPENSSL
 static int ssl_socket_connect(struct imap_socket *sock UNUSED,
-			      const struct imap_server_conf *cfg,
+			      const struct imap_server_conf *cfg UNUSED,
 			      int use_tls_only UNUSED)
 {
 	fprintf(stderr, "SSL requested but SSL support not compiled in\n");
-- 
2.46.0.585.gd6679c16d8

[PATCH 02/15] update-ref: mark more unused parameters in parser callbacks

[Jeff King]

This is a continuation of 44ad082968 (update-ref: mark unused parameter
in parser callbacks, 2023-08-29), as we've grown a few more virtual
functions since then.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/update-ref.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 6a6a2ff55d..8f31da9a4b 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -274,7 +274,7 @@ static void parse_cmd_update(struct ref_transaction *transaction,
 }
 
 static void parse_cmd_symref_update(struct ref_transaction *transaction,
-				    const char *next, const char *end)
+				    const char *next, const char *end UNUSED)
 {
 	char *refname, *new_target, *old_arg;
 	char *old_target = NULL;
@@ -360,7 +360,7 @@ static void parse_cmd_create(struct ref_transaction *transaction,
 
 
 static void parse_cmd_symref_create(struct ref_transaction *transaction,
-				    const char *next, const char *end)
+				    const char *next, const char *end UNUSED)
 {
 	struct strbuf err = STRBUF_INIT;
 	char *refname, *new_target;
@@ -423,7 +423,7 @@ static void parse_cmd_delete(struct ref_transaction *transaction,
 
 
 static void parse_cmd_symref_delete(struct ref_transaction *transaction,
-				    const char *next, const char *end)
+				    const char *next, const char *end UNUSED)
 {
 	struct strbuf err = STRBUF_INIT;
 	char *refname, *old_target;
@@ -479,7 +479,7 @@ static void parse_cmd_verify(struct ref_transaction *transaction,
 }
 
 static void parse_cmd_symref_verify(struct ref_transaction *transaction,
-				    const char *next, const char *end)
+				    const char *next, const char *end UNUSED)
 {
 	struct strbuf err = STRBUF_INIT;
 	struct object_id old_oid;
-- 
2.46.0.585.gd6679c16d8

[PATCH 03/15] refs: mark unused parameters in ref_store fsck callbacks

[Jeff King]

Commit ab6f79d8df (refs: set up ref consistency check infrastructure,
2024-08-08) added virtual functions to the ref store for doing fsck
checks. But the packed and reftable backends do not yet do anything.

Let's annotate them to silence -Wunused-parameter.

Signed-off-by: Jeff King <peff@peff.net>
---
 refs/packed-backend.c   | 4 ++--
 refs/reftable-backend.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index f00106df14..afd292ec6a 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1733,8 +1733,8 @@ static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_s
 	return empty_ref_iterator_begin();
 }
 
-static int packed_fsck(struct ref_store *ref_store,
-		       struct fsck_options *o)
+static int packed_fsck(struct ref_store *ref_store UNUSED,
+		       struct fsck_options *o UNUSED)
 {
 	return 0;
 }
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index 8b7ffbf66f..7337d4422d 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -2309,8 +2309,8 @@ static int reftable_be_reflog_expire(struct ref_store *ref_store,
 	return ret;
 }
 
-static int reftable_be_fsck(struct ref_store *ref_store,
-			    struct fsck_options *o)
+static int reftable_be_fsck(struct ref_store *ref_store UNUSED,
+			    struct fsck_options *o UNUSED)
 {
 	return 0;
 }
-- 
2.46.0.585.gd6679c16d8

[PATCH 04/15] refs: mark unused parameters in do_for_each_reflog_helper()

[Jeff King]

This is an each_ref_fn callback, so it has to match that interface. We
marked most of these in 63e14ee2d6 (refs: mark unused each_ref_fn
parameters, 2022-08-19), but in this case:

  - this function was created in 31f898397b (refs: drop unused params
    from the reflog iterator callback, 2024-02-21), and most of the
    arguments were correctly mark as UNUSED, but "flags" was missed.

  - commit e8207717f1 (refs: add referent to each_ref_fn, 2024-08-09)
    added a new argument to the each_ref_fn callback. In most callbacks
    it added an UNUSED annotation, but it missed one case.

Signed-off-by: Jeff King <peff@peff.net>
---
 refs.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/refs.c b/refs.c
index 74de3d3009..584674520e 100644
--- a/refs.c
+++ b/refs.c
@@ -2390,9 +2390,10 @@ struct do_for_each_reflog_help {
 	void *cb_data;
 };
 
-static int do_for_each_reflog_helper(const char *refname, const char *referent,
+static int do_for_each_reflog_helper(const char *refname,
+				     const char *referent UNUSED,
 				     const struct object_id *oid UNUSED,
-				     int flags,
+				     int flags UNUSED,
 				     void *cb_data)
 {
 	struct do_for_each_reflog_help *hp = cb_data;
-- 
2.46.0.585.gd6679c16d8

[PATCH 05/15] oss-fuzz: mark unused argv/argc argument

[Jeff King]

The dummy fuzz cmd_main() does not look at its argc/argv parameters
(since it should never even be run), but has to match the usual
cmd_main() declaration.

Mark them to silence -Wunused-parameter.

Signed-off-by: Jeff King <peff@peff.net>
---
 oss-fuzz/dummy-cmd-main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/oss-fuzz/dummy-cmd-main.c b/oss-fuzz/dummy-cmd-main.c
index 071cb231ba..8ef776d06f 100644
--- a/oss-fuzz/dummy-cmd-main.c
+++ b/oss-fuzz/dummy-cmd-main.c
@@ -8,7 +8,7 @@
  * executed.
  */
 
-int cmd_main(int argc, const char **argv) {
+int cmd_main(int argc UNUSED, const char **argv UNUSED) {
 	BUG("We should not execute cmd_main() from a fuzz target");
 	return 1;
 }
-- 
2.46.0.585.gd6679c16d8

[PATCH 06/15] t/helper: mark more unused argv/argc arguments

[Jeff King]

This is a continuation of 126e3b3d2a (t/helper: mark unused argv/argc
arguments, 2023-03-28) to cover a few new cases:

 - test-example-tap was added since that commit

 - test-hashmap used to accept the "ignorecase" argument on the command
   line. But since most of its logic was moved to a unit-test in
   3469a23659 (t: port helper/test-hashmap.c to unit-tests/t-hashmap.c,
   2024-08-03), it now ignores its argv entirely.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/helper/test-example-tap.c | 2 +-
 t/helper/test-hashmap.c     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/helper/test-example-tap.c b/t/helper/test-example-tap.c
index d072ad559f..f35667bd29 100644
--- a/t/helper/test-example-tap.c
+++ b/t/helper/test-example-tap.c
@@ -70,7 +70,7 @@ static void t_empty(void)
 	; /* empty */
 }
 
-int cmd__example_tap(int argc, const char **argv)
+int cmd__example_tap(int argc UNUSED, const char **argv UNUSED)
 {
 	test_res = TEST(check_res = check_int(1, ==, 1), "passing test");
 	TEST(t_res(1), "passing test and assertion return 1");
diff --git a/t/helper/test-hashmap.c b/t/helper/test-hashmap.c
index 195e6278be..7782ae585e 100644
--- a/t/helper/test-hashmap.c
+++ b/t/helper/test-hashmap.c
@@ -138,7 +138,7 @@ static void perf_hashmap(unsigned int method, unsigned int rounds)
  *
  * perfhashmap method rounds -> test hashmap.[ch] performance
  */
-int cmd__hashmap(int argc, const char **argv)
+int cmd__hashmap(int argc UNUSED, const char **argv UNUSED)
 {
 	struct string_list parts = STRING_LIST_INIT_NODUP;
 	struct strbuf line = STRBUF_INIT;
-- 
2.46.0.585.gd6679c16d8

[PATCH 07/15] unit-tests: ignore unused argc/argv

[Jeff King]

All of the unit test programs have their own cmd_main() function, but
none of them actually look at the argc/argv that is passed in.

In the long run we may want them to handle options for the test harness.
But we'd probably do that with a shared harness cmd_main(), dispatching
to the individual tests. In the meantime, let's annotate the unused
parameters to avoid triggering -Wunused-parameter.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/unit-tests/t-ctype.c           | 2 +-
 t/unit-tests/t-hash.c            | 2 +-
 t/unit-tests/t-mem-pool.c        | 2 +-
 t/unit-tests/t-prio-queue.c      | 2 +-
 t/unit-tests/t-reftable-basics.c | 2 +-
 t/unit-tests/t-reftable-merged.c | 2 +-
 t/unit-tests/t-reftable-pq.c     | 2 +-
 t/unit-tests/t-reftable-record.c | 2 +-
 t/unit-tests/t-reftable-tree.c   | 2 +-
 t/unit-tests/t-strbuf.c          | 2 +-
 t/unit-tests/t-strcmp-offset.c   | 2 +-
 t/unit-tests/t-strvec.c          | 2 +-
 t/unit-tests/t-trailer.c         | 2 +-
 13 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/t/unit-tests/t-ctype.c b/t/unit-tests/t-ctype.c
index d6ac1fe678..24cc566cfe 100644
--- a/t/unit-tests/t-ctype.c
+++ b/t/unit-tests/t-ctype.c
@@ -33,7 +33,7 @@
 	"\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f" \
 	"\x7f"
 
-int cmd_main(int argc, const char **argv) {
+int cmd_main(int argc UNUSED, const char **argv UNUSED) {
 	TEST_CHAR_CLASS(isspace, " \n\r\t");
 	TEST_CHAR_CLASS(isdigit, DIGIT);
 	TEST_CHAR_CLASS(isalpha, LOWER UPPER);
diff --git a/t/unit-tests/t-hash.c b/t/unit-tests/t-hash.c
index e9a78bf2c0..e62647019b 100644
--- a/t/unit-tests/t-hash.c
+++ b/t/unit-tests/t-hash.c
@@ -38,7 +38,7 @@ static void check_hash_data(const void *data, size_t data_length,
 		     "SHA1 and SHA256 (%s) works", #literal); \
 	} while (0)
 
-int cmd_main(int argc, const char **argv)
+int cmd_main(int argc UNUSED, const char **argv UNUSED)
 {
 	struct strbuf aaaaaaaaaa_100000 = STRBUF_INIT;
 	struct strbuf alphabet_100000 = STRBUF_INIT;
diff --git a/t/unit-tests/t-mem-pool.c b/t/unit-tests/t-mem-pool.c
index a0d57df761..fe500c704b 100644
--- a/t/unit-tests/t-mem-pool.c
+++ b/t/unit-tests/t-mem-pool.c
@@ -20,7 +20,7 @@ static void t_calloc_100(struct mem_pool *pool)
 	check(pool->mp_block->end != NULL);
 }
 
-int cmd_main(int argc, const char **argv)
+int cmd_main(int argc UNUSED, const char **argv UNUSED)
 {
 	TEST(setup_static(t_calloc_100, 1024 * 1024),
 	     "mem_pool_calloc returns 100 zeroed bytes with big block");
diff --git a/t/unit-tests/t-prio-queue.c b/t/unit-tests/t-prio-queue.c
index 7a4e5780e1..fe6ae37935 100644
--- a/t/unit-tests/t-prio-queue.c
+++ b/t/unit-tests/t-prio-queue.c
@@ -69,7 +69,7 @@ static void test_prio_queue(int *input, size_t input_size,
 #define TEST_INPUT(input, result) \
 	test_prio_queue(input, ARRAY_SIZE(input), result, ARRAY_SIZE(result))
 
-int cmd_main(int argc, const char **argv)
+int cmd_main(int argc UNUSED, const char **argv UNUSED)
 {
 	TEST(TEST_INPUT(((int []){ 2, 6, 3, 10, 9, 5, 7, 4, 5, 8, 1, DUMP }),
 			((int []){ 1, 2, 3, 4, 5, 5, 6, 7, 8, 9, 10 })),
diff --git a/t/unit-tests/t-reftable-basics.c b/t/unit-tests/t-reftable-basics.c
index 4e80bdf16d..7ffee74dca 100644
--- a/t/unit-tests/t-reftable-basics.c
+++ b/t/unit-tests/t-reftable-basics.c
@@ -145,7 +145,7 @@ static void test_u16_roundtrip(void)
 	check_int(in, ==, out);
 }
 
-int cmd_main(int argc, const char *argv[])
+int cmd_main(int argc UNUSED, const char *argv[] UNUSED)
 {
 	TEST(test_common_prefix(), "common_prefix_size works");
 	TEST(test_parse_names_normal(), "parse_names works for basic input");
diff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c
index b6263ee8b5..2d65d5c3cb 100644
--- a/t/unit-tests/t-reftable-merged.c
+++ b/t/unit-tests/t-reftable-merged.c
@@ -452,7 +452,7 @@ static void t_default_write_opts(void)
 }
 
 
-int cmd_main(int argc, const char *argv[])
+int cmd_main(int argc UNUSED, const char *argv[] UNUSED)
 {
 	TEST(t_default_write_opts(), "merged table with default write opts");
 	TEST(t_merged_logs(), "merged table with multiple log updates for same ref");
diff --git a/t/unit-tests/t-reftable-pq.c b/t/unit-tests/t-reftable-pq.c
index 039bd0f1f9..ada4c19f18 100644
--- a/t/unit-tests/t-reftable-pq.c
+++ b/t/unit-tests/t-reftable-pq.c
@@ -142,7 +142,7 @@ static void t_merged_iter_pqueue_top(void)
 	merged_iter_pqueue_release(&pq);
 }
 
-int cmd_main(int argc, const char *argv[])
+int cmd_main(int argc UNUSED, const char *argv[] UNUSED)
 {
 	TEST(t_pq_record(), "pq works with record-based comparison");
 	TEST(t_pq_index(), "pq works with index-based comparison");
diff --git a/t/unit-tests/t-reftable-record.c b/t/unit-tests/t-reftable-record.c
index cb649ee419..a7f67d4d9f 100644
--- a/t/unit-tests/t-reftable-record.c
+++ b/t/unit-tests/t-reftable-record.c
@@ -532,7 +532,7 @@ static void t_reftable_index_record_roundtrip(void)
 	strbuf_release(&in.u.idx.last_key);
 }
 
-int cmd_main(int argc, const char *argv[])
+int cmd_main(int argc UNUSED, const char *argv[] UNUSED)
 {
 	TEST(t_reftable_ref_record_comparison(), "comparison operations work on ref record");
 	TEST(t_reftable_log_record_comparison(), "comparison operations work on log record");
diff --git a/t/unit-tests/t-reftable-tree.c b/t/unit-tests/t-reftable-tree.c
index e7d774d774..700479d34b 100644
--- a/t/unit-tests/t-reftable-tree.c
+++ b/t/unit-tests/t-reftable-tree.c
@@ -75,7 +75,7 @@ static void t_infix_walk(void)
 	tree_free(root);
 }
 
-int cmd_main(int argc, const char *argv[])
+int cmd_main(int argc UNUSED, const char *argv[] UNUSED)
 {
 	TEST(t_tree_search(), "tree_search works");
 	TEST(t_infix_walk(), "infix_walk works");
diff --git a/t/unit-tests/t-strbuf.c b/t/unit-tests/t-strbuf.c
index 6027dafef7..3f4044d697 100644
--- a/t/unit-tests/t-strbuf.c
+++ b/t/unit-tests/t-strbuf.c
@@ -105,7 +105,7 @@ static void t_addstr(struct strbuf *buf, const void *data)
 	check_str(buf->buf + orig_len, text);
 }
 
-int cmd_main(int argc, const char **argv)
+int cmd_main(int argc UNUSED, const char **argv UNUSED)
 {
 	if (!TEST(t_static_init(), "static initialization works"))
 		test_skip_all("STRBUF_INIT is broken");
diff --git a/t/unit-tests/t-strcmp-offset.c b/t/unit-tests/t-strcmp-offset.c
index fe4c2706b1..6880f21161 100644
--- a/t/unit-tests/t-strcmp-offset.c
+++ b/t/unit-tests/t-strcmp-offset.c
@@ -24,7 +24,7 @@ static void check_strcmp_offset(const char *string1, const char *string2,
 				 expect_offset),                           \
 	     "strcmp_offset(%s, %s) works", #string1, #string2)
 
-int cmd_main(int argc, const char **argv)
+int cmd_main(int argc UNUSED, const char **argv UNUSED)
 {
 	TEST_STRCMP_OFFSET("abc", "abc", 0, 3);
 	TEST_STRCMP_OFFSET("abc", "def", -1, 0);
diff --git a/t/unit-tests/t-strvec.c b/t/unit-tests/t-strvec.c
index fa1a041469..c3b610dda7 100644
--- a/t/unit-tests/t-strvec.c
+++ b/t/unit-tests/t-strvec.c
@@ -227,7 +227,7 @@ static void t_detach(void)
 	free(detached);
 }
 
-int cmd_main(int argc, const char **argv)
+int cmd_main(int argc UNUSED, const char **argv UNUSED)
 {
 	TEST(t_static_init(), "static initialization");
 	TEST(t_dynamic_init(), "dynamic initialization");
diff --git a/t/unit-tests/t-trailer.c b/t/unit-tests/t-trailer.c
index 2ecca359d9..e1c6ad7461 100644
--- a/t/unit-tests/t-trailer.c
+++ b/t/unit-tests/t-trailer.c
@@ -308,7 +308,7 @@ static void run_t_trailer_iterator(void)
 	}
 }
 
-int cmd_main(int argc, const char **argv)
+int cmd_main(int argc UNUSED, const char **argv UNUSED)
 {
 	run_t_trailer_iterator();
 	return test_done();
-- 
2.46.0.585.gd6679c16d8

[PATCH 08/15] reftable: ignore unused argc/argv in test functions

[Jeff King]

There are several reftable test "main" functions that don't look at
their argc/argv. They don't technically need to take these parameters,
as they are called individually by cmd__reftable(). But it probably
makes sense to keep them all consistent for now. In the long run these
will probably all get converted to the unit-test framework anyway.

Signed-off-by: Jeff King <peff@peff.net>
---
 reftable/block_test.c     | 2 +-
 reftable/readwrite_test.c | 2 +-
 reftable/stack_test.c     | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/reftable/block_test.c b/reftable/block_test.c
index 90aecd5a7c..f8e31d2d3c 100644
--- a/reftable/block_test.c
+++ b/reftable/block_test.c
@@ -116,7 +116,7 @@ static void test_block_read_write(void)
 	}
 }
 
-int block_test_main(int argc, const char *argv[])
+int block_test_main(int argc UNUSED, const char *argv[] UNUSED)
 {
 	RUN_TEST(test_block_read_write);
 	return 0;
diff --git a/reftable/readwrite_test.c b/reftable/readwrite_test.c
index f411abfe9c..15a113b78a 100644
--- a/reftable/readwrite_test.c
+++ b/reftable/readwrite_test.c
@@ -951,7 +951,7 @@ static void test_corrupt_table(void)
 	strbuf_release(&buf);
 }
 
-int readwrite_test_main(int argc, const char *argv[])
+int readwrite_test_main(int argc UNUSED, const char *argv[] UNUSED)
 {
 	RUN_TEST(test_log_zlib_corruption);
 	RUN_TEST(test_corrupt_table);
diff --git a/reftable/stack_test.c b/reftable/stack_test.c
index 8c36590ff0..f84b92f312 100644
--- a/reftable/stack_test.c
+++ b/reftable/stack_test.c
@@ -1084,7 +1084,7 @@ static void test_reftable_stack_compaction_concurrent_clean(void)
 	clear_dir(dir);
 }
 
-int stack_test_main(int argc, const char *argv[])
+int stack_test_main(int argc UNUSED, const char *argv[] UNUSED)
 {
 	RUN_TEST(test_empty_add);
 	RUN_TEST(test_read_file);
-- 
2.46.0.585.gd6679c16d8

[PATCH 09/15] reftable: drop obsolete test function declarations

[Jeff King]

These functions were moved to the unit test framework in ba9661b457 (t:
move reftable/record_test.c to the unit testing framework, 2024-07-02)
and b34116a30c (t: move reftable/basics_test.c to the unit testing
framework, 2024-05-29). The declarations in reftable-tests.h are
leftover cruft.

Signed-off-by: Jeff King <peff@peff.net>
---
OK, I lied in the cover letter, this one isn't just adding UNUSED. But
hopefully it is also obviously correct. ;)

 reftable/reftable-tests.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/reftable/reftable-tests.h b/reftable/reftable-tests.h
index 4b666810af..941e5dd011 100644
--- a/reftable/reftable-tests.h
+++ b/reftable/reftable-tests.h
@@ -9,9 +9,7 @@ license that can be found in the LICENSE file or at
 #ifndef REFTABLE_TESTS_H
 #define REFTABLE_TESTS_H
 
-int basics_test_main(int argc, const char **argv);
 int block_test_main(int argc, const char **argv);
-int record_test_main(int argc, const char **argv);
 int readwrite_test_main(int argc, const char **argv);
 int stack_test_main(int argc, const char **argv);
 int reftable_dump_main(int argc, char *const *argv);
-- 
2.46.0.585.gd6679c16d8

[PATCH 10/15] reftable: mark unused parameters in virtual functions

[Jeff King]

The reftable code uses a lot of virtual function pointers, but many of
the concrete implementations do not need all of the parameters.

For the most part these are obviously fine to just mark as UNUSED (e.g.,
the empty_iterator functions unsurprisingly do not do anything). Here
are a few cases where I dug a little deeper (but still ended up just
marking them UNUSED):

  - the iterator exclude_patterns is best-effort and optional (though it
    would be nice to support in the long run as an optimization)

  - ignoring the ref_store in many transaction functions is unexpected,
    but works because the ref_transaction itself carries enough
    information to do what we need.

  - ignoring "err" for in some cases (e.g., transaction abort) is OK
    because we do not return any errors. It is a little odd for
    reftable_be_create_reflog(), though, since we do return errors
    there. We should perhaps be creating string error messages at this
    layer, but I've punted on that for now.

Signed-off-by: Jeff King <peff@peff.net>
---
 refs/reftable-backend.c          | 14 +++++++-------
 reftable/blocksource.c           |  8 ++++----
 reftable/generic.c               |  8 +++++---
 reftable/iter.c                  |  3 ++-
 reftable/record.c                | 31 ++++++++++++++++++-------------
 reftable/stack_test.c            |  4 ++--
 reftable/test_framework.c        |  2 +-
 reftable/writer.c                |  2 +-
 t/unit-tests/t-reftable-merged.c |  2 +-
 9 files changed, 41 insertions(+), 33 deletions(-)

diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index 7337d4422d..a2de39f2ca 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -614,7 +614,7 @@ static struct reftable_ref_iterator *ref_iterator_for_stack(struct reftable_ref_
 
 static struct ref_iterator *reftable_be_iterator_begin(struct ref_store *ref_store,
 						       const char *prefix,
-						       const char **exclude_patterns,
+						       const char **exclude_patterns UNUSED,
 						       unsigned int flags)
 {
 	struct reftable_ref_iterator *main_iter, *worktree_iter;
@@ -1123,9 +1123,9 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,
 	return ret;
 }
 
-static int reftable_be_transaction_abort(struct ref_store *ref_store,
+static int reftable_be_transaction_abort(struct ref_store *ref_store UNUSED,
 					 struct ref_transaction *transaction,
-					 struct strbuf *err)
+					 struct strbuf *err UNUSED)
 {
 	struct reftable_transaction_data *tx_data = transaction->backend_data;
 	free_transaction_data(tx_data);
@@ -1315,7 +1315,7 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data
 	return ret;
 }
 
-static int reftable_be_transaction_finish(struct ref_store *ref_store,
+static int reftable_be_transaction_finish(struct ref_store *ref_store UNUSED,
 					  struct ref_transaction *transaction,
 					  struct strbuf *err)
 {
@@ -1726,8 +1726,8 @@ static int reftable_reflog_iterator_advance(struct ref_iterator *ref_iterator)
 	return ITER_OK;
 }
 
-static int reftable_reflog_iterator_peel(struct ref_iterator *ref_iterator,
-						 struct object_id *peeled)
+static int reftable_reflog_iterator_peel(struct ref_iterator *ref_iterator UNUSED,
+					 struct object_id *peeled UNUSED)
 {
 	BUG("reftable reflog iterator cannot be peeled");
 	return -1;
@@ -1988,7 +1988,7 @@ static int write_reflog_existence_table(struct reftable_writer *writer,
 
 static int reftable_be_create_reflog(struct ref_store *ref_store,
 				     const char *refname,
-				     struct strbuf *errmsg)
+				     struct strbuf *errmsg UNUSED)
 {
 	struct reftable_ref_store *refs =
 		reftable_be_downcast(ref_store, REF_STORE_WRITE, "create_reflog");
diff --git a/reftable/blocksource.c b/reftable/blocksource.c
index eeed254ba9..abce4bb2e1 100644
--- a/reftable/blocksource.c
+++ b/reftable/blocksource.c
@@ -13,14 +13,14 @@ license that can be found in the LICENSE file or at
 #include "reftable-blocksource.h"
 #include "reftable-error.h"
 
-static void strbuf_return_block(void *b, struct reftable_block *dest)
+static void strbuf_return_block(void *b UNUSED, struct reftable_block *dest)
 {
 	if (dest->len)
 		memset(dest->data, 0xff, dest->len);
 	reftable_free(dest->data);
 }
 
-static void strbuf_close(void *b)
+static void strbuf_close(void *b UNUSED)
 {
 }
 
@@ -55,7 +55,7 @@ void block_source_from_strbuf(struct reftable_block_source *bs,
 	bs->arg = buf;
 }
 
-static void malloc_return_block(void *b, struct reftable_block *dest)
+static void malloc_return_block(void *b UNUSED, struct reftable_block *dest)
 {
 	if (dest->len)
 		memset(dest->data, 0xff, dest->len);
@@ -85,7 +85,7 @@ static uint64_t file_size(void *b)
 	return ((struct file_block_source *)b)->size;
 }
 
-static void file_return_block(void *b, struct reftable_block *dest)
+static void file_return_block(void *b UNUSED, struct reftable_block *dest UNUSED)
 {
 }
 
diff --git a/reftable/generic.c b/reftable/generic.c
index 28ae26145e..a00725d9c4 100644
--- a/reftable/generic.c
+++ b/reftable/generic.c
@@ -201,17 +201,19 @@ int iterator_next(struct reftable_iterator *it, struct reftable_record *rec)
 	return it->ops->next(it->iter_arg, rec);
 }
 
-static int empty_iterator_seek(void *arg, struct reftable_record *want)
+static int empty_iterator_seek(void *arg UNUSED,
+			       struct reftable_record *want UNUSED)
 {
 	return 0;
 }
 
-static int empty_iterator_next(void *arg, struct reftable_record *rec)
+static int empty_iterator_next(void *arg UNUSED,
+			       struct reftable_record *rec UNUSED)
 {
 	return 1;
 }
 
-static void empty_iterator_close(void *arg)
+static void empty_iterator_close(void *arg UNUSED)
 {
 }
 
diff --git a/reftable/iter.c b/reftable/iter.c
index fddea31e51..9e8b2952fd 100644
--- a/reftable/iter.c
+++ b/reftable/iter.c
@@ -127,7 +127,8 @@ static int indexed_table_ref_iter_next_block(struct indexed_table_ref_iter *it)
 	return 0;
 }
 
-static int indexed_table_ref_iter_seek(void *p, struct reftable_record *want)
+static int indexed_table_ref_iter_seek(void *p UNUSED,
+				       struct reftable_record *want UNUSED)
 {
 	BUG("seeking indexed table is not supported");
 	return -1;
diff --git a/reftable/record.c b/reftable/record.c
index a2cba5ef74..2ec0c6d346 100644
--- a/reftable/record.c
+++ b/reftable/record.c
@@ -516,7 +516,7 @@ static void reftable_obj_record_release(void *rec)
 	memset(obj, 0, sizeof(struct reftable_obj_record));
 }
 
-static void reftable_obj_record_print(const void *rec, int hash_size)
+static void reftable_obj_record_print(const void *rec, int hash_size UNUSED)
 {
 	const struct reftable_obj_record *obj = rec;
 	char hex[GIT_MAX_HEXSZ + 1] = { 0 };
@@ -532,7 +532,7 @@ static void reftable_obj_record_print(const void *rec, int hash_size)
 }
 
 static void reftable_obj_record_copy_from(void *rec, const void *src_rec,
-					  int hash_size)
+					  int hash_size UNUSED)
 {
 	struct reftable_obj_record *obj = rec;
 	const struct reftable_obj_record *src =
@@ -559,7 +559,7 @@ static uint8_t reftable_obj_record_val_type(const void *rec)
 }
 
 static int reftable_obj_record_encode(const void *rec, struct string_view s,
-				      int hash_size)
+				      int hash_size UNUSED)
 {
 	const struct reftable_obj_record *r = rec;
 	struct string_view start = s;
@@ -594,7 +594,8 @@ static int reftable_obj_record_encode(const void *rec, struct string_view s,
 
 static int reftable_obj_record_decode(void *rec, struct strbuf key,
 				      uint8_t val_type, struct string_view in,
-				      int hash_size, struct strbuf *scratch UNUSED)
+				      int hash_size UNUSED,
+				      struct strbuf *scratch UNUSED)
 {
 	struct string_view start = in;
 	struct reftable_obj_record *r = rec;
@@ -647,12 +648,13 @@ static int reftable_obj_record_decode(void *rec, struct strbuf key,
 	return start.len - in.len;
 }
 
-static int not_a_deletion(const void *p)
+static int not_a_deletion(const void *p UNUSED)
 {
 	return 0;
 }
 
-static int reftable_obj_record_equal_void(const void *a, const void *b, int hash_size)
+static int reftable_obj_record_equal_void(const void *a, const void *b,
+					  int hash_size UNUSED)
 {
 	struct reftable_obj_record *ra = (struct reftable_obj_record *) a;
 	struct reftable_obj_record *rb = (struct reftable_obj_record *) b;
@@ -1066,7 +1068,7 @@ static void reftable_index_record_key(const void *r, struct strbuf *dest)
 }
 
 static void reftable_index_record_copy_from(void *rec, const void *src_rec,
-					    int hash_size)
+					    int hash_size UNUSED)
 {
 	struct reftable_index_record *dst = rec;
 	const struct reftable_index_record *src = src_rec;
@@ -1082,13 +1084,13 @@ static void reftable_index_record_release(void *rec)
 	strbuf_release(&idx->last_key);
 }
 
-static uint8_t reftable_index_record_val_type(const void *rec)
+static uint8_t reftable_index_record_val_type(const void *rec UNUSED)
 {
 	return 0;
 }
 
 static int reftable_index_record_encode(const void *rec, struct string_view out,
-					int hash_size)
+					int hash_size UNUSED)
 {
 	const struct reftable_index_record *r =
 		(const struct reftable_index_record *)rec;
@@ -1104,8 +1106,10 @@ static int reftable_index_record_encode(const void *rec, struct string_view out,
 }
 
 static int reftable_index_record_decode(void *rec, struct strbuf key,
-					uint8_t val_type, struct string_view in,
-					int hash_size, struct strbuf *scratch UNUSED)
+					uint8_t val_type UNUSED,
+					struct string_view in,
+					int hash_size UNUSED,
+					struct strbuf *scratch UNUSED)
 {
 	struct string_view start = in;
 	struct reftable_index_record *r = rec;
@@ -1122,7 +1126,8 @@ static int reftable_index_record_decode(void *rec, struct strbuf key,
 	return start.len - in.len;
 }
 
-static int reftable_index_record_equal(const void *a, const void *b, int hash_size)
+static int reftable_index_record_equal(const void *a, const void *b,
+				       int hash_size UNUSED)
 {
 	struct reftable_index_record *ia = (struct reftable_index_record *) a;
 	struct reftable_index_record *ib = (struct reftable_index_record *) b;
@@ -1137,7 +1142,7 @@ static int reftable_index_record_cmp(const void *_a, const void *_b)
 	return strbuf_cmp(&a->last_key, &b->last_key);
 }
 
-static void reftable_index_record_print(const void *rec, int hash_size)
+static void reftable_index_record_print(const void *rec, int hash_size UNUSED)
 {
 	const struct reftable_index_record *idx = rec;
 	/* TODO: escape null chars? */
diff --git a/reftable/stack_test.c b/reftable/stack_test.c
index f84b92f312..1a638cd2e0 100644
--- a/reftable/stack_test.c
+++ b/reftable/stack_test.c
@@ -399,7 +399,7 @@ static void test_reftable_stack_auto_compaction_fails_gracefully(void)
 	clear_dir(dir);
 }
 
-static int write_error(struct reftable_writer *wr, void *arg)
+static int write_error(struct reftable_writer *wr UNUSED, void *arg)
 {
 	return *((int *)arg);
 }
@@ -816,7 +816,7 @@ static void test_reflog_expire(void)
 	reftable_log_record_release(&log);
 }
 
-static int write_nothing(struct reftable_writer *wr, void *arg)
+static int write_nothing(struct reftable_writer *wr, void *arg UNUSED)
 {
 	reftable_writer_set_limits(wr, 1, 1);
 	return 0;
diff --git a/reftable/test_framework.c b/reftable/test_framework.c
index 4066924eee..a07fec5d84 100644
--- a/reftable/test_framework.c
+++ b/reftable/test_framework.c
@@ -21,7 +21,7 @@ ssize_t strbuf_add_void(void *b, const void *data, size_t sz)
 	return sz;
 }
 
-int noop_flush(void *arg)
+int noop_flush(void *arg UNUSED)
 {
 	return 0;
 }
diff --git a/reftable/writer.c b/reftable/writer.c
index 45b3e9ce1f..9d5e6072bc 100644
--- a/reftable/writer.c
+++ b/reftable/writer.c
@@ -544,7 +544,7 @@ static void write_object_record(void *void_arg, void *key)
 done:;
 }
 
-static void object_record_free(void *void_arg, void *key)
+static void object_record_free(void *void_arg UNUSED, void *key)
 {
 	struct obj_index_tree_node *entry = key;
 
diff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c
index 2d65d5c3cb..99f8fcadfe 100644
--- a/t/unit-tests/t-reftable-merged.c
+++ b/t/unit-tests/t-reftable-merged.c
@@ -22,7 +22,7 @@ static ssize_t strbuf_add_void(void *b, const void *data, const size_t sz)
 	return sz;
 }
 
-static int noop_flush(void *arg)
+static int noop_flush(void *arg UNUSED)
 {
 	return 0;
 }
-- 
2.46.0.585.gd6679c16d8

[PATCH 11/15] t-hashmap: mark unused parameters in callback function

[Jeff King]

The t_intern() setup function doesn't operate on a hashmap, so it
ignores its parameters. But we can't drop them since it is passed as a
pointer to setup(), so we have to match the other setup functions. Mark
them to silence -Wunused-parameter.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/unit-tests/t-hashmap.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/t/unit-tests/t-hashmap.c b/t/unit-tests/t-hashmap.c
index 09a48c2c4e..da102eb541 100644
--- a/t/unit-tests/t-hashmap.c
+++ b/t/unit-tests/t-hashmap.c
@@ -322,7 +322,8 @@ static void t_alloc(struct hashmap *map, unsigned int ignore_case)
 	free(removed);
 }
 
-static void t_intern(struct hashmap *map, unsigned int ignore_case)
+static void t_intern(struct hashmap *map UNUSED,
+		     unsigned int ignore_case UNUSED)
 {
 	const char *values[] = { "value1", "Value1", "value2", "value2" };
 
-- 
2.46.0.585.gd6679c16d8

Re: [PATCH 11/15] t-hashmap: mark unused parameters in callback function

[Ghanshyam Thakkar]

Jeff King <peff@peff.net> wrote:
> The t_intern() setup function doesn't operate on a hashmap, so it
> ignores its parameters. But we can't drop them since it is passed as a
> pointer to setup(), so we have to match the other setup functions. Mark
> them to silence -Wunused-parameter.

Sorry, but I didn't understand why we can't drop them and just call
t_intern() instead of setup(t_intern) (I should've done that, when
writing this). By 'other setup functions', do you mean other test
functions which use setup()? I don't think it is necessary to have
uniformity in function signatures of all the test functions.

diff --git a/t/unit-tests/t-hashmap.c b/t/unit-tests/t-hashmap.c
index 09a48c2c4e..83b79dff39 100644
--- a/t/unit-tests/t-hashmap.c
+++ b/t/unit-tests/t-hashmap.c
@@ -322,7 +322,7 @@ static void t_alloc(struct hashmap *map, unsigned int ignore_case)
 	free(removed);
 }
 
-static void t_intern(struct hashmap *map, unsigned int ignore_case)
+static void t_intern(void)
 {
 	const char *values[] = { "value1", "Value1", "value2", "value2" };
 
@@ -356,6 +356,6 @@ int cmd_main(int argc UNUSED, const char **argv UNUSED)
 	TEST(setup(t_iterate, 0), "iterate works");
 	TEST(setup(t_iterate, 1), "iterate (case insensitive) works");
 	TEST(setup(t_alloc, 0), "grow / shrink works");
-	TEST(setup(t_intern, 0), "string interning works");
+	TEST(t_intern(), "string interning works");
 	return test_done();
 }

Thanks.

Re: [PATCH 11/15] t-hashmap: mark unused parameters in callback function

[Jeff King]

On Sat, Aug 17, 2024 at 08:02:36PM +0530, Ghanshyam Thakkar wrote:

> Jeff King <peff@peff.net> wrote:
> > The t_intern() setup function doesn't operate on a hashmap, so it
> > ignores its parameters. But we can't drop them since it is passed as a
> > pointer to setup(), so we have to match the other setup functions. Mark
> > them to silence -Wunused-parameter.
> 
> Sorry, but I didn't understand why we can't drop them and just call
> t_intern() instead of setup(t_intern) (I should've done that, when
> writing this). By 'other setup functions', do you mean other test
> functions which use setup()? I don't think it is necessary to have
> uniformity in function signatures of all the test functions.

I just assumed that setup() was something that should be called for each
test. But yeah, looking at it, it is really only creating and cleaning
up the hashmap. Since t_intern() doesn't need any of that, then it
should be OK to just skip setup() entirely.

> -static void t_intern(struct hashmap *map, unsigned int ignore_case)
> +static void t_intern(void)
>  {
>  	const char *values[] = { "value1", "Value1", "value2", "value2" };
>  
> @@ -356,6 +356,6 @@ int cmd_main(int argc UNUSED, const char **argv UNUSED)
>  	TEST(setup(t_iterate, 0), "iterate works");
>  	TEST(setup(t_iterate, 1), "iterate (case insensitive) works");
>  	TEST(setup(t_alloc, 0), "grow / shrink works");
> -	TEST(setup(t_intern, 0), "string interning works");
> +	TEST(t_intern(), "string interning works");
>  	return test_done();
>  }

Yeah, that makes sense. I'll update the patch accordingly. Thanks.

-Peff

[PATCH 12/15] test-mergesort: mark unused parameters in trivial callback

[Jeff King]

The mode_copy() function does nothing, but since it's used as a function
pointer within "struct mode", it has to conform to the interface. Mark
it to quiet -Wunused-parameter.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/helper/test-mergesort.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/helper/test-mergesort.c b/t/helper/test-mergesort.c
index 42ccc87051..328bfe2977 100644
--- a/t/helper/test-mergesort.c
+++ b/t/helper/test-mergesort.c
@@ -122,7 +122,7 @@ static const struct dist *get_dist_by_name(const char *name)
 	return NULL;
 }
 
-static void mode_copy(int *arr, int n)
+static void mode_copy(int *arr UNUSED, int n UNUSED)
 {
 	/* nothing */
 }
-- 
2.46.0.585.gd6679c16d8

[PATCH 13/15] setup: mark unused parameter in config callback

[Jeff King]

This is logically a continuation of 783a86c142 (config: mark unused
callback parameters, 2022-08-19), but this case was introduced much
later in 4412a04fe6 (init.templateDir: consider this config setting
protected, 2024-03-29).

Signed-off-by: Jeff King <peff@peff.net>
---
 setup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/setup.c b/setup.c
index 5f81d9fac0..bd44ca21cc 100644
--- a/setup.c
+++ b/setup.c
@@ -1907,7 +1907,7 @@ struct template_dir_cb_data {
 };
 
 static int template_dir_cb(const char *key, const char *value,
-			   const struct config_context *ctx, void *d)
+			   const struct config_context *ctx UNUSED, void *d)
 {
 	struct template_dir_cb_data *data = d;
 
-- 
2.46.0.585.gd6679c16d8

[PATCH 14/15] daemon: mark unused parameters in non-posix fallbacks

[Jeff King]

If NO_POSIX_GOODIES is set, we compile fallback versions of a few
functions. These don't do anything, so their parameters are unused, but
we must keep them to match the ones on the other side of the #ifdef.
Mark them to quiet -Wunused-parameter.

Signed-off-by: Jeff King <peff@peff.net>
---
 daemon.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/daemon.c b/daemon.c
index 17d331b2f3..428a445559 100644
--- a/daemon.c
+++ b/daemon.c
@@ -1175,13 +1175,13 @@ static int service_loop(struct socketlist *socklist)
 
 struct credentials;
 
-static void drop_privileges(struct credentials *cred)
+static void drop_privileges(struct credentials *cred UNUSED)
 {
 	/* nothing */
 }
 
-static struct credentials *prepare_credentials(const char *user_name,
-    const char *group_name)
+static struct credentials *prepare_credentials(const char *user_name UNUSED,
+					       const char *group_name UNUSED)
 {
 	die("--user not supported on this platform");
 }
-- 
2.46.0.585.gd6679c16d8

[PATCH 15/15] scalar: mark unused parameters in dummy function

[Jeff King]

We have a dummy load_builtin_commands() function to satisfy the linker,
but which we never expect to be called. Mark its parameters to avoid
complaints from -Wunused-parameter.

Signed-off-by: Jeff King <peff@peff.net>
---
 scalar.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scalar.c b/scalar.c
index 1fe8a93e65..6166a8dd4c 100644
--- a/scalar.c
+++ b/scalar.c
@@ -400,7 +400,8 @@ static int delete_enlistment(struct strbuf *enlistment)
  * Dummy implementation; Using `get_version_info()` would cause a link error
  * without this.
  */
-void load_builtin_commands(const char *prefix, struct cmdnames *cmds)
+void load_builtin_commands(const char *prefix UNUSED,
+			   struct cmdnames *cmds UNUSED)
 {
 	die("not implemented");
 }
-- 
2.46.0.585.gd6679c16d8