Hi Junio,

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

> Johannes Schindelin <johannes.schinde...@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

Reply via email to