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

Re: [PATCH v2 2/3] pretty.c: capture invalid trailer argument

From
Junio C Hamano <gitster@pobox.com>
Date
Jan 30, 2021, 00:01 UTC
Message-ID
<xmqqsg6jqomc.fsf@gitster.c.googlers.com>
In-Reply-To
<245e48eb6835cae4e61f65af780b766d990d4b5f.1611954543.git.gitgitgadget@gmail.com>
"Hariom Verma via GitGitGadget" <gitgitgadget@gmail.com> writes:
> +			size_t invalid_arg_len = strcspn(*arg, ",)");
> +			*invalid_arg = xstrndup(*arg, invalid_arg_len);
> +			return 1;
How about doing this only when invalid_arg is not NULL, i.e.
	} else if (!match_placeholder_bool_arg(....) &&
		   ...
        	   !match_placeholder_bool_arg(....)) {
		if (invalid_arg) {
			size_t len = strcspn(*arg, ",)");
			*invalid_arg = xstrndup(*arg, len);
		}
		return -1;
	}

Note that I just used 'len'; when the scope of a variable is so short, it is clear the length of what thing it refers to from the context, and there is no point in using a variable name that long.

In any case, by doing so, the callers that are not interested in the report can just pass NULL, which means ...

Show 5 quoted lines
> @@ -1464,12 +1472,13 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
>  		struct strbuf sepbuf = STRBUF_INIT;
>  		struct strbuf kvsepbuf = STRBUF_INIT;
>  		size_t ret = 0;
> +		char *unused = NULL;
... this will become unneeded, and ...
Show 7 quoted lines
>  		opts.no_divider = 1;
>  
>  		if (*arg == ':') {
>  			arg++;
> -			if (format_set_trailers_options(&opts, &filter_list, &sepbuf, &kvsepbuf, &arg))
> +			if (format_set_trailers_options(&opts, &filter_list, &sepbuf, &kvsepbuf, &arg, &unused))
>  				goto trailer_out;
... this will pass NULL, and ...
Show 7 quoted lines
>  		}
>  		if (*arg == ')') {
> @@ -1479,6 +1488,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
>  	trailer_out:
>  		string_list_clear(&filter_list, 0);
>  		strbuf_release(&sepbuf);
> +		free((char *)unused);
... this will become unneeded.
Show 16 quoted lines
>  		return ret;
>  	}
>  
> diff --git a/pretty.h b/pretty.h
> index 7369cf7e148..d902cdd70a9 100644
> --- a/pretty.h
> +++ b/pretty.h
> @@ -151,6 +151,7 @@ int format_set_trailers_options(struct process_trailer_options *opts,
>  			struct string_list *filter_list,
>  			struct strbuf *sepbuf,
>  			struct strbuf *kvsepbuf,
> -			const char **arg);
> +			const char **arg,
> +			char **invalid_arg);
>  
>  #endif /* PRETTY_H */
Previous: Hariom vermaNext: Hariom verma
Message 11 of 43 in “Unify trailers formatting logic for pretty.c and ref-filter.c”
  1. 0/2 Unify trailers formatting logic for pretty.c and ref-filter.cHariom Verma via GitGitGadget, Sep 5, 2020
  2. 1/2 pretty.c: refactor trailer logic to `format_set_trailers_options()`Hariom Verma via GitGitGadget, Sep 5, 2020
  3. René ScharfeSep 5, 2020
  4. 2/2 ref-filter: using pretty.c logic for trailersHariom Verma via GitGitGadget, Sep 5, 2020
  5. 0/3 Unify trailers formatting logic for pretty.c and ref-filter.cHariom Verma via GitGitGadget, Jan 29, 2021
  6. 1/3 pretty.c: refactor trailer logic to `format_set_trailers_options()`Hariom Verma via GitGitGadget, Jan 29, 2021
  7. Junio C HamanoJan 29, 2021
  8. 2/3 pretty.c: capture invalid trailer argumentHariom Verma via GitGitGadget, Jan 29, 2021
  9. Christian CouderJan 29, 2021
  10. Hariom vermaJan 30, 2021
  11. Junio C HamanoJan 30, 2021
  12. Hariom vermaJan 30, 2021
  13. Junio C HamanoJan 30, 2021
  14. Hariom vermaJan 30, 2021
  15. 3/3 ref-filter: use pretty.c logic for trailersHariom Verma via GitGitGadget, Jan 29, 2021
  16. Ævar Arnfjörð BjarmasonJan 30, 2021
  17. Hariom vermaFeb 4, 2021
  18. Ævar Arnfjörð BjarmasonFeb 4, 2021
  19. Junio C HamanoJan 30, 2021
  20. Junio C HamanoJan 30, 2021
  21. Hariom vermaJan 30, 2021
  22. Junio C HamanoJan 30, 2021
  23. 0/3 Unify trailers formatting logic for pretty.c and ref-filter.cHariom Verma via GitGitGadget, Feb 6, 2021
  24. 1/3 pretty.c: refactor trailer logic to `format_set_trailers_options()`Hariom Verma via GitGitGadget, Feb 6, 2021
  25. 2/3 pretty.c: capture invalid trailer argumentHariom Verma via GitGitGadget, Feb 6, 2021
  26. 3/3 ref-filter: use pretty.c logic for trailersHariom Verma via GitGitGadget, Feb 6, 2021
  27. Junio C HamanoFeb 7, 2021
  28. Hariom vermaFeb 7, 2021
  29. Junio C HamanoFeb 7, 2021
  30. Hariom vermaFeb 7, 2021
  31. Junio C HamanoFeb 7, 2021
  32. Hariom vermaFeb 8, 2021
  33. Junio C HamanoFeb 8, 2021
  34. Junio C HamanoFeb 7, 2021
  35. Junio C HamanoFeb 7, 2021
  36. 0/4 Unify trailers formatting logic for pretty.c and ref-filter.cHariom Verma via GitGitGadget, Feb 13, 2021
  37. 1/4 t6300: use function to test trailer optionsHariom Verma via GitGitGadget, Feb 13, 2021
  38. 2/4 pretty.c: refactor trailer logic to `format_set_trailers_options()`Hariom Verma via GitGitGadget, Feb 13, 2021
  39. 3/4 pretty.c: capture invalid trailer argumentHariom Verma via GitGitGadget, Feb 13, 2021
  40. 4/4 ref-filter: use pretty.c logic for trailersHariom Verma via GitGitGadget, Feb 13, 2021
  41. brian m. carlsonFeb 8, 2021
  42. brian m. carlsonFeb 9, 2021
  43. Junio C HamanoFeb 9, 2021

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.