cd/f143d3e1a7f857a693d1c664ad1e917efa5fbc

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