Re: [PATCH 3/7] builtin/gc: fix leaking config values
- From
James Liu <james@jamesliu.io>
- Date
- Aug 15, 2024, 05:22 UTC
- Message-ID
- <D3G8EY3R09BC.2BKB6UND5J5MA@jamesliu.io>
- In-Reply-To
- <310e361371efc156c3aaac94439bdbeaa965155f.1723533091.git.ps@pks.im>
On Tue Aug 13, 2024 at 5:17 PM AEST, Patrick Steinhardt wrote:
Show 25 quoted lines
> 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.