Re: [PATCH v4 11/16] am -3: use merge_recursive() directly again

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jul 26, 2016, 12:30 UTC
Message-ID: <alpine.DEB.2.20.1607261426340.14111@virtualbox>
In-Reply-To: <xmqqy44p2wds.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Mon, 25 Jul 2016, Junio C Hamano wrote:

Show 9 quoted lines

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > Note: the code now calls merge_recursive_generic() again. Unlike
> > merge_trees() and merge_recursive(), this function returns 0 upon success,
> > as most of Git's functions. Therefore, the error value -1 naturally is
> > handled correctly, and we do not have to take care of it specifically.
> 
> I've finished reading through up to this point and I'd stop for
> now.

If you want, I can break out the subsequent patches into a separate series. I just thought that you might want to have them here, as I implemented them in response to the concern you raised in a previous iteration of the same patch series: you pointed out that returning a negative error value still does not let the caller handle the error message, and with the subsequent patches that is now possible, too.

> Some of the patches I didn't look beyond the context presented in
> the patches, so it is very possible that I missed leaks caused by
> early returns and things like that, but I didn't see anything
> glaringly wrong.  Looks very promising.

Thanks.

I did try my best to catch all resource leaks, but I did stare at those patches so often and so long that it is easy for some obvious bug to have slipped by. Therefore, I would appreciate it if you (or somebody as diligent as you) could have a careful look in particular at the "merge-recursive: switch to returning errors instead of dying" patch.

I may have missed something as stupid as an unclosed file handle, after all.

Thank you, Dscho