Re: [PATCH v7 5/6] refs: allow reference location in refstorage config

Junio C Hamano [off-list ref] writes:

Toon Claes [off-list ref] writes:

quoted

quoted

+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