67/6dacbd707f14a9e29aff3cae51f09397942637

   1 Return-Path: <jani@nikula.org>\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 041EC431FB6\r
   6         for <notmuch@notmuchmail.org>; Mon,  7 May 2012 14:14:19 -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.699\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-0.699 tagged_above=-999 required=5\r
  12         tests=[HTML_MESSAGE=0.001, RCVD_IN_DNSWL_LOW=-0.7] 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 97veg8Yqlq81 for <notmuch@notmuchmail.org>;\r
  16         Mon,  7 May 2012 14:14:17 -0700 (PDT)\r
  17 Received: from mail-pz0-f45.google.com (mail-pz0-f45.google.com\r
  18         [209.85.210.45]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
  19         (No client certificate requested)\r
  20         by olra.theworths.org (Postfix) with ESMTPS id 5297A431FAE\r
  21         for <notmuch@notmuchmail.org>; Mon,  7 May 2012 14:14:17 -0700 (PDT)\r
  22 Received: by dadv2 with SMTP id v2so1373526dad.18\r
  23         for <notmuch@notmuchmail.org>; Mon, 07 May 2012 14:14:16 -0700 (PDT)\r
  24 X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;\r
  25         d=google.com; s=20120113;\r
  26         h=mime-version:in-reply-to:references:date:message-id:subject:from:to\r
  27         :cc:content-type:x-gm-message-state;\r
  28         bh=1XJHml9Fai3ajFhy8njaKRybJ9bdn4wi71JTo2fkpHg=;\r
  29         b=mOYmGNPyWdZGVzSReOh3GMZmJndsYVkhoMrB1S9cG+Hra6QxwihmERdrYETMcZx9yR\r
  30         //BFaI/LgnRNktwIwjYP9uJ4mHnKlv6XChQP93ZtPACQtU6P285TnkF9iTa3b/5sn1Vg\r
  31         gXJXLM/aeohszPV7Is9G2PEI0S5jCZq/Tq7pjqPykI/2urUcLSuyZpVGiYPwFOiCVXor\r
  32         zPJackD7I/L/kSNHaMI40Hpf1uxzV1DW0QYRNVh9Bxi8bARWldpcysTCgx8Wni6c9FRK\r
  33         WFsDXetSlFNjOxyfHTREcueXbLW+ZOxiLGOdkbBDTMrrFchXdfrvjQsWe7DutSJUA7vc\r
  34         3AEw==\r
  35 MIME-Version: 1.0\r
  36 Received: by 10.68.236.5 with SMTP id uq5mr455946pbc.165.1336425256404; Mon,\r
  37         07 May 2012 14:14:16 -0700 (PDT)\r
  38 Received: by 10.68.52.33 with HTTP; Mon, 7 May 2012 14:14:16 -0700 (PDT)\r
  39 Received: by 10.68.52.33 with HTTP; Mon, 7 May 2012 14:14:16 -0700 (PDT)\r
  40 In-Reply-To:\r
  41  <CAB+hUn_rcz00p4kobjcf57aiyqSAWSPGgRSCNNG0b5eSye=3DQ@mail.gmail.com>\r
  42 References: <1336414186-15293-1-git-send-email-amdragon@mit.edu>\r
  43         <1336414186-15293-3-git-send-email-amdragon@mit.edu>\r
  44         <CAB+hUn_rcz00p4kobjcf57aiyqSAWSPGgRSCNNG0b5eSye=3DQ@mail.gmail.com>\r
  45 Date: Tue, 8 May 2012 00:14:16 +0300\r
  46 Message-ID:\r
  47  <CAB+hUn917qxCVDat64NWpc9r5+9g7yS3y4snkv_p8Oo6Vh7OyQ@mail.gmail.com>\r
  48 Subject: Re: [PATCH 2/2] new: Centralize file type stat-ing logic\r
  49 From: Jani Nikula <jani@nikula.org>\r
  50 To: Austin Clements <amdragon@mit.edu>\r
  51 Content-Type: multipart/alternative; boundary=047d7b33c9c6ad473d04bf78c08a\r
  52 X-Gm-Message-State:\r
  53  ALoCoQko9LcsJsIgWSM/hFYSIQSv26f8OUh8J4IapJWfjPERaQjv4mx9V6NmvBdVtDh3XV80rQPa\r
  54 Cc: Notmuch Mail <notmuch@notmuchmail.org>, Vladimir Marek <vlmarek@volny.cz>\r
  55 X-BeenThere: notmuch@notmuchmail.org\r
  56 X-Mailman-Version: 2.1.13\r
  57 Precedence: list\r
  58 List-Id: "Use and development of the notmuch mail system."\r
  59         <notmuch.notmuchmail.org>\r
  60 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  61         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  62 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  63 List-Post: <mailto:notmuch@notmuchmail.org>\r
  64 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  65 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  66         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  67 X-List-Received-Date: Mon, 07 May 2012 21:14:19 -0000\r
  68 \r
  69 --047d7b33c9c6ad473d04bf78c08a\r
  70 Content-Type: text/plain; charset=UTF-8\r
  71 \r
  72 On May 8, 2012 12:08 AM, "Jani Nikula" <jani@nikula.org> wrote:\r
  73 >\r
  74 >\r
  75 > On May 7, 2012 9:10 PM, "Austin Clements" <amdragon@mit.edu> wrote:\r
  76 > >\r
  77 > > This moves our logic to get a file's type into one function.  This has\r
  78 > > several benefits: we can support OSes and file systems that do not\r
  79 > > provide dirent.d_type or always return DT_UNKNOWN, complex\r
  80 > > symlink-handling logic has been replaced by a simple stat fall-through\r
  81 > > in one place, and the error message for un-stat-able file is more\r
  82 > > accurate (previously, the error always mentioned directories, even\r
  83 > > though a broken symlink is not a directory).\r
  84 >\r
  85 > Please find some quick drive-by-review below.\r
  86 \r
  87 Forgot to add that the general approach seems sensible to me.\r
  88 \r
  89 >\r
  90 > J.\r
  91 >\r
  92 > > ---\r
  93 > >  notmuch-new.c |   99\r
  94 ++++++++++++++++++++++++++++++++++-----------------------\r
  95 > >  test/new      |    2 +-\r
  96 > >  2 files changed, 60 insertions(+), 41 deletions(-)\r
  97 > >\r
  98 > > diff --git a/notmuch-new.c b/notmuch-new.c\r
  99 > > index cb720cc..cf2580e 100644\r
 100 > > --- a/notmuch-new.c\r
 101 > > +++ b/notmuch-new.c\r
 102 > > @@ -154,6 +154,44 @@ dirent_sort_strcmp_name (const struct dirent **a,\r
 103 const struct dirent **b)\r
 104 > >     return strcmp ((*a)->d_name, (*b)->d_name);\r
 105 > >  }\r
 106 > >\r
 107 > > +/* Return the type of a directory entry relative to path as a stat(2)\r
 108 > > + * mode.  Like stat, this follows symlinks.  Returns -1 and sets errno\r
 109 > > + * if the file's type cannot be determined (which includes dangling\r
 110 > > + * symlinks).\r
 111 > > + */\r
 112 > > +static int\r
 113 > > +dirent_type (const char *path, const struct dirent *entry)\r
 114 > > +{\r
 115 > > +    struct stat statbuf;\r
 116 > > +    char *abspath;\r
 117 > > +    int err;\r
 118 > > +\r
 119 > > +#ifdef _DIRENT_HAVE_D_TYPE\r
 120 > > +    /* Mapping from d_type to stat mode_t.  We omit DT_LNK so that\r
 121 > > +     * we'll fall through to stat and get the real file type. */\r
 122 > > +    static const mode_t modes[] = {\r
 123 > > +       [DT_BLK]  = S_IFBLK,\r
 124 > > +       [DT_CHR]  = S_IFCHR,\r
 125 > > +       [DT_DIR]  = S_IFDIR,\r
 126 > > +       [DT_FIFO] = S_IFIFO,\r
 127 > > +       [DT_REG]  = S_IFREG,\r
 128 > > +       [DT_SOCK] = S_IFSOCK\r
 129 > > +    };\r
 130 > > +    if (entry->d_type < sizeof(modes)/sizeof(modes[0]) &&\r
 131 >\r
 132 > ARRAY_SIZE()\r
 133 >\r
 134 > > +       modes[entry->d_type])\r
 135 > > +       return modes[entry->d_type];\r
 136 > > +#endif\r
 137 > > +\r
 138 > > +    abspath = talloc_asprintf (NULL, "%s/%s", path, entry->d_name);\r
 139 > > +    if (!abspath)\r
 140 > > +       return -1;\r
 141 >\r
 142 > Does talloc set errno in this case? I suspect not.\r
 143 >\r
 144 > > +    err = stat(abspath, &statbuf);\r
 145 > > +    talloc_free (abspath);\r
 146 >\r
 147 > This likely breaks your promise about errno. You can't trust\r
 148 talloc_free() not calling some function that sets errno.\r
 149 >\r
 150 > > +    if (err < 0)\r
 151 > > +       return -1;\r
 152 > > +    return statbuf.st_mode & S_IFMT;\r
 153 > > +}\r
 154 > > +\r
 155 > >  /* Test if the directory looks like a Maildir directory.\r
 156 > >  *\r
 157 > >  * Search through the array of directory entries to see if we can find\r
 158 all\r
 159 > > @@ -162,12 +200,12 @@ dirent_sort_strcmp_name (const struct dirent **a,\r
 160 const struct dirent **b)\r
 161 > >  * Return 1 if the directory looks like a Maildir and 0 otherwise.\r
 162 > >  */\r
 163 > >  static int\r
 164 > > -_entries_resemble_maildir (struct dirent **entries, int count)\r
 165 > > +_entries_resemble_maildir (const char *path, struct dirent **entries,\r
 166 int count)\r
 167 > >  {\r
 168 > >     int i, found = 0;\r
 169 > >\r
 170 > >     for (i = 0; i < count; i++) {\r
 171 > > -       if (entries[i]->d_type != DT_DIR && entries[i]->d_type !=\r
 172 DT_UNKNOWN)\r
 173 > > +       if (dirent_type (path, entries[i]) != S_IFDIR)\r
 174 > >            continue;\r
 175 > >\r
 176 > >        if (strcmp(entries[i]->d_name, "new") == 0 ||\r
 177 > > @@ -250,7 +288,7 @@ add_files_recursive (notmuch_database_t *notmuch,\r
 178 > >     notmuch_status_t status, ret = NOTMUCH_STATUS_SUCCESS;\r
 179 > >     notmuch_message_t *message = NULL;\r
 180 > >     struct dirent **fs_entries = NULL;\r
 181 > > -    int i, num_fs_entries;\r
 182 > > +    int i, num_fs_entries, entry_type;\r
 183 > >     notmuch_directory_t *directory;\r
 184 > >     notmuch_filenames_t *db_files = NULL;\r
 185 > >     notmuch_filenames_t *db_subdirs = NULL;\r
 186 > > @@ -317,7 +355,7 @@ add_files_recursive (notmuch_database_t *notmuch,\r
 187 > >     }\r
 188 > >\r
 189 > >     /* Pass 1: Recurse into all sub-directories. */\r
 190 > > -    is_maildir = _entries_resemble_maildir (fs_entries,\r
 191 num_fs_entries);\r
 192 > > +    is_maildir = _entries_resemble_maildir (path, fs_entries,\r
 193 num_fs_entries);\r
 194 > >\r
 195 > >     for (i = 0; i < num_fs_entries; i++) {\r
 196 > >        if (interrupted)\r
 197 > > @@ -325,17 +363,16 @@ add_files_recursive (notmuch_database_t *notmuch,\r
 198 > >\r
 199 > >        entry = fs_entries[i];\r
 200 > >\r
 201 > > -       /* We only want to descend into directories.\r
 202 > > -        * But symlinks can be to directories too, of course.\r
 203 > > -        *\r
 204 > > -        * And if the filesystem doesn't tell us the file type in the\r
 205 > > -        * scandir results, then it might be a directory (and if not,\r
 206 > > -        * then we'll stat and return immediately in the next level of\r
 207 > > -        * recursion). */\r
 208 > > -       if (entry->d_type != DT_DIR &&\r
 209 > > -           entry->d_type != DT_LNK &&\r
 210 > > -           entry->d_type != DT_UNKNOWN)\r
 211 > > -       {\r
 212 > > +       /* We only want to descend into directories (and symlinks to\r
 213 > > +        * directories). */\r
 214 > > +       entry_type = dirent_type (path, entry);\r
 215 > > +       if (entry_type == -1) {\r
 216 > > +           /* Be pessimistic, e.g. so we don't loose lots of mail\r
 217 >\r
 218 > s/loose/lose/ ?\r
 219 >\r
 220 > > +            * just because a user broke a symlink. */\r
 221 > > +           fprintf (stderr, "Error reading file %s/%s: %s\n",\r
 222 > > +                    path, entry->d_name, strerror (errno));\r
 223 >\r
 224 > You can't trust errno here, as explained above.\r
 225 >\r
 226 > > +           return NOTMUCH_STATUS_FILE_ERROR;\r
 227 > > +       } else if (entry_type != S_IFDIR) {\r
 228 > >            continue;\r
 229 > >        }\r
 230 > >\r
 231 > > @@ -425,31 +462,13 @@ add_files_recursive (notmuch_database_t *notmuch,\r
 232 > >            notmuch_filenames_move_to_next (db_subdirs);\r
 233 > >        }\r
 234 > >\r
 235 > > -       /* If we're looking at a symlink, we only want to add it if it\r
 236 > > -        * links to a regular file, (and not to a directory, say).\r
 237 > > -        *\r
 238 > > -        * Similarly, if the file is of unknown type (due to filesystem\r
 239 > > -        * limitations), then we also need to look closer.\r
 240 > > -        *\r
 241 > > -        * In either case, a stat does the trick.\r
 242 > > -        */\r
 243 > > -       if (entry->d_type == DT_LNK || entry->d_type == DT_UNKNOWN) {\r
 244 > > -           int err;\r
 245 > > -\r
 246 > > -           next = talloc_asprintf (notmuch, "%s/%s", path,\r
 247 entry->d_name);\r
 248 > > -           err = stat (next, &st);\r
 249 > > -           talloc_free (next);\r
 250 > > -           next = NULL;\r
 251 > > -\r
 252 > > -           /* Don't emit an error for a link pointing nowhere, since\r
 253 > > -            * the directory-traversal pass will have already done\r
 254 > > -            * that. */\r
 255 > > -           if (err)\r
 256 > > -               continue;\r
 257 > > -\r
 258 > > -           if (! S_ISREG (st.st_mode))\r
 259 > > -               continue;\r
 260 > > -       } else if (entry->d_type != DT_REG) {\r
 261 > > +       /* Only add regular files (and symlinks to regular files). */\r
 262 > > +       entry_type = dirent_type (path, entry);\r
 263 > > +       if (entry_type == -1) {\r
 264 > > +           fprintf (stderr, "Error reading file %s/%s: %s\n",\r
 265 > > +                    path, entry->d_name, strerror (errno));\r
 266 >\r
 267 > Ditto.\r
 268 >\r
 269 > > +           return NOTMUCH_STATUS_FILE_ERROR;\r
 270 > > +       } else if (entry_type != S_IFREG) {\r
 271 > >            continue;\r
 272 > >        }\r
 273 > >\r
 274 > > diff --git a/test/new b/test/new\r
 275 > > index 26253db..e3900f5 100755\r
 276 > > --- a/test/new\r
 277 > > +++ b/test/new\r
 278 > > @@ -140,7 +140,7 @@ test_begin_subtest "Broken symlink aborts"\r
 279 > >  ln -s does-not-exist "${MAIL_DIR}/broken"\r
 280 > >  output=$(NOTMUCH_NEW 2>&1)\r
 281 > >  test_expect_equal "$output" \\r
 282 > > -"Error reading directory /run/shm/nm/tmp.new/mail/broken: No such file\r
 283 or directory\r
 284 > > +"Error reading file /run/shm/nm/tmp.new/mail/broken: No such file or\r
 285 directory\r
 286 > >  Note: A fatal error was encountered: Something went wrong trying to\r
 287 read or write a file\r
 288 > >  No new mail."\r
 289 > >  rm "${MAIL_DIR}/broken"\r
 290 > > --\r
 291 > > 1.7.10\r
 292 > >\r
 293 > > _______________________________________________\r
 294 > > notmuch mailing list\r
 295 > > notmuch@notmuchmail.org\r
 296 > > http://notmuchmail.org/mailman/listinfo/notmuch\r
 297 \r
 298 --047d7b33c9c6ad473d04bf78c08a\r
 299 Content-Type: text/html; charset=UTF-8\r
 300 Content-Transfer-Encoding: quoted-printable\r
 301 \r
 302 <p><br>\r
 303 On May 8, 2012 12:08 AM, &quot;Jani Nikula&quot; &lt;<a href=3D"mailto:jani=\r
 304 @nikula.org">jani@nikula.org</a>&gt; wrote:<br>\r
 305 &gt;<br>\r
 306 &gt;<br>\r
 307 &gt; On May 7, 2012 9:10 PM, &quot;Austin Clements&quot; &lt;<a href=3D"mai=\r
 308 lto:amdragon@mit.edu">amdragon@mit.edu</a>&gt; wrote:<br>\r
 309 &gt; &gt;<br>\r
 310 &gt; &gt; This moves our logic to get a file&#39;s type into one function. =\r
 311 =C2=A0This has<br>\r
 312 &gt; &gt; several benefits: we can support OSes and file systems that do no=\r
 313 t<br>\r
 314 &gt; &gt; provide dirent.d_type or always return DT_UNKNOWN, complex<br>\r
 315 &gt; &gt; symlink-handling logic has been replaced by a simple stat fall-th=\r
 316 rough<br>\r
 317 &gt; &gt; in one place, and the error message for un-stat-able file is more=\r
 318 <br>\r
 319 &gt; &gt; accurate (previously, the error always mentioned directories, eve=\r
 320 n<br>\r
 321 &gt; &gt; though a broken symlink is not a directory).<br>\r
 322 &gt;<br>\r
 323 &gt; Please find some quick drive-by-review below.</p>\r
 324 <p>Forgot to add that the general approach seems sensible to me.<br></p>\r
 325 <p>&gt;<br>\r
 326 &gt; J.<br>\r
 327 &gt;<br>\r
 328 &gt; &gt; ---<br>\r
 329 &gt; &gt; =C2=A0notmuch-new.c | =C2=A0 99 +++++++++++++++++++++++++++++++++=\r
 330 +-----------------------<br>\r
 331 &gt; &gt; =C2=A0test/new =C2=A0 =C2=A0 =C2=A0| =C2=A0 =C2=A02 +-<br>\r
 332 &gt; &gt; =C2=A02 files changed, 60 insertions(+), 41 deletions(-)<br>\r
 333 &gt; &gt;<br>\r
 334 &gt; &gt; diff --git a/notmuch-new.c b/notmuch-new.c<br>\r
 335 &gt; &gt; index cb720cc..cf2580e 100644<br>\r
 336 &gt; &gt; --- a/notmuch-new.c<br>\r
 337 &gt; &gt; +++ b/notmuch-new.c<br>\r
 338 &gt; &gt; @@ -154,6 +154,44 @@ dirent_sort_strcmp_name (const struct dirent=\r
 339  **a, const struct dirent **b)<br>\r
 340 &gt; &gt; =C2=A0 =C2=A0 return strcmp ((*a)-&gt;d_name, (*b)-&gt;d_name);<b=\r
 341 r>\r
 342 &gt; &gt; =C2=A0}<br>\r
 343 &gt; &gt;<br>\r
 344 &gt; &gt; +/* Return the type of a directory entry relative to path as a st=\r
 345 at(2)<br>\r
 346 &gt; &gt; + * mode. =C2=A0Like stat, this follows symlinks. =C2=A0Returns -=\r
 347 1 and sets errno<br>\r
 348 &gt; &gt; + * if the file&#39;s type cannot be determined (which includes d=\r
 349 angling<br>\r
 350 &gt; &gt; + * symlinks).<br>\r
 351 &gt; &gt; + */<br>\r
 352 &gt; &gt; +static int<br>\r
 353 &gt; &gt; +dirent_type (const char *path, const struct dirent *entry)<br>\r
 354 &gt; &gt; +{<br>\r
 355 &gt; &gt; + =C2=A0 =C2=A0struct stat statbuf;<br>\r
 356 &gt; &gt; + =C2=A0 =C2=A0char *abspath;<br>\r
 357 &gt; &gt; + =C2=A0 =C2=A0int err;<br>\r
 358 &gt; &gt; +<br>\r
 359 &gt; &gt; +#ifdef _DIRENT_HAVE_D_TYPE<br>\r
 360 &gt; &gt; + =C2=A0 =C2=A0/* Mapping from d_type to stat mode_t. =C2=A0We om=\r
 361 it DT_LNK so that<br>\r
 362 &gt; &gt; + =C2=A0 =C2=A0 * we&#39;ll fall through to stat and get the real=\r
 363  file type. */<br>\r
 364 &gt; &gt; + =C2=A0 =C2=A0static const mode_t modes[] =3D {<br>\r
 365 &gt; &gt; + =C2=A0 =C2=A0 =C2=A0 [DT_BLK] =C2=A0=3D S_IFBLK,<br>\r
 366 &gt; &gt; + =C2=A0 =C2=A0 =C2=A0 [DT_CHR] =C2=A0=3D S_IFCHR,<br>\r
 367 &gt; &gt; + =C2=A0 =C2=A0 =C2=A0 [DT_DIR] =C2=A0=3D S_IFDIR,<br>\r
 368 &gt; &gt; + =C2=A0 =C2=A0 =C2=A0 [DT_FIFO] =3D S_IFIFO,<br>\r
 369 &gt; &gt; + =C2=A0 =C2=A0 =C2=A0 [DT_REG] =C2=A0=3D S_IFREG,<br>\r
 370 &gt; &gt; + =C2=A0 =C2=A0 =C2=A0 [DT_SOCK] =3D S_IFSOCK<br>\r
 371 &gt; &gt; + =C2=A0 =C2=A0};<br>\r
 372 &gt; &gt; + =C2=A0 =C2=A0if (entry-&gt;d_type &lt; sizeof(modes)/sizeof(mod=\r
 373 es[0]) &amp;&amp;<br>\r
 374 &gt;<br>\r
 375 &gt; ARRAY_SIZE()<br>\r
 376 &gt;<br>\r
 377 &gt; &gt; + =C2=A0 =C2=A0 =C2=A0 modes[entry-&gt;d_type])<br>\r
 378 &gt; &gt; + =C2=A0 =C2=A0 =C2=A0 return modes[entry-&gt;d_type];<br>\r
 379 &gt; &gt; +#endif<br>\r
 380 &gt; &gt; +<br>\r
 381 &gt; &gt; + =C2=A0 =C2=A0abspath =3D talloc_asprintf (NULL, &quot;%s/%s&quo=\r
 382 t;, path, entry-&gt;d_name);<br>\r
 383 &gt; &gt; + =C2=A0 =C2=A0if (!abspath)<br>\r
 384 &gt; &gt; + =C2=A0 =C2=A0 =C2=A0 return -1;<br>\r
 385 &gt;<br>\r
 386 &gt; Does talloc set errno in this case? I suspect not.<br>\r
 387 &gt;<br>\r
 388 &gt; &gt; + =C2=A0 =C2=A0err =3D stat(abspath, &amp;statbuf);<br>\r
 389 &gt; &gt; + =C2=A0 =C2=A0talloc_free (abspath);<br>\r
 390 &gt;<br>\r
 391 &gt; This likely breaks your promise about errno. You can&#39;t trust tallo=\r
 392 c_free() not calling some function that sets errno.<br>\r
 393 &gt;<br>\r
 394 &gt; &gt; + =C2=A0 =C2=A0if (err &lt; 0)<br>\r
 395 &gt; &gt; + =C2=A0 =C2=A0 =C2=A0 return -1;<br>\r
 396 &gt; &gt; + =C2=A0 =C2=A0return statbuf.st_mode &amp; S_IFMT;<br>\r
 397 &gt; &gt; +}<br>\r
 398 &gt; &gt; +<br>\r
 399 &gt; &gt; =C2=A0/* Test if the directory looks like a Maildir directory.<br=\r
 400 >\r
 401 &gt; &gt; =C2=A0*<br>\r
 402 &gt; &gt; =C2=A0* Search through the array of directory entries to see if w=\r
 403 e can find all<br>\r
 404 &gt; &gt; @@ -162,12 +200,12 @@ dirent_sort_strcmp_name (const struct diren=\r
 405 t **a, const struct dirent **b)<br>\r
 406 &gt; &gt; =C2=A0* Return 1 if the directory looks like a Maildir and 0 othe=\r
 407 rwise.<br>\r
 408 &gt; &gt; =C2=A0*/<br>\r
 409 &gt; &gt; =C2=A0static int<br>\r
 410 &gt; &gt; -_entries_resemble_maildir (struct dirent **entries, int count)<b=\r
 411 r>\r
 412 &gt; &gt; +_entries_resemble_maildir (const char *path, struct dirent **ent=\r
 413 ries, int count)<br>\r
 414 &gt; &gt; =C2=A0{<br>\r
 415 &gt; &gt; =C2=A0 =C2=A0 int i, found =3D 0;<br>\r
 416 &gt; &gt;<br>\r
 417 &gt; &gt; =C2=A0 =C2=A0 for (i =3D 0; i &lt; count; i++) {<br>\r
 418 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 if (entries[i]-&gt;d_type !=3D DT_DIR &amp=\r
 419 ;&amp; entries[i]-&gt;d_type !=3D DT_UNKNOWN)<br>\r
 420 &gt; &gt; + =C2=A0 =C2=A0 =C2=A0 if (dirent_type (path, entries[i]) !=3D S_=\r
 421 IFDIR)<br>\r
 422 &gt; &gt; =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0continue;<br>\r
 423 &gt; &gt;<br>\r
 424 &gt; &gt; =C2=A0 =C2=A0 =C2=A0 =C2=A0if (strcmp(entries[i]-&gt;d_name, &quo=\r
 425 t;new&quot;) =3D=3D 0 ||<br>\r
 426 &gt; &gt; @@ -250,7 +288,7 @@ add_files_recursive (notmuch_database_t *notm=\r
 427 uch,<br>\r
 428 &gt; &gt; =C2=A0 =C2=A0 notmuch_status_t status, ret =3D NOTMUCH_STATUS_SUC=\r
 429 CESS;<br>\r
 430 &gt; &gt; =C2=A0 =C2=A0 notmuch_message_t *message =3D NULL;<br>\r
 431 &gt; &gt; =C2=A0 =C2=A0 struct dirent **fs_entries =3D NULL;<br>\r
 432 &gt; &gt; - =C2=A0 =C2=A0int i, num_fs_entries;<br>\r
 433 &gt; &gt; + =C2=A0 =C2=A0int i, num_fs_entries, entry_type;<br>\r
 434 &gt; &gt; =C2=A0 =C2=A0 notmuch_directory_t *directory;<br>\r
 435 &gt; &gt; =C2=A0 =C2=A0 notmuch_filenames_t *db_files =3D NULL;<br>\r
 436 &gt; &gt; =C2=A0 =C2=A0 notmuch_filenames_t *db_subdirs =3D NULL;<br>\r
 437 &gt; &gt; @@ -317,7 +355,7 @@ add_files_recursive (notmuch_database_t *notm=\r
 438 uch,<br>\r
 439 &gt; &gt; =C2=A0 =C2=A0 }<br>\r
 440 &gt; &gt;<br>\r
 441 &gt; &gt; =C2=A0 =C2=A0 /* Pass 1: Recurse into all sub-directories. */<br>\r
 442 &gt; &gt; - =C2=A0 =C2=A0is_maildir =3D _entries_resemble_maildir (fs_entri=\r
 443 es, num_fs_entries);<br>\r
 444 &gt; &gt; + =C2=A0 =C2=A0is_maildir =3D _entries_resemble_maildir (path, fs=\r
 445 _entries, num_fs_entries);<br>\r
 446 &gt; &gt;<br>\r
 447 &gt; &gt; =C2=A0 =C2=A0 for (i =3D 0; i &lt; num_fs_entries; i++) {<br>\r
 448 &gt; &gt; =C2=A0 =C2=A0 =C2=A0 =C2=A0if (interrupted)<br>\r
 449 &gt; &gt; @@ -325,17 +363,16 @@ add_files_recursive (notmuch_database_t *no=\r
 450 tmuch,<br>\r
 451 &gt; &gt;<br>\r
 452 &gt; &gt; =C2=A0 =C2=A0 =C2=A0 =C2=A0entry =3D fs_entries[i];<br>\r
 453 &gt; &gt;<br>\r
 454 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 /* We only want to descend into directorie=\r
 455 s.<br>\r
 456 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 =C2=A0* But symlinks can be to directories=\r
 457  too, of course.<br>\r
 458 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 =C2=A0*<br>\r
 459 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 =C2=A0* And if the filesystem doesn&#39;t =\r
 460 tell us the file type in the<br>\r
 461 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 =C2=A0* scandir results, then it might be =\r
 462 a directory (and if not,<br>\r
 463 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 =C2=A0* then we&#39;ll stat and return imm=\r
 464 ediately in the next level of<br>\r
 465 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 =C2=A0* recursion). */<br>\r
 466 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 if (entry-&gt;d_type !=3D DT_DIR &amp;&amp=\r
 467 ;<br>\r
 468 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 entry-&gt;d_type !=3D DT_LNK=\r
 469  &amp;&amp;<br>\r
 470 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 entry-&gt;d_type !=3D DT_UNK=\r
 471 NOWN)<br>\r
 472 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 {<br>\r
 473 &gt; &gt; + =C2=A0 =C2=A0 =C2=A0 /* We only want to descend into directorie=\r
 474 s (and symlinks to<br>\r
 475 &gt; &gt; + =C2=A0 =C2=A0 =C2=A0 =C2=A0* directories). */<br>\r
 476 &gt; &gt; + =C2=A0 =C2=A0 =C2=A0 entry_type =3D dirent_type (path, entry);<=\r
 477 br>\r
 478 &gt; &gt; + =C2=A0 =C2=A0 =C2=A0 if (entry_type =3D=3D -1) {<br>\r
 479 &gt; &gt; + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Be pessimistic, e.g. so w=\r
 480 e don&#39;t loose lots of mail<br>\r
 481 &gt;<br>\r
 482 &gt; s/loose/lose/ ?<br>\r
 483 &gt;<br>\r
 484 &gt; &gt; + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* just because a user =\r
 485 broke a symlink. */<br>\r
 486 &gt; &gt; + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 fprintf (stderr, &quot;Error=\r
 487  reading file %s/%s: %s\n&quot;,<br>\r
 488 &gt; &gt; + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =\r
 489 =C2=A0path, entry-&gt;d_name, strerror (errno));<br>\r
 490 &gt;<br>\r
 491 &gt; You can&#39;t trust errno here, as explained above.<br>\r
 492 &gt;<br>\r
 493 &gt; &gt; + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return NOTMUCH_STATUS_FILE_E=\r
 494 RROR;<br>\r
 495 &gt; &gt; + =C2=A0 =C2=A0 =C2=A0 } else if (entry_type !=3D S_IFDIR) {<br>\r
 496 &gt; &gt; =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0continue;<br>\r
 497 &gt; &gt; =C2=A0 =C2=A0 =C2=A0 =C2=A0}<br>\r
 498 &gt; &gt;<br>\r
 499 &gt; &gt; @@ -425,31 +462,13 @@ add_files_recursive (notmuch_database_t *no=\r
 500 tmuch,<br>\r
 501 &gt; &gt; =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0notmuch_filenames_move_t=\r
 502 o_next (db_subdirs);<br>\r
 503 &gt; &gt; =C2=A0 =C2=A0 =C2=A0 =C2=A0}<br>\r
 504 &gt; &gt;<br>\r
 505 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 /* If we&#39;re looking at a symlink, we o=\r
 506 nly want to add it if it<br>\r
 507 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 =C2=A0* links to a regular file, (and not =\r
 508 to a directory, say).<br>\r
 509 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 =C2=A0*<br>\r
 510 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 =C2=A0* Similarly, if the file is of unkno=\r
 511 wn type (due to filesystem<br>\r
 512 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 =C2=A0* limitations), then we also need to=\r
 513  look closer.<br>\r
 514 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 =C2=A0*<br>\r
 515 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 =C2=A0* In either case, a stat does the tr=\r
 516 ick.<br>\r
 517 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 =C2=A0*/<br>\r
 518 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 if (entry-&gt;d_type =3D=3D DT_LNK || entr=\r
 519 y-&gt;d_type =3D=3D DT_UNKNOWN) {<br>\r
 520 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 int err;<br>\r
 521 &gt; &gt; -<br>\r
 522 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 next =3D talloc_asprintf (no=\r
 523 tmuch, &quot;%s/%s&quot;, path, entry-&gt;d_name);<br>\r
 524 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 err =3D stat (next, &amp;st)=\r
 525 ;<br>\r
 526 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 talloc_free (next);<br>\r
 527 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 next =3D NULL;<br>\r
 528 &gt; &gt; -<br>\r
 529 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Don&#39;t emit an error f=\r
 530 or a link pointing nowhere, since<br>\r
 531 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* the directory-traver=\r
 532 sal pass will have already done<br>\r
 533 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* that. */<br>\r
 534 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (err)<br>\r
 535 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 continue;<br>\r
 536 &gt; &gt; -<br>\r
 537 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (! S_ISREG (st.st_mode))<=\r
 538 br>\r
 539 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 continue;<br>\r
 540 &gt; &gt; - =C2=A0 =C2=A0 =C2=A0 } else if (entry-&gt;d_type !=3D DT_REG) {=\r
 541 <br>\r
 542 &gt; &gt; + =C2=A0 =C2=A0 =C2=A0 /* Only add regular files (and symlinks to=\r
 543  regular files). */<br>\r
 544 &gt; &gt; + =C2=A0 =C2=A0 =C2=A0 entry_type =3D dirent_type (path, entry);<=\r
 545 br>\r
 546 &gt; &gt; + =C2=A0 =C2=A0 =C2=A0 if (entry_type =3D=3D -1) {<br>\r
 547 &gt; &gt; + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 fprintf (stderr, &quot;Error=\r
 548  reading file %s/%s: %s\n&quot;,<br>\r
 549 &gt; &gt; + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =\r
 550 =C2=A0path, entry-&gt;d_name, strerror (errno));<br>\r
 551 &gt;<br>\r
 552 &gt; Ditto.<br>\r
 553 &gt;<br>\r
 554 &gt; &gt; + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return NOTMUCH_STATUS_FILE_E=\r
 555 RROR;<br>\r
 556 &gt; &gt; + =C2=A0 =C2=A0 =C2=A0 } else if (entry_type !=3D S_IFREG) {<br>\r
 557 &gt; &gt; =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0continue;<br>\r
 558 &gt; &gt; =C2=A0 =C2=A0 =C2=A0 =C2=A0}<br>\r
 559 &gt; &gt;<br>\r
 560 &gt; &gt; diff --git a/test/new b/test/new<br>\r
 561 &gt; &gt; index 26253db..e3900f5 100755<br>\r
 562 &gt; &gt; --- a/test/new<br>\r
 563 &gt; &gt; +++ b/test/new<br>\r
 564 &gt; &gt; @@ -140,7 +140,7 @@ test_begin_subtest &quot;Broken symlink abort=\r
 565 s&quot;<br>\r
 566 &gt; &gt; =C2=A0ln -s does-not-exist &quot;${MAIL_DIR}/broken&quot;<br>\r
 567 &gt; &gt; =C2=A0output=3D$(NOTMUCH_NEW 2&gt;&amp;1)<br>\r
 568 &gt; &gt; =C2=A0test_expect_equal &quot;$output&quot; \<br>\r
 569 &gt; &gt; -&quot;Error reading directory /run/shm/nm/tmp.new/mail/broken: N=\r
 570 o such file or directory<br>\r
 571 &gt; &gt; +&quot;Error reading file /run/shm/nm/tmp.new/mail/broken: No suc=\r
 572 h file or directory<br>\r
 573 &gt; &gt; =C2=A0Note: A fatal error was encountered: Something went wrong t=\r
 574 rying to read or write a file<br>\r
 575 &gt; &gt; =C2=A0No new mail.&quot;<br>\r
 576 &gt; &gt; =C2=A0rm &quot;${MAIL_DIR}/broken&quot;<br>\r
 577 &gt; &gt; --<br>\r
 578 &gt; &gt; 1.7.10<br>\r
 579 &gt; &gt;<br>\r
 580 &gt; &gt; _______________________________________________<br>\r
 581 &gt; &gt; notmuch mailing list<br>\r
 582 &gt; &gt; <a href=3D"mailto:notmuch@notmuchmail.org">notmuch@notmuchmail.or=\r
 583 g</a><br>\r
 584 &gt; &gt; <a href=3D"http://notmuchmail.org/mailman/listinfo/notmuch">http:=\r
 585 //notmuchmail.org/mailman/listinfo/notmuch</a><br>\r
 586 </p>\r
 587 \r
 588 --047d7b33c9c6ad473d04bf78c08a--\r