Re: [PATCH v4 06/11] bundle-uri client: add helper for testing server

[Derrick Stolee <derrickstolee@github.com>]

On 12/30/22 11:31 AM, Jeff King wrote:
> On Thu, Dec 22, 2022 at 03:14:12PM +0000, Ævar Arnfjörð Bjarmason via GitGitGadget wrote:
> 
>> +static int cmd_ls_remote(int argc, const char **argv)
>> +{
>> +	const char *uploadpack = NULL;
>> +	struct string_list server_options = STRING_LIST_INIT_DUP;
> 
> These two variables are initialized to NULL and empty respectively, and
> then not accessed until here:
> 
>> +	transport = transport_get(remote, NULL);
>> +	if (uploadpack)
>> +		transport_set_option(transport, TRANS_OPT_UPLOADPACK, uploadpack);
>> +	if (server_options.nr)
>> +		transport->server_options = &server_options;
> 
> where neither conditional will trigger, since they will still be NULL
> and empty.
> 
> Is this function missing some argv parsing that would affect these? Or
> if it's not needed, would we want to remove them, like:

...

> Not a huge deal, but I noticed that Coverity complained about the
> uploadpack one because this hit 'next' (the server_options one I found
> manually, but it was kind of obvious when looking at the other).

Yes, removing these lines would be fine. Perhaps there were
uses for these in an earlier version that were dropped. But
we can remove them now and then add them back when they
actually connect to functionality.

Thanks,
-Stolee