Re: [PATCH] pretend_sha1_file(): Change return type from int to void
- From
Tobias Klauser <tklauser@distanz.ch>
- Date
- Oct 8, 2015, 07:45 UTC
- Message-ID
- <20151008074526.GD11304@distanz.ch>
- In-Reply-To
- <xmqqa8rutlu4.fsf@gitster.mtv.corp.google.com>
On 2015-10-07 at 23:22:59 +0200, Junio C Hamano <gitster@pobox.com> wrote:
Show 15 quoted lines
> 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.
Show 19 quoted lines
>
> 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
>