Re: [PATCH v5 00/11] Deprecate .git/info/grafts

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Apr 27, 2018, 21:03 UTC
Message-ID: <nycvar.QRO.7.76.6.1804272258380.72@tvgsbejvaqbjf.bet>
In-Reply-To: <xmqqin8e38wf.fsf@gitster-ct.c.googlers.com>

Hi Junio,

On Thu, 26 Apr 2018, Junio C Hamano wrote:

Show 20 quoted lines

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> >  -	if (export_object(&old_oid, type, raw, tmpfile))
> >  -		return -1;
> >  -	if (launch_editor(tmpfile, NULL, NULL) < 0)
> >  -		return error("editing object file failed");
> >  -	if (import_object(&new_oid, type, raw, tmpfile))
> >  +	tmpfile = git_pathdup("REPLACE_EDITOBJ");
> >  +	if (export_object(&old_oid, type, raw, tmpfile) ||
> >  +	    (launch_editor(tmpfile, NULL, NULL) < 0 &&
> >  +	     error("editing object file failed")) ||
> >  +	    import_object(&new_oid, type, raw, tmpfile)) {
> >  +		free(tmpfile);
> >   		return -1;
> >  -
> >  +	}
> 
> I know the above is to avoid leaking tmpfile, but a single if ()
> condition that makes multiple calls to functions primarily for their
> side effects is too ugly to live.

I changed it back to individual conditional blocks, with every single one of them having their own free(tmpfile). That is at least clearer.

Ciao, Dscho