On Sat, 05 Jul 2014 10:18:05 -0300, David Bremner <david@tethera.net> wrote:
> Peter Wang <novalazy@gmail.com> writes:
> 
> > -    cleanup_path = tmppath;
> > -
> > -    if (! copy_stdin (fdin, fdout))
> > -	goto FAIL;
> > +    if (! copy_stdin (fdin, fdout)) {
> > +	close (fdout);
> > +	unlink (tmppath);
> > +	return FALSE;
> > +    }
> 
> I'm not completely convinced by replacement of the "goto FAIL" with the
> multiple returns.  I'd lean to towards being consistent with the notmuch
> codebase unless the FAIL block is really horrendous

Eh, when I came back to the code I found it unnecessary convoluted.
However, you can squash in the attached patch if you like.
As an objective measure, the function with the FAIL block is longer.

> 
> Is there a good reason to use TRUE and FALSE for return values rather
> than EXIT_SUCCESS and EXIT_FAILURE? It seems like the latter would be
> overall slightly simpler in notmuch_insert_command.

Not sure what you have in mind.  I think CLI exit codes should be
confined to notmuch_insert_command.

Peter