cc/2b44e56cbcd703632f38c9929724af5d202371

   1 Return-Path: <novalazy@gmail.com>\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 428F1431FB6\r
   6         for <notmuch@notmuchmail.org>; Sat,  5 Jul 2014 20:57:44 -0700 (PDT)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: -0.799\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-0.799 tagged_above=-999 required=5\r
  12         tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1,\r
  13         FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
  14 Received: from olra.theworths.org ([127.0.0.1])\r
  15         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  16         with ESMTP id qs09UJcXr8Xq for <notmuch@notmuchmail.org>;\r
  17         Sat,  5 Jul 2014 20:57:37 -0700 (PDT)\r
  18 Received: from mail-pa0-f42.google.com (mail-pa0-f42.google.com\r
  19         [209.85.220.42]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
  20         (No client certificate requested)\r
  21         by olra.theworths.org (Postfix) with ESMTPS id DE0AF431FAF\r
  22         for <notmuch@notmuchmail.org>; Sat,  5 Jul 2014 20:57:36 -0700 (PDT)\r
  23 Received: by mail-pa0-f42.google.com with SMTP id lj1so3704622pab.1\r
  24         for <notmuch@notmuchmail.org>; Sat, 05 Jul 2014 20:57:36 -0700 (PDT)\r
  25 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113;\r
  26         h=date:message-id:from:to:cc:subject:in-reply-to:references\r
  27         :mime-version:content-type:content-disposition\r
  28         :content-transfer-encoding;\r
  29         bh=4zEpWlBODIKCL7+LnthhP5ZJlT/YM4xoRMvrncBiaDw=;\r
  30         b=ZvRhxU2Fz9kSXcPiwOTDmEvS/Wn5UtmuBo4lUgjBYzvFAnCqhnVNJ8XvcvAqi0KfCN\r
  31         0YYDpoivnpxOHk1l9YgANTq92TE0UUIetB7RTRcRrL7c6zzreYQgfVeqtjV15D19VkGx\r
  32         mqMLZeh1jkWdCwCtUbl6/iHHDq82dP0mY0tRa5G5aC6KHnPASAQhMaZRoGGTrGPwSgh5\r
  33         cfvAo/015b1jQYKtvYFgI0L00P27jnhEvhrUHj/NpFgTm5h0afNntBaEIrbcBg2LdFCi\r
  34         VV8rp/FzhdjnDTyhocalErCnrqzL6w83O+MG8y7JoWmDZxYvf2H9U9TDAFHftXpomq0w\r
  35         MqaA==\r
  36 X-Received: by 10.68.160.163 with SMTP id xl3mr20977648pbb.39.1404619055900;\r
  37         Sat, 05 Jul 2014 20:57:35 -0700 (PDT)\r
  38 Received: from localhost (215.42.233.220.static.exetel.com.au.\r
  39         [220.233.42.215])\r
  40         by mx.google.com with ESMTPSA id r3sm14819829pdd.8.2014.07.05.20.57.32\r
  41         for <multiple recipients>\r
  42         (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\r
  43         Sat, 05 Jul 2014 20:57:34 -0700 (PDT)\r
  44 Date: Sun, 6 Jul 2014 13:57:28 +1000\r
  45 Message-ID: <20140706135728.GE1142@hili.localdomain>\r
  46 From: Peter Wang <novalazy@gmail.com>\r
  47 To: David Bremner <david@tethera.net>\r
  48 Subject: Re: [PATCH v2 06/10] cli: refactor insert\r
  49 In-Reply-To: <87simgq702.fsf@maritornes.cs.unb.ca>\r
  50 References: <1397653165-15620-1-git-send-email-novalazy@gmail.com>\r
  51         <1397653165-15620-7-git-send-email-novalazy@gmail.com>\r
  52         <87simgq702.fsf@maritornes.cs.unb.ca>\r
  53 MIME-Version: 1.0\r
  54 Content-Type: multipart/mixed; boundary="uhW2wu+cthCu47Lg"\r
  55 Content-Disposition: inline\r
  56 Content-Transfer-Encoding: 8bit\r
  57 Cc: notmuch@notmuchmail.org\r
  58 X-BeenThere: notmuch@notmuchmail.org\r
  59 X-Mailman-Version: 2.1.13\r
  60 Precedence: list\r
  61 List-Id: "Use and development of the notmuch mail system."\r
  62         <notmuch.notmuchmail.org>\r
  63 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  64         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  65 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  66 List-Post: <mailto:notmuch@notmuchmail.org>\r
  67 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  68 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  69         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  70 X-List-Received-Date: Sun, 06 Jul 2014 03:57:44 -0000\r
  71 \r
  72 \r
  73 --uhW2wu+cthCu47Lg\r
  74 Content-Type: text/plain; charset=utf-8\r
  75 Content-Disposition: inline\r
  76 Content-Transfer-Encoding: 8bit\r
  77 \r
  78 On Sat, 05 Jul 2014 10:18:05 -0300, David Bremner <david@tethera.net> wrote:\r
  79 > Peter Wang <novalazy@gmail.com> writes:\r
  80 > \r
  81 > > -    cleanup_path = tmppath;\r
  82 > > -\r
  83 > > -    if (! copy_stdin (fdin, fdout))\r
  84 > > -   goto FAIL;\r
  85 > > +    if (! copy_stdin (fdin, fdout)) {\r
  86 > > +   close (fdout);\r
  87 > > +   unlink (tmppath);\r
  88 > > +   return FALSE;\r
  89 > > +    }\r
  90 > \r
  91 > I'm not completely convinced by replacement of the "goto FAIL" with the\r
  92 > multiple returns.  I'd lean to towards being consistent with the notmuch\r
  93 > codebase unless the FAIL block is really horrendous\r
  94 \r
  95 Eh, when I came back to the code I found it unnecessary convoluted.\r
  96 However, you can squash in the attached patch if you like.\r
  97 As an objective measure, the function with the FAIL block is longer.\r
  98 \r
  99 > \r
 100 > Is there a good reason to use TRUE and FALSE for return values rather\r
 101 > than EXIT_SUCCESS and EXIT_FAILURE? It seems like the latter would be\r
 102 > overall slightly simpler in notmuch_insert_command.\r
 103 \r
 104 Not sure what you have in mind.  I think CLI exit codes should be\r
 105 confined to notmuch_insert_command.\r
 106 \r
 107 Peter\r
 108 \r
 109 --uhW2wu+cthCu47Lg\r
 110 Content-Type: text/x-diff; charset=utf-8\r
 111 Content-Disposition: attachment; filename="0001-goto-fail.patch"\r
 112 Content-Transfer-Encoding: 8bit\r
 113 \r
 114 >From be07c53d6d5f22ced723a44f996323d2a982113e Mon Sep 17 00:00:00 2001\r
 115 From: Peter Wang <novalazy@gmail.com>\r
 116 Date: Sun, 6 Jul 2014 12:52:26 +1000\r
 117 Subject: [PATCH] goto fail\r
 118 \r
 119 ---\r
 120  notmuch-insert.c | 32 ++++++++++++++++++--------------\r
 121  1 file changed, 18 insertions(+), 14 deletions(-)\r
 122 \r
 123 diff --git a/notmuch-insert.c b/notmuch-insert.c\r
 124 index 7db4f73..8dfc8bb 100644\r
 125 --- a/notmuch-insert.c\r
 126 +++ b/notmuch-insert.c\r
 127 @@ -342,26 +342,25 @@ write_message (void *ctx, int fdin, const char *dir, char **newpath)\r
 128  {\r
 129      char *tmppath;\r
 130      char *newdir;\r
 131 +    char *cleanup_path;\r
 132      int fdout;\r
 133  \r
 134      fdout = maildir_open_tmp_file (ctx, dir, &tmppath, newpath, &newdir);\r
 135      if (fdout < 0)\r
 136         return FALSE;\r
 137  \r
 138 -    if (! copy_stdin (fdin, fdout)) {\r
 139 -       close (fdout);\r
 140 -       unlink (tmppath);\r
 141 -       return FALSE;\r
 142 -    }\r
 143 +    cleanup_path = tmppath;\r
 144 +\r
 145 +    if (! copy_stdin (fdin, fdout))\r
 146 +       goto FAIL;\r
 147  \r
 148      if (fsync (fdout) != 0) {\r
 149         fprintf (stderr, "Error: fsync failed: %s\n", strerror (errno));\r
 150 -       close (fdout);\r
 151 -       unlink (tmppath);\r
 152 -       return FALSE;\r
 153 +       goto FAIL;\r
 154      }\r
 155  \r
 156      close (fdout);\r
 157 +    fdout = -1;\r
 158  \r
 159      /* Atomically move the new message file from the Maildir 'tmp' directory\r
 160       * to the 'new' directory.  We follow the Dovecot recommendation to\r
 161 @@ -370,16 +369,21 @@ write_message (void *ctx, int fdin, const char *dir, char **newpath)\r
 162       */\r
 163      if (rename (tmppath, *newpath) != 0) {\r
 164         fprintf (stderr, "Error: rename() failed: %s\n", strerror (errno));\r
 165 -       unlink (tmppath);\r
 166 -       return FALSE;\r
 167 +       goto FAIL;\r
 168      }\r
 169  \r
 170 -    if (! sync_dir (newdir)) {\r
 171 -       unlink (*newpath);\r
 172 -       return FALSE;\r
 173 -    }\r
 174 +    cleanup_path = *newpath;\r
 175 +\r
 176 +    if (! sync_dir (newdir))\r
 177 +       goto FAIL;\r
 178  \r
 179      return TRUE;\r
 180 +\r
 181 +  FAIL:\r
 182 +    if (fdout >= 0)\r
 183 +       close (fdout);\r
 184 +    unlink (cleanup_path);\r
 185 +    return FALSE;\r
 186  }\r
 187  \r
 188  int\r
 189 -- \r
 190 1.8.4\r
 191 \r
 192 \r
 193 --uhW2wu+cthCu47Lg--\r