Re: [PATCH v2 1/2] var: do not print usage() with a correct invocation
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Nov 25, 2022, 22:45 UTC
- Message-ID
- <221125.86tu2mmz1e.gmgdl@evledraar.gmail.com>
- In-Reply-To
- <a7ff842a3e8d30cad7f18427bc812f542b998efc.1669395151.git.gitgitgadget@gmail.com>
On Fri, Nov 25 2022, Sean Allred via GitGitGadget wrote:
Show 7 quoted lines
> From: Sean Allred <allred.sean@gmail.com> > > 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"?
Show 7 quoted lines
> 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.
Show 12 quoted lines
> -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....
Show 11 quoted lines
> 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.