> > @@ -99,6 +94,15 @@ static int promisor_remote_config(const char *var, const char *value, void *data
> > size_t namelen;
> > const char *subkey;
> >
> > + if (!strcmp(var, "extensions.partialclone")) {
> > + /*
> > + * NULL value is handled in handle_extension_v0 in setup.c.
> > + */
> > + if (value)
> > + repository_format_partial_clone = xstrdup(value);
> > + return 0;
> > + }
>
> This is actually slightly hard to parse out. I was trying to figure
> out where repository_format_partial_clone was initialized, and it's
> not handled when value is NULL in handle_extension_v0; it's the fact
> that repository_format_partial_clone is declared a static global
> variable.
>
> But in the next patch you make it a member of struct
> promisor_remote_config, and instead rely on the xcalloc call in
> promisor_remote_init().
>
> That means everything is properly initialized and you haven't made any
> mistakes here, but the logic is a bit hard to follow. Perhaps it'd be
> nicer to just write this as
>
> + if (!strcmp(var, "extensions.partialclone")) {
> + repository_format_partial_clone = xstrdup_or_null(value);
> + return 0;
> + }
>
> which makes the code shorter and easier to follow, at least for me.
Hmm...is your concern about the case in which
repository_format_partial_clone is uninitialized, or about ignoring a
potential NULL value? If the former, I don't see how your suggestion
fixes things, since extensions.partialclone may never have been in the
config in the first place (and would thus leave
repository_format_partial_clone uninitialized, if it weren't for the
fact that it is in static storage and thus initialized to 0). If the
latter, I guess I should be more detailed about how it's being handled
in setup.c (or maybe just leave out the comment altogether - the code
here can handle a NULL repository_format_partial_clone for some reason).
