[PATCH v7 03/12] cli: add insert command
Peter Wang
novalazy at gmail.com
Sun Jun 23 05:19:42 PDT 2013
On Sun, 23 Jun 2013 07:42:49 +0100, Mark Walters <markwalters1009 at 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
More information about the notmuch
mailing list