From: Tomi Ollila <tomi.ollila@iki.fi>
Date: Mon, 26 Nov 2012 09:39:41 +0000 (+0200)
Subject: Re: [PATCH v2 05/20] insert: copy stdin to Maildir tmp file
X-Git-Url: http://git.tremily.us/gitweb.cgi?a=commitdiff_plain;h=0c1581c19650e633e9c09ede2e4d398ca730a75f;p=notmuch-archives.git

Re: [PATCH v2 05/20] insert: copy stdin to Maildir tmp file
---

diff --git a/10/170c39b9c0dc2fff2e414db447ec6956360d10 b/10/170c39b9c0dc2fff2e414db447ec6956360d10
new file mode 100644
index 000000000..7ad3a2b03
--- /dev/null
+++ b/10/170c39b9c0dc2fff2e414db447ec6956360d10
@@ -0,0 +1,131 @@
+Return-Path: <tomi.ollila@iki.fi>
+X-Original-To: notmuch@notmuchmail.org
+Delivered-To: notmuch@notmuchmail.org
+Received: from localhost (localhost [127.0.0.1])
+	by olra.theworths.org (Postfix) with ESMTP id E596D431FAF
+	for <notmuch@notmuchmail.org>; Mon, 26 Nov 2012 01:39:46 -0800 (PST)
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org
+X-Spam-Flag: NO
+X-Spam-Score: 0
+X-Spam-Level: 
+X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none]
+	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 U3xKyyiXwLgQ for <notmuch@notmuchmail.org>;
+	Mon, 26 Nov 2012 01:39:46 -0800 (PST)
+Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34])
+	by olra.theworths.org (Postfix) with ESMTP id E08D9431FAE
+	for <notmuch@notmuchmail.org>; Mon, 26 Nov 2012 01:39:45 -0800 (PST)
+Received: from guru.guru-group.fi (localhost [IPv6:::1])
+	by guru.guru-group.fi (Postfix) with ESMTP id 353071000E5;
+	Mon, 26 Nov 2012 11:39:42 +0200 (EET)
+From: Tomi Ollila <tomi.ollila@iki.fi>
+To: Peter Wang <novalazy@gmail.com>, notmuch@notmuchmail.org
+Subject: Re: [PATCH v2 05/20] insert: copy stdin to Maildir tmp file
+In-Reply-To: <1353806206-29133-6-git-send-email-novalazy@gmail.com>
+References: <1353806206-29133-1-git-send-email-novalazy@gmail.com>
+	<1353806206-29133-6-git-send-email-novalazy@gmail.com>
+User-Agent: Notmuch/0.14+84~g8a199bf (http://notmuchmail.org) Emacs/24.2.1
+	(x86_64-unknown-linux-gnu)
+X-Face: HhBM'cA~<r"^Xv\KRN0P{vn'Y"Kd;zg_y3S[4)KSN~s?O\"QPoL
+	$[Xv_BD:i/F$WiEWax}R(MPS`^UaptOGD`*/=@\1lKoVa9tnrg0TW?"r7aRtgk[F
+	!)g;OY^,BjTbr)Np:%c_o'jj,Z
+Date: Mon, 26 Nov 2012 11:39:41 +0200
+Message-ID: <m2txscitya.fsf@guru.guru-group.fi>
+MIME-Version: 1.0
+Content-Type: text/plain
+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: Mon, 26 Nov 2012 09:39:47 -0000
+
+On Sun, Nov 25 2012, Peter Wang <novalazy@gmail.com> wrote:
+
+> Read the new message from standard input into the Maildir tmp file.
+> ---
+
+There are a few issues that gort my attention in this particular function:
+
+
+>  notmuch-insert.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++----
+>  1 file changed, 47 insertions(+), 4 deletions(-)
+>
+> diff --git a/notmuch-insert.c b/notmuch-insert.c
+> index 371fb47..88e8533 100644
+> --- a/notmuch-insert.c
+> +++ b/notmuch-insert.c
+> @@ -94,6 +94,47 @@ maildir_open_tmp_file (void *ctx, const char *dir,
+>      return fd;
+>  }
+>  
+> +/* Copy the contents of fdin into fdout. */
+> +static notmuch_bool_t
+> +copy_fd_data (int fdin, int fdout)
+> +{
+> +    char buf[4096];
+
+Copying in 4k blocks is slow when at least when doing file to file copy.
+Also socket buffers can often hold much more data. When reading from
+network and saving to file (in low-load machine) this is OK, but otherwise
+something like 64k buffer works better(*).
+
+(*) Now that I said it I have to measure this yet another time ;)
+
+> +    char *p;
+> +    ssize_t remain;
+> +    ssize_t written;
+> +
+> +    for (;;) {
+> +	remain = read (fdin, buf, sizeof(buf));
+
+space between sizeof and (buf)
+
+> +	if (remain == 0)
+> +	    break;
+> +	if (remain < 0) {
+> +	    if (errno == EINTR)
+> +		continue;
+> +	    fprintf (stderr, "Error: reading from standard input: %s\n",
+> +		     strerror (errno));
+> +	    return FALSE;
+> +	}
+
+You're claiming in function name & and its description that this is more
+"generic" copy function -- yet error message speaks about 'standard input'.
+
+> +
+> +	p = buf;
+> +	do {
+> +	    written = write (fdout, p, remain);
+> +	    if (written == 0)
+> +		return FALSE;
+> +	    if (written < 0) {
+> +		if (errno == EINTR)
+> +		    continue;
+> +		fprintf (stderr, "Error: writing to temporary file: %s",
+> +			 strerror (errno));
+> +		return FALSE;
+> +	    }
+
+Ditto.
+
+> +	    p += written;
+> +	    remain -= written;
+> +	} while (remain > 0);
+> +    }
+> +
+> +    return TRUE;
+> +}
+> +
+
+Tomi