Re: [PATCH v5 3/4] builtin/stash: provide a way to export stashes to a ref

"brian m. carlson" [off-list ref] writes:

+static int write_commit_with_parents(struct repository *r,
+				     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;
+	struct ident_split id;
+
+	this = lookup_commit_reference(r, oid);
+	buffer = repo_get_commit_buffer(r, this, &bufsize);
+	orig_author = find_commit_header(buffer, "author", &author_len);
+	orig_committer = find_commit_header(buffer, "committer", &committer_len);
+	if (split_ident_line(&id, orig_author, author_len) < 0 ||
+	    split_ident_line(&id, orig_committer, committer_len) < 0) {
+		ret = error(_("invalid author or committer for %s"), oid_to_hex(oid));
+		goto out;
+	}
+	p = strstr(buffer, "\n\n");
+
+	if (!orig_author || !orig_committer || !p) {
+		ret = error(_("cannot parse commit %s"), oid_to_hex(oid));
+		goto out;
+	}

It is too late to check the NULL-ness of orig_author and
orig_committer here.  They have already been used without checking
for their NULL-ness to call split_ident_line() that happily will
dereference its second "const char *line" parameter, so we would
have already segfaulted.

As fsck.c::verify_headers() say, it is not a crime to lack the
"\n\n" after the last header item, if the commit truly lacks any
message.  So '!p' is a bit overly strict, but in practice I do not
think our tools saved a byte by omitting the empty line after the
header even when creating a commit with an empty message for a long
time, so this may be OK.  On the other hand, preparing for a stash
entry a third-party reimplementation prepared would not be too hard
to do here.

	if (!orig_author || !orig_committer) {
		ret = error(_("cannot parse..."));
		goto out;
	}
	if (split_ident_line(...) < 0 ||
	    split_ident_line(...) < 0) {
		ret = error(_("invalid au..."));
		goto out;
	}

	p = strstr(buffer, "\n\n");
	strbuf_addstr(&msg, "git stash: ");

	if (p)
		strbuf_add(&msg, p+2, bufsize - (p + 2 - buffer));
	strbuf_complete_line(&mesg);

or something.

+	/* Jump to message. */
+	p += 2;
+	strbuf_addstr(&msg, "git stash: ");
+	strbuf_add(&msg, p, bufsize - (p - buffer));