On Sun, 23 Jun 2013 07:42:49 +0100, Mark Walters <markwalters1009@gmail.com> wrote:
> 
> This is a +1 modulo one small bug (I think) I found below. I am happy to
> delay the fail-on-index-fail option, especially as that will need some
> bikeshedding on its name.
> 
> Also when posting new versions please include a diff from the previous
> version (this is more difficult if there was significant rebasing). This
> makew the v6 to v7 change obvious (the one comment change and the
> bugfix).
> 
> Moreover doing the diff with v4 (which I happen to have locally) I
> found the bug below.
> 
> Best wishes
> 
> Mark
> 
> 
> 
...
> > +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 FAIL;
> > +
> > +    if (fsync (fdout) != 0) {
> > +	fprintf (stderr, "Error: fsync failed: %s\n", strerror (errno));
> > +	goto FAIL;
> > +    }
> > +
> > +    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 FAIL;
> > +    }
> > +
> > +    if (! sync_dir (newdir))
> > +	goto FAIL;
> 
> I think cleanup_path needs to be updated before the sync_dir is test as
> newpath should be unlinked rather than oldpath. (v4 explicitly unlinked newpath)
> 

Thanks for the continued close review.
I'll post a followup to this specific patch.

Peter