Re: [PATCH] clone: add remote.cloneDefault config option

> >  static int git_clone_config(const char *k, const char *v, void *cb)
> >  {
> > +	if (!strcmp(k, "clone.defaultremotename")) {
> > +		if (remote_name != default_remote_name)
> > +			free(remote_name);
> > +		remote_name = xstrdup(v);
> 
> This feels strange.  The usual arrangement is
> 
>     - initialize the variable to NULL (or any value that the code
>       can tell that nobody touched it);
> 
>     - let git_config() callback to update the variable, taking care
>       of freeing and strduping as needed.  Note that free(NULL) is
>       kosher.
> 
>     - let parse_options() to further update the variable, taking
>       care of freeing and strduping as needed.
> 
>     - finally, if the variable is still NULL, give it its default
>       value.
> 
> so there is no room for the "if the variable has the value of the
> fallback default, do things differently" logic to be in the
> git_config() callback function.

> > @@ -1153,10 +1154,20 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
> >  
> >  	/*
> >  	 * re-read config after init_db and write_config to pick up any config
> > -	 * injected by --template and --config, respectively
> > +	 * injected by --template and --config, respectively.
> >  	 */
> 
> Squash this "oops, I forgot to finish the sentence" to the step the
> mistake was introduced, i.e. "use more conventional..."