Re: [PATCH v4 06/10] list-objects-filter-options: make filter_spec a string_list

From: Jonathan Tan <jonathantanmy@google.com>
Date: Jun 22, 2019, 00:37 UTC
Message-ID: <20190622003713.248581-1-jonathantanmy@google.com>
In-Reply-To: <1a95dd91927973038c3d59bc3215556e448f0e63.1560558910.git.matvore@google.com>

Patch 5 and this patch look good to me.

Show 21 quoted lines

> @@ -1134,27 +1134,25 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
>  		transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, "1");
>  
>  	if (option_upload_pack)
>  		transport_set_option(transport, TRANS_OPT_UPLOADPACK,
>  				     option_upload_pack);
>  
>  	if (server_options.nr)
>  		transport->server_options = &server_options;
>  
>  	if (filter_options.choice) {
> -		struct strbuf expanded_filter_spec = STRBUF_INIT;
> -		expand_list_objects_filter_spec(&filter_options,
> -						&expanded_filter_spec);
> +		const char *spec =
> +			expand_list_objects_filter_spec(&filter_options);
>  		transport_set_option(transport, TRANS_OPT_LIST_OBJECTS_FILTER,
> -				     expanded_filter_spec.buf);
> +				     spec);
>  		transport_set_option(transport, TRANS_OPT_FROM_PROMISOR, "1");
> -		strbuf_release(&expanded_filter_spec);

So expand_list_objects_filter_spec() now returns a filter_options-owned string (instead of previously writing to a strbuf), which is why we no longer need to do any freeing or releasing. That makes sense. (Same for the other call sites.)

Show 13 quoted lines

> @@ -177,72 +177,89 @@ static int parse_combine_filter(
>  	}
>  	return result;
>  }
>  
>  int parse_list_objects_filter(struct list_objects_filter_options *filter_options,
>  			      const char *arg)
>  {
>  	struct strbuf buf = STRBUF_INIT;
>  	if (filter_options->choice)
>  		die(_("multiple filter-specs cannot be combined"));
> -	filter_options->filter_spec = strdup(arg);
> +	string_list_append(&filter_options->filter_spec, xstrdup(arg));

This append needs to be called with xstrdup, because a zero-initialized string list is NODUP. OK.