From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <novalazy@gmail.com>
Received: from localhost (localhost [127.0.0.1])
	by olra.theworths.org (Postfix) with ESMTP id 1F924431FB6
	for <notmuch@notmuchmail.org>; Sun, 23 Jun 2013 05:20:00 -0700 (PDT)
X-Virus-Scanned: Debian amavisd-new at olra.theworths.org
X-Spam-Flag: NO
X-Spam-Score: -0.799
X-Spam-Level: 
X-Spam-Status: No, score=-0.799 tagged_above=-999 required=5
	tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1,
	FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled
Received: from olra.theworths.org ([127.0.0.1])
	by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)
	with ESMTP id TgnN++6RnrOF for <notmuch@notmuchmail.org>;
	Sun, 23 Jun 2013 05:19:51 -0700 (PDT)
Received: from mail-pa0-f41.google.com (mail-pa0-f41.google.com
	[209.85.220.41]) (using TLSv1 with cipher RC4-SHA (128/128 bits))
	(No client certificate requested)
	by olra.theworths.org (Postfix) with ESMTPS id 2F58B431FAE
	for <notmuch@notmuchmail.org>; Sun, 23 Jun 2013 05:19:51 -0700 (PDT)
Received: by mail-pa0-f41.google.com with SMTP id bj3so9838371pad.0
	for <notmuch@notmuchmail.org>; Sun, 23 Jun 2013 05:19:50 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113;
	h=date:message-id:from:to:cc:subject:in-reply-to:references
	:mime-version:content-type:content-disposition
	:content-transfer-encoding;
	bh=ZW6UzJMwrRRJSXPcRbbiWxrZ+/TepwD+4SVexBmvwQQ=;
	b=qINvEgDsn5Ss7pmOy2YbNEETI3t+L2MuEt8sNB2Oam3/BlvMXtpUqKz88F7ZlTUbbt
	fT3pdIcw9amIjFoBJz0tcsxgGycAi0vEJABBz4NPaARtilglz6iQ1oMNx3tv08k+Ad2W
	8iN++1zrWhbbqKbdX0O1Xem+wOmNI44dGgHIrEKn6OkbB5k2dv4zBVfLuUs6J6dmzy7I
	WvJAxOGszZf15AGnW8f81acq0jOFQIt5PxlpX/8oM+fC0xJSI/IZaZY/Os5egT8DqbgR
	aOPgczQ2lHGF/LVAFZHglYldfAIrygdp2PM2wtpOXrsKli4jFUrDEz4rAdeQWI3xM+dM
	w48Q==
X-Received: by 10.66.160.97 with SMTP id xj1mr12932797pab.5.1371989988343;
	Sun, 23 Jun 2013 05:19:48 -0700 (PDT)
Received: from localhost (215.42.233.220.static.exetel.com.au.
	[220.233.42.215])
	by mx.google.com with ESMTPSA id ri8sm13336924pbc.3.2013.06.23.05.19.45
	for <multiple recipients>
	(version=TLSv1.2 cipher=RC4-SHA bits=128/128);
	Sun, 23 Jun 2013 05:19:47 -0700 (PDT)
Date: Sun, 23 Jun 2013 22:19:42 +1000
Message-ID: <20130623221942.GA18938@hili.localdomain>
From: Peter Wang <novalazy@gmail.com>
To: Mark Walters <markwalters1009@gmail.com>
Subject: Re: [PATCH v7 03/12] cli: add insert command
In-Reply-To: <8761x5uy3a.fsf@qmul.ac.uk>
References: <1371961445-15182-1-git-send-email-novalazy@gmail.com>
	<1371961445-15182-4-git-send-email-novalazy@gmail.com>
	<8761x5uy3a.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
Cc: notmuch@notmuchmail.org
X-BeenThere: notmuch@notmuchmail.org
X-Mailman-Version: 2.1.13
Precedence: list
List-Id: "Use and development of the notmuch mail system."
	<notmuch.notmuchmail.org>
List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,
	<mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>
List-Archive: <http://notmuchmail.org/pipermail/notmuch>
List-Post: <mailto:notmuch@notmuchmail.org>
List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>
List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,
	<mailto:notmuch-request@notmuchmail.org?subject=subscribe>
X-List-Received-Date: Sun, 23 Jun 2013 12:20:00 -0000

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