On Fri, 29 Mar 2013 19:59:56 -0400, David Bremner <david@tethera.net> wrote:
> 
> It took longer than I thought (of course) but I finally finished looking
> at the first 6 patches. 
> 
> I already mentioned a minor man page issue in a seperate message.
> 
> I took a second pass through 03/12, and I think I would prefer thethe
> control flow of insert_message be closer to the standard style in
> notmuch of using a return value variable and a single cleanup block at
> the end.  Reasonable people can disagree about issues of style, but in
> the end consistency of the code base is also important.
> 
> d

static notmuch_bool_t
insert_message (void *ctx, notmuch_database_t *notmuch, int fdin,
		const char *dir, tag_op_list_t *tag_ops)
{
    char *tmppath;
    char *newpath;
    char *newdir;
    int fdout;
    char *cleanup_path;

    fdout = maildir_open_tmp_file (ctx, dir, &tmppath, &newpath, &newdir);
    if (fdout < 0)
	return FALSE;

    cleanup_path = tmppath;

    if (! copy_stdin (fdin, fdout))
	goto DONE;

    if (fsync (fdout) != 0) {
	fprintf (stderr, "Error: fsync failed: %s\n", strerror (errno));
	goto DONE;
    }

    close (fdout);
    fdout = -1;

    /* Atomically move the new message file from the Maildir 'tmp' directory
     * to the 'new' directory.  We follow the Dovecot recommendation to
     * simply use rename() instead of link() and unlink().
     * See also: http://wiki.dovecot.org/MailboxFormat/Maildir#Mail_delivery
     */
    if (rename (tmppath, newpath) != 0) {
	fprintf (stderr, "Error: rename() failed: %s\n", strerror (errno));
	goto DONE;
    }

    cleanup_path = newpath;

    if (! add_file_to_database (notmuch, newpath, tag_ops)) {
	/* XXX add an option to keep the file in maildir? */
	goto DONE;
    }

    if (! sync_dir (newdir))
	goto DONE;

    cleanup_path = NULL; /* success */

  DONE:
    if (fdout >= 0)
	close (fdout);

    if (cleanup_path) {
	unlink (cleanup_path);
	return FALSE;
    }

    return TRUE;
}