From: Karthik Nayak <karthik.188@gmail.com>
Date: Sun, 22 Feb 2026 20:15:41 GMT
Subject: Re: [PATCH v7 5/6] refs: allow reference location in refstorage config
Message-ID: <CAOLa=ZQsfOpP1cxFCjLWqbfxQ_upzuKHDojhNYtU=oFmeZsjVw@mail.gmail.com>
In-Reply-To: <xmqqbjhjz793.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

> Toon Claes <toon@iotcl.com> writes:
>
>>> +static void parse_reference_uri(const char *value, char **format,
>>> +				char **payload)
>>> +{
>>> +	const char *schema_end;
>>> +
>>> +	schema_end = strstr(value, "://");
>>> +	if (!schema_end) {
>>> +		*format = xstrdup(value);
>>> +		*payload = NULL;
>>> +	} else {
>>> +		*format = xstrndup(value, schema_end - value);
>>> +		*payload = xstrdup_or_null(schema_end + 3);
>>
>> Also here, why did you put the negated condition in the if clause?
>
> Hmph, would it make it easier to follow if you swap them?
>
> 	if (schema_end) {
> 		*format = xstrndup(value, schema_end - value);
> 		*payload = xstrdup_or_null(schema_end + 3);
> 	} else {
> 		*format = xstrdup(value);
> 		*payload = NULL;
> 	}
>
> Maybe it is just me, but I often find it easier to follow if the
> case that require shorter and/or simpler body, or the case that is
> narrower (e.g., error condition), comes first before the main logic.
> It is in line with preferring an early return on a more specific
> condition.  It frees readers from having to worry about these cases
> early and let them concentrate on what is expected to usually happen
> in the code.
>
> In this particular case, I do not know which one I would prefer,
> though.
>
> Thanks.

Kinda similar thought process. Since the URI format is new, the most
likely case here is that 'strstr' will not find a match. That's why the
negative case is first.

But I'd be happy to change, if others feel differently.

Karthik