On Sat, Aug 17, 2024 at 08:02:36PM +0530, Ghanshyam Thakkar wrote:
> Jeff King <peff@xxxxxxxx> 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
