Re: [PATCH v2 7/7] merge-ort: convert more error() cases to path_msg()

From: Jeff King <peff@peff.net>
Date: Jul 2, 2024, 21:33 UTC
Message-ID: <20240702213326.GA591768@coredump.intra.peff.net>
In-Reply-To: <500433edf49a4df448b330e4ed9201cfac83cecf.1718766019.git.gitgitgadget@gmail.com>

On Wed, Jun 19, 2024 at 03:00:19AM +0000, Elijah Newren via GitGitGadget wrote:

Show 26 quoted lines

> +static int read_oid_strbuf(struct merge_options *opt,
> +			   const struct object_id *oid,
> +			   struct strbuf *dst,
> +			   const char *path)
>  {
>  	void *buf;
>  	enum object_type type;
>  	unsigned long size;
>  	buf = repo_read_object_file(the_repository, oid, &type, &size);
> -	if (!buf)
> -		return error(_("cannot read object %s"), oid_to_hex(oid));
> +	if (!buf) {
> +		path_msg(opt, ERROR_OBJECT_READ_FAILED, 0,
> +			 path, NULL, NULL, NULL,
> +			 _("error: cannot read object %s"), oid_to_hex(oid));
> +		return -1;
> +	}
>  	if (type != OBJ_BLOB) {
>  		free(buf);
> -		return error(_("object %s is not a blob"), oid_to_hex(oid));
> +		path_msg(opt, ERROR_OBJECT_NOT_A_BLOB, 0,
> +			 path, NULL, NULL, NULL,
> +			 _("error: object %s is not a blob"), oid_to_hex(oid));
>  	}
>  	strbuf_attach(dst, buf, size, size + 1);
>  	return 0;

This loses the early return in the "type != OBJ_BLOB" code path. So we free(buf), but then continue on to the strbuf_attach() call on the dangling pointer. Should it "return -1" like the earlier conditional?

-Peff