| Message ID | 20240817082447.GK10287@coredump.intra.peff.net (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | f24a9b78a90b333f6857a371be14fd3ee9842add |
| Headers | show |
| Series | marking some more unused parameters | expand |
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.
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
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" };
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(-)