Re: [PATCH v5 09/10] http: read HTTP WWW-Authenticate response headers

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Jan 12, 2023, 08:41 UTC
Message-ID: <230112.86wn5s2l5r.gmgdl@evledraar.gmail.com>
In-Reply-To: <bcfec529d9581db3181cc6f3b3d4b4b8d315ca0c.1673475190.git.gitgitgadget@gmail.com>

On Wed, Jan 11 2023, Matthew John Cheetham via GitGitGadget wrote:

Show 17 quoted lines

> From: Matthew John Cheetham <mjcheetham@outlook.com>
> [...]
> +		} else if (buf.len) {
> +			const char *prev = values->v[values->nr - 1];
> +			struct strbuf append = STRBUF_INIT;
> +			strbuf_addstr(&append, prev);
> +
> +			/* Join two non-empty values with a single space. */
> +			if (append.len)
> +				strbuf_addch(&append, ' ');
> +
> +			strbuf_addbuf(&append, &buf);
> +
> +			strvec_pop(values);
> +			strvec_push_nodup(values, strbuf_detach(&append, NULL));
> +		}
> +

I've written something like the strvec_push_nodup() patch that preceded this myself for similar reasons, and as recently noted in [1] I think such a thing (although I implemented a different interface) might be useful in general.

But this really doesn't seem like a good justification for adding this new API. Let's instead do:

	} else if (buf.len) {
		const char *prev = values->v[values->nr - 1];
		/* Join two non-empty values with a single space. */
		const char *const sp = *prev ? " " : ""

		strvec_pop(values);
		strvec_pushf(values, "%s%s%s", prev, sp, buf.buf);
	}

There may be cases where a public strvec_push_nodup() simplifies things, but this doesn't seem like such a case, just use strvec_pushf() directly instead, and skip the strbuf & strbuf_detach().

I haven't compiled/tested the above, so there may e.g. be a typo in there. But I think the general concept should work in this case.

1. https://lore.kernel.org/git/RFC-cover-0.5-00000000000-20221215T090226Z-avarab@gmail.com/