Re: [PATCH 3/7] builtin/gc: fix leaking config values

["James Liu" <james@jamesliu.io>]

On Tue Aug 13, 2024 at 5:17 PM AEST, Patrick Steinhardt wrote:
> Note that there is one small gotcha here with the "--prune" option. Next
> to passing a string, this option also accepts the "--no-prune" option
> that overrides the default or configured value. We thus need to discern
> between the option not having been passed by the user and the negative
> variant of it. This is done by using a simple sentinel value that lets
> us discern these cases.
>
> @@ -644,12 +673,15 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
>  	struct child_process rerere_cmd = CHILD_PROCESS_INIT;
>  	struct maintenance_run_opts opts = {0};
>  	struct gc_config cfg = GC_CONFIG_INIT;
> +	const char *prune_expire_sentinel = "sentinel";
> +	const char *prune_expire_arg = prune_expire_sentinel;
> +	int ret;
>  
>  	struct option builtin_gc_options[] = {
>  		OPT__QUIET(&quiet, N_("suppress progress reporting")),
> -		{ OPTION_STRING, 0, "prune", &cfg.prune_expire, N_("date"),
> +		{ OPTION_STRING, 0, "prune", &prune_expire_arg, N_("date"),
>  			N_("prune unreferenced objects"),
> -			PARSE_OPT_OPTARG, NULL, (intptr_t)cfg.prune_expire },
> +			PARSE_OPT_OPTARG, NULL, (intptr_t)prune_expire_arg },
>  		OPT_BOOL(0, "cruft", &cfg.cruft_packs, N_("pack unreferenced objects separately")),
>  		OPT_MAGNITUDE(0, "max-cruft-size", &cfg.max_cruft_size,
>  			      N_("with --cruft, limit the size of new cruft packs")),

I was wondering how the `no-*` options worked since they're not
explicitly defined in the `builtin_gc_options` array. I guess
they're handled internally by `parse_options()`, and if the `no-` prefix
was present, it leaves that argument unset.