From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Wed, 13 Apr 2022 15:36:05 GMT
Subject: Re: [PATCH v4 3/4] builtin/stash: provide a way to export stashes to a ref
Message-ID: <220413.86ee21f0g9.gmgdl@evledraar.gmail.com>
In-Reply-To: <20220407215352.3491567-4-sandals@crustytoothpaste.net>


On Thu, Apr 07 2022, brian m. carlson wrote:

> +static int write_commit_with_parents(struct object_id *out, const struct object_id *oid, struct commit_list *parents)
> +{
> +	size_t author_len, committer_len;
> +	struct commit *this;
> +	const char *orig_author, *orig_committer;
> +	char *author = NULL, *committer = NULL;
> +	const char *buffer;
> +	unsigned long bufsize;
> +	const char *p;
> +	struct strbuf msg = STRBUF_INIT;
> +	int ret = 0;
> +
> +	this = lookup_commit_reference(the_repository, oid);
> +	buffer = get_commit_buffer(this, &bufsize);
> +	orig_author = find_commit_header(buffer, "author", &author_len);
> +	orig_committer = find_commit_header(buffer, "committer", &committer_len);

In builtin/am.c we also start with this, but follow it with passing the
data through split_ident_line(), any reason we're not doing the same
thing here, and are there cases wehre that would result in propagating
not-sanity-checked-enough data?


> +	p = memmem(buffer, bufsize, "\n\n", 2);
> +
> +	if (!orig_author || !orig_committer || !p) {
> +		ret = error(_("cannot parse commit %s"), oid_to_hex(oid));
> +		goto out;
> +	}
> +	/* Jump to message. */
> +	p += 2;
> +	strbuf_addstr(&msg, "git stash: ");
> +	strbuf_add(&msg, p, bufsize - (p - buffer));

And more on those behavior differences, the am.c version seems to not
care that we *might* have a \0 in a commit buffer and just uses
xstrdup(msg + 2).

But this one looks like it will faithfully carry that \0-containing
message forward.

Probably OK, just checking...