Hi Junio,

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

Johannes Schindelin [off-list ref] writes:

quoted

 -	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