Re: [PATCH] pretend_sha1_file(): Change return type from int to void

[Tobias Klauser <tklauser@distanz.ch>]

On 2015-10-07 at 23:22:59 +0200, Junio C Hamano <gitster@pobox.com> wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > As to the patch, I cannot speak for Junio, of course, but my
> > preference would be to keep the return type. Traditionally, functions
> > that can fail either die() or return an int; non-zero indicates an
> > error. In this case, it seems that we do not have any condition
> > (yet...) under which an error could occur. It does not seem very
> > unlikely that we may eventually have such conditions, though, hence my
> > preference.
> 
> Perhaps the attached is a better approach.
> 
> Even though the current implementation of "pretend" implementation
> does not, future generations are allowed to make pretend_sha1_file()
> return failure when appropriate.

For my original patch I didn't consider that pretend_sha1_file() might
return failure in the future. I was just confused by the fact that the
return value was seemingly useless (but now I realize that unused !=
useless ;-), sorry for the noise.

Please disregard my patch and apply yours instead, if you see fit.

> 
>  builtin/blame.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 203a981..fa24f8f 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -2362,7 +2362,8 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,
>  	convert_to_git(path, buf.buf, buf.len, &buf, 0);
>  	origin->file.ptr = buf.buf;
>  	origin->file.size = buf.len;
> -	pretend_sha1_file(buf.buf, buf.len, OBJ_BLOB, origin->blob_sha1);
> +	if (pretend_sha1_file(buf.buf, buf.len, OBJ_BLOB, origin->blob_sha1))
> +		die("failed to create a fake commit for the working tree version.");
>  
>  	/*
>  	 * Read the current index, replace the path entry with
>