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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2015-10-07 at 23:22:59 +0200, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Johannes Schindelin <johannes.schindelin@xxxxxx> 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
> 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]