Re: [PATCH 04/18] insert: copy stdin to Maildir tmp file

Subject: Re: [PATCH 04/18] insert: copy stdin to Maildir tmp file

Date: Mon, 26 Nov 2012 18:21:21 +0200

To: Peter Wang

Cc: notmuch@notmuchmail.org

From: Ali Polatel


On Wed, Jul 25, 2012 at 11:42:33PM +1000, Peter Wang wrote:
>Read the new message from standard input into the Maildir tmp file.
>---
> notmuch-insert.c |   51 +++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 files changed, 49 insertions(+), 2 deletions(-)
>
>diff --git a/notmuch-insert.c b/notmuch-insert.c
>index f01a6f2..340f7e4 100644
>--- a/notmuch-insert.c
>+++ b/notmuch-insert.c
>@@ -75,21 +75,68 @@ maildir_open_tmp (void *ctx, const char *dir, char **tmppath, char **newpath)
> }
>
> static notmuch_bool_t
>+copy_fd_data (int fdin, int fdout)
>+{
>+    char buf[4096];
>+    char *p;
>+    ssize_t remain;
>+    ssize_t written;
>+
>+    for (;;) {
>+	remain = read (fdin, buf, sizeof(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;
>+	}
>+
>+	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;
>+	    }
>+	    p += written;
>+	    remain -= written;
>+	} while (remain > 0);
>+    }
>+
>+    return TRUE;
>+}

LGTM. As an optimisation we can also consider using the splice(2) system
call if it is available (as notmuch-deliver does). In case splice()
fails with ENOSYS or EINVAL we can fall back to this method.

I can write the patch for it once this is accepted.
Thanks for the good work!

>+static notmuch_bool_t
> insert_message (void *ctx, notmuch_database_t *notmuch, int fdin,
> 		const char *dir)
> {
>     char *tmppath;
>     char *newpath;
>     int fdout;
>+    notmuch_bool_t ret;
>
>     fdout = maildir_open_tmp (ctx, dir, &tmppath, &newpath);
>     if (fdout < 0) {
> 	return FALSE;
>     }
>
>+    ret = copy_fd_data (fdin, fdout);
>+
>     close (fdout);
>-    unlink (tmppath);
>-    return FALSE;
>+
>+    if (!ret) {
>+	unlink (tmppath);
>+    }
>+
>+    return ret;
> }
>
> int
>-- 
>1.7.4.4
>
>_______________________________________________
>notmuch mailing list
>notmuch@notmuchmail.org
>http://notmuchmail.org/mailman/listinfo/notmuch
part-000.sig (application/pgp-signature)

Thread: