Re: [PATCH v2 1/5] am: don't die in read_author_script()

From: Junio C Hamano <gitster@pobox.com>
To: Phillip Wood <phillip.wood@talktalk.net>
Cc: Git Mailing List <git@vger.kernel.org>,
	Eric Sunshine <sunshine@sunshineco.com>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Phillip Wood <phillip.wood@dunelm.org.uk>
Subject: Re: [PATCH v2 1/5] am: don't die in read_author_script()
Date: Thu, 25 Oct 2018 17:44:13 +0900	[thread overview]
Message-ID: <xmqq5zxqiffm.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20181018100023.7327-2-phillip.wood@talktalk.net> (Phillip Wood's message of "Thu, 18 Oct 2018 11:00:19 +0100")

Phillip Wood <phillip.wood@talktalk.net> writes:

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> The caller is already prepared to handle errors returned from this
> function so there is no need for it to die if it cannot read the file.
>
> Suggested-by: Eric Sunshine <sunshine@sunshineco.com>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>  builtin/am.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

OK.  Having fewer die's in reusable codepath can only be a good
thing.

>
> diff --git a/builtin/am.c b/builtin/am.c
> index 5e866d17c7..b68578bc3f 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -318,7 +318,8 @@ static int read_author_script(struct am_state *state)
>  	if (fd < 0) {
>  		if (errno == ENOENT)
>  			return 0;
> -		die_errno(_("could not open '%s' for reading"), filename);
> +		return error_errno(_("could not open '%s' for reading"),
> +				   filename);
>  	}
>  	strbuf_read(&buf, fd, 0);
>  	close(fd);