On Mon, Feb 17, 2025 at 12:25 PM Patrick Steinhardt [off-list ref] wrote:

On Sat, Feb 15, 2025 at 04:27:17AM +0530, Usman Akinyemi wrote:

quoted

@@ -35,7 +34,8 @@ int cmd_verify_tag(int argc,
              OPT_END()
      };

-     git_config(git_default_config, NULL);
+     if (repo)
+             repo_config(repo, git_default_config, NULL);

I recently noticed that we have `usage_with_options_if_asked()`. Should
we use that function rather than making the call to `git_config()`
conditional? Otherwise it's not obvious why we have the conditional in
the first place.

Hi Patrick,

I think the function is `show_usage_with_options_if_asked()`. The function
is quite different from `git_config()` or the `repo_config()`.  The
config function
consults the configuration file for setting up config values and it
uses the `repo`
variable during this. While `show_usage_with_options_if_asked()`
is used when the "-h" option is passed to the builtin functions to display
the help string.

In a case when "-h" is passed to the builtin functions which use the
RUN_SETUP macro,
the `repo` config will be NULL.

There are some builtin commands functions that which has
the`git_config()` function
comes before `show_usage_with_options_if_asked()` or it's variant and
some, `git_config()`
comes after.

For those that have `git_config()` comes after
`show_usage_with_options_if_asked()` , no need for the check, since
the
 `show_usage_with_options_if_asked()`call will exit without reaching
`git_config()`. For scenario where the `git_config()`
comes earlier, we have to check the `repo` to see if it is NULL, if it
is NULL, we are sure this happens when the "-h" is
passed to the function and we do not need to setup and configuration
since `show_usage_with_options_if_asked()`
will exit.

So, the condition is necessary else, NULL value will be passed to the
`git_config()` which will lead to accessing NULL
value.

Thank you.

The same comment also applies to subsequent commits.

Patrick