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

Re: [PATCH v2] pack-objects: lazily set up "struct rev_info", don't leak

From
Derrick Stolee <derrickstolee@github.com>
Date
Mar 28, 2022, 15:58 UTC
Message-ID
<4781bae6-4ac6-1217-b018-aef286cdca7c@github.com>
In-Reply-To
<patch-v2-1.1-9951d92176e-20220328T154049Z-avarab@gmail.com>
On 3/28/2022 11:43 AM, Ævar Arnfjörð Bjarmason wrote:
> An earlier version of this commit[3] went behind
> opt_parse_list_objects_filter()'s back by faking up a "struct option"
> before calling it. Let's avoid that and instead create a blessed API
> for this pattern.
...
Show 51 quoted lines
> diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
> index f02d8df1422..4b25287886d 100644
> --- a/list-objects-filter-options.c
> +++ b/list-objects-filter-options.c
> @@ -285,6 +285,10 @@ int opt_parse_list_objects_filter(const struct option *opt,
>  				  const char *arg, int unset)
>  {
>  	struct list_objects_filter_options *filter_options = opt->value;
> +	opt_lof_init init = (opt_lof_init)opt->defval;
> +
> +	if (init)
> +		filter_options = init(opt->value);
>  
>  	if (unset || !arg)
>  		list_objects_filter_set_no_filter(filter_options);
> diff --git a/list-objects-filter-options.h b/list-objects-filter-options.h
> index 90e4bc96252..ffc02d77e76 100644
> --- a/list-objects-filter-options.h
> +++ b/list-objects-filter-options.h
> @@ -104,13 +104,31 @@ void parse_list_objects_filter(
>  	struct list_objects_filter_options *filter_options,
>  	const char *arg);
>  
> +/**
> + * The opt->value to opt_parse_list_objects_filter() is either a
> + * "struct list_objects_filter_option *" when using
> + * OPT_PARSE_LIST_OBJECTS_FILTER().
> + *
> + * Or, if using no "struct option" field is used by the callback,
> + * except the "defval" which is expected to be an "opt_lof_init"
> + * function, which is called with the "opt->value" and must return a
> + * pointer to the ""struct list_objects_filter_option *" to be used.
> + *
> + * The OPT_PARSE_LIST_OBJECTS_FILTER_INIT() can be used e.g. the
> + * "struct list_objects_filter_option" is embedded in a "struct
> + * rev_info", which the "defval" could be tasked with lazily
> + * initializing. See cmd_pack_objects() for an example.
> + */
>  int opt_parse_list_objects_filter(const struct option *opt,
>  				  const char *arg, int unset);
> +typedef struct list_objects_filter_options *(*opt_lof_init)(void *);
> +#define OPT_PARSE_LIST_OBJECTS_FILTER_INIT(fo, init) \
> +	{ OPTION_CALLBACK, 0, "filter", (fo), N_("args"), \
> +	  N_("object filtering"), 0, opt_parse_list_objects_filter, \
> +	  (intptr_t)(init) }
>  
>  #define OPT_PARSE_LIST_OBJECTS_FILTER(fo) \
> -	OPT_CALLBACK(0, "filter", fo, N_("args"), \
> -	  N_("object filtering"), \
> -	  opt_parse_list_objects_filter)
> +	OPT_PARSE_LIST_OBJECTS_FILTER_INIT((fo), NULL)

I like this way of combining the macros into one implementation (guarded with a NULL check).

This version looks good to me.

Thanks, -Stolee

Previous: Ævar Arnfjörð BjarmasonNext: Junio C Hamano
Message 24 of 25 in “Partial bundle follow ups”
  1. 0/5 Partial bundle follow upsDerrick Stolee via GitGitGadget, Mar 22, 2022
  2. 2/5 pack-objects: move revs out of get_object_list()Derrick Stolee via GitGitGadget, Mar 22, 2022
  3. 1/5 list-objects-filter: remove CL_ARG__FILTERDerrick Stolee via GitGitGadget, Mar 22, 2022
  4. 3/5 pack-objects: parse --filter directly into revs.filterDerrick Stolee via GitGitGadget, Mar 22, 2022
  5. Ramsay JonesMar 22, 2022
  6. Derrick StoleeMar 23, 2022
  7. Ævar Arnfjörð BjarmasonMar 22, 2022
  8. 5/5 bundle: output hash information in 'verify'Derrick Stolee via GitGitGadget, Mar 22, 2022
  9. 4/5 bundle: move capabilities to end of 'verify'Derrick Stolee via GitGitGadget, Mar 22, 2022
  10. Bagas SanjayaMar 23, 2022
  11. Derrick StoleeMar 23, 2022
  12. Junio C HamanoMar 23, 2022
  13. pack-objects: lazily set up "struct rev_info", don't leakÆvar Arnfjörð Bjarmason, Mar 25, 2022
  14. Derrick StoleeMar 25, 2022
  15. Ævar Arnfjörð BjarmasonMar 25, 2022
  16. Derrick StoleeMar 25, 2022
  17. Ævar Arnfjörð BjarmasonMar 25, 2022
  18. Derrick StoleeMar 25, 2022
  19. Ævar Arnfjörð BjarmasonMar 26, 2022
  20. Derrick StoleeMar 28, 2022
  21. Junio C HamanoMar 25, 2022
  22. Ævar Arnfjörð BjarmasonMar 26, 2022
  23. pack-objects: lazily set up "struct rev_info", don't leakÆvar Arnfjörð Bjarmason, Mar 28, 2022
  24. Derrick StoleeMar 28, 2022
  25. Junio C HamanoMar 28, 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.