git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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.

Previous: Sean Allred via GitGitGadgetNext: Sean Allred
Message 8 of 15 in “Improve consistency of git-var”
  1. 0/3 Improve consistency of git-varSean Allred via GitGitGadget, Nov 24, 2022
  2. 1/3 var: do not print usage() with a correct invocationSean Allred via GitGitGadget, Nov 24, 2022
  3. 2/3 var: remove read_varSean Allred via GitGitGadget, Nov 24, 2022
  4. Junio C HamanoNov 25, 2022
  5. 3/3 var: allow GIT_EDITOR to return nullSean Allred via GitGitGadget, Nov 24, 2022
  6. 0/2 Improve consistency of git-varSean Allred via GitGitGadget, Nov 25, 2022
  7. 1/2 var: do not print usage() with a correct invocationSean Allred via GitGitGadget, Nov 25, 2022
  8. Ævar Arnfjörð BjarmasonNov 25, 2022
  9. Sean AllredNov 26, 2022
  10. 2/2 var: allow GIT_EDITOR to return nullSean Allred via GitGitGadget, Nov 25, 2022
  11. Ævar Arnfjörð BjarmasonNov 25, 2022
  12. Sean AllredNov 26, 2022
  13. 0/2 Improve consistency of git-varSean Allred via GitGitGadget, Nov 26, 2022
  14. 1/2 var: do not print usage() with a correct invocationSean Allred via GitGitGadget, Nov 26, 2022
  15. 2/2 var: allow GIT_EDITOR to return nullSean Allred via GitGitGadget, Nov 26, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.