[PATCH v2 06/10] cli: refactor insert

Peter Wang novalazy at gmail.com
Sat Jul 5 20:57:28 PDT 2014


On Sat, 05 Jul 2014 10:18:05 -0300, David Bremner <david at tethera.net> wrote:
> Peter Wang <novalazy at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-goto-fail.patch
Type: text/x-diff
Size: 1806 bytes
Desc: not available
URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20140706/5cdb4025/attachment.patch>


More information about the notmuch mailing list