10/170c39b9c0dc2fff2e414db447ec6956360d10

   1 Return-Path: <tomi.ollila@iki.fi>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5         by olra.theworths.org (Postfix) with ESMTP id E596D431FAF\r
   6         for <notmuch@notmuchmail.org>; Mon, 26 Nov 2012 01:39:46 -0800 (PST)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: 0\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none]\r
  12         autolearn=disabled\r
  13 Received: from olra.theworths.org ([127.0.0.1])\r
  14         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  15         with ESMTP id U3xKyyiXwLgQ for <notmuch@notmuchmail.org>;\r
  16         Mon, 26 Nov 2012 01:39:46 -0800 (PST)\r
  17 Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34])\r
  18         by olra.theworths.org (Postfix) with ESMTP id E08D9431FAE\r
  19         for <notmuch@notmuchmail.org>; Mon, 26 Nov 2012 01:39:45 -0800 (PST)\r
  20 Received: from guru.guru-group.fi (localhost [IPv6:::1])\r
  21         by guru.guru-group.fi (Postfix) with ESMTP id 353071000E5;\r
  22         Mon, 26 Nov 2012 11:39:42 +0200 (EET)\r
  23 From: Tomi Ollila <tomi.ollila@iki.fi>\r
  24 To: Peter Wang <novalazy@gmail.com>, notmuch@notmuchmail.org\r
  25 Subject: Re: [PATCH v2 05/20] insert: copy stdin to Maildir tmp file\r
  26 In-Reply-To: <1353806206-29133-6-git-send-email-novalazy@gmail.com>\r
  27 References: <1353806206-29133-1-git-send-email-novalazy@gmail.com>\r
  28         <1353806206-29133-6-git-send-email-novalazy@gmail.com>\r
  29 User-Agent: Notmuch/0.14+84~g8a199bf (http://notmuchmail.org) Emacs/24.2.1\r
  30         (x86_64-unknown-linux-gnu)\r
  31 X-Face: HhBM'cA~<r"^Xv\KRN0P{vn'Y"Kd;zg_y3S[4)KSN~s?O\"QPoL\r
  32         $[Xv_BD:i/F$WiEWax}R(MPS`^UaptOGD`*/=@\1lKoVa9tnrg0TW?"r7aRtgk[F\r
  33         !)g;OY^,BjTbr)Np:%c_o'jj,Z\r
  34 Date: Mon, 26 Nov 2012 11:39:41 +0200\r
  35 Message-ID: <m2txscitya.fsf@guru.guru-group.fi>\r
  36 MIME-Version: 1.0\r
  37 Content-Type: text/plain\r
  38 X-BeenThere: notmuch@notmuchmail.org\r
  39 X-Mailman-Version: 2.1.13\r
  40 Precedence: list\r
  41 List-Id: "Use and development of the notmuch mail system."\r
  42         <notmuch.notmuchmail.org>\r
  43 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  44         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  45 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  46 List-Post: <mailto:notmuch@notmuchmail.org>\r
  47 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  48 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  49         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  50 X-List-Received-Date: Mon, 26 Nov 2012 09:39:47 -0000\r
  51 \r
  52 On Sun, Nov 25 2012, Peter Wang <novalazy@gmail.com> wrote:\r
  53 \r
  54 > Read the new message from standard input into the Maildir tmp file.\r
  55 > ---\r
  56 \r
  57 There are a few issues that gort my attention in this particular function:\r
  58 \r
  59 \r
  60 >  notmuch-insert.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++----\r
  61 >  1 file changed, 47 insertions(+), 4 deletions(-)\r
  62 >\r
  63 > diff --git a/notmuch-insert.c b/notmuch-insert.c\r
  64 > index 371fb47..88e8533 100644\r
  65 > --- a/notmuch-insert.c\r
  66 > +++ b/notmuch-insert.c\r
  67 > @@ -94,6 +94,47 @@ maildir_open_tmp_file (void *ctx, const char *dir,\r
  68 >      return fd;\r
  69 >  }\r
  70 >  \r
  71 > +/* Copy the contents of fdin into fdout. */\r
  72 > +static notmuch_bool_t\r
  73 > +copy_fd_data (int fdin, int fdout)\r
  74 > +{\r
  75 > +    char buf[4096];\r
  76 \r
  77 Copying in 4k blocks is slow when at least when doing file to file copy.\r
  78 Also socket buffers can often hold much more data. When reading from\r
  79 network and saving to file (in low-load machine) this is OK, but otherwise\r
  80 something like 64k buffer works better(*).\r
  81 \r
  82 (*) Now that I said it I have to measure this yet another time ;)\r
  83 \r
  84 > +    char *p;\r
  85 > +    ssize_t remain;\r
  86 > +    ssize_t written;\r
  87 > +\r
  88 > +    for (;;) {\r
  89 > +     remain = read (fdin, buf, sizeof(buf));\r
  90 \r
  91 space between sizeof and (buf)\r
  92 \r
  93 > +     if (remain == 0)\r
  94 > +         break;\r
  95 > +     if (remain < 0) {\r
  96 > +         if (errno == EINTR)\r
  97 > +             continue;\r
  98 > +         fprintf (stderr, "Error: reading from standard input: %s\n",\r
  99 > +                  strerror (errno));\r
 100 > +         return FALSE;\r
 101 > +     }\r
 102 \r
 103 You're claiming in function name & and its description that this is more\r
 104 "generic" copy function -- yet error message speaks about 'standard input'.\r
 105 \r
 106 > +\r
 107 > +     p = buf;\r
 108 > +     do {\r
 109 > +         written = write (fdout, p, remain);\r
 110 > +         if (written == 0)\r
 111 > +             return FALSE;\r
 112 > +         if (written < 0) {\r
 113 > +             if (errno == EINTR)\r
 114 > +                 continue;\r
 115 > +             fprintf (stderr, "Error: writing to temporary file: %s",\r
 116 > +                      strerror (errno));\r
 117 > +             return FALSE;\r
 118 > +         }\r
 119 \r
 120 Ditto.\r
 121 \r
 122 > +         p += written;\r
 123 > +         remain -= written;\r
 124 > +     } while (remain > 0);\r
 125 > +    }\r
 126 > +\r
 127 > +    return TRUE;\r
 128 > +}\r
 129 > +\r
 130 \r
 131 Tomi\r