On Fri, Nov 25 2022, Sean Allred via GitGitGadget wrote:
> From: Sean Allred <allred.sean@xxxxxxxxx>
>
> Before, git-var could print usage() even if the command was invoked
> correctly with a variable defined in git_vars -- provided that its
> read() function returned NULL.
>
> Now, we only print usage() only if it was called with a logical
"we only ... only if", drop/combine some "only"?
> variable that wasn't defined -- regardless of read().
>
> Since we now know the variable is valid when we call read_var(), we
> can avoid printing usage() here (and exiting with code 129) and
> instead exit quietly with code 1. While exiting with a different code
> can be a breaking change, it's far better than changing the exit
> status more generally from 'failure' to 'success'.
I honestly don't still don't grok what was different here before/after,
whatever we are now/should be doing here, a test as part of this change
asserting the new behavior would be really useful.
> -static const char *read_var(const char *var)
> +static const struct git_var *get_git_var(const char *var)
> {
> struct git_var *ptr;
> - const char *val;
> - val = NULL;
> for (ptr = git_vars; ptr->read; ptr++) {
> if (strcmp(var, ptr->name) == 0) {
> - val = ptr->read(IDENT_STRICT);
> - break;
> + return ptr;
> }
> {
> + const struct git_var *git_var = NULL;
This assignment to "NULL" can be dropped, i.e....
> const char *val = NULL;
> if (argc != 2)
> usage(var_usage);
> @@ -91,10 +89,15 @@ int cmd_var(int argc, const char **argv, const char *prefix)
> return 0;
> }
> git_config(git_default_config, NULL);
> - val = read_var(argv[1]);
> - if (!val)
> +
> + git_var = get_git_var(argv[1]);
...we first assign to it here, and if we use it uninit'd before the
compiler will tell us.
