--- /dev/null
+Return-Path: <jani@nikula.org>\r
+X-Original-To: notmuch@notmuchmail.org\r
+Delivered-To: notmuch@notmuchmail.org\r
+Received: from localhost (localhost [127.0.0.1])\r
+ by olra.theworths.org (Postfix) with ESMTP id 041EC431FB6\r
+ for <notmuch@notmuchmail.org>; Mon, 7 May 2012 14:14:19 -0700 (PDT)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: -0.699\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=-0.699 tagged_above=-999 required=5\r
+ tests=[HTML_MESSAGE=0.001, RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
+Received: from olra.theworths.org ([127.0.0.1])\r
+ by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
+ with ESMTP id 97veg8Yqlq81 for <notmuch@notmuchmail.org>;\r
+ Mon, 7 May 2012 14:14:17 -0700 (PDT)\r
+Received: from mail-pz0-f45.google.com (mail-pz0-f45.google.com\r
+ [209.85.210.45]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
+ (No client certificate requested)\r
+ by olra.theworths.org (Postfix) with ESMTPS id 5297A431FAE\r
+ for <notmuch@notmuchmail.org>; Mon, 7 May 2012 14:14:17 -0700 (PDT)\r
+Received: by dadv2 with SMTP id v2so1373526dad.18\r
+ for <notmuch@notmuchmail.org>; Mon, 07 May 2012 14:14:16 -0700 (PDT)\r
+X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;\r
+ d=google.com; s=20120113;\r
+ h=mime-version:in-reply-to:references:date:message-id:subject:from:to\r
+ :cc:content-type:x-gm-message-state;\r
+ bh=1XJHml9Fai3ajFhy8njaKRybJ9bdn4wi71JTo2fkpHg=;\r
+ b=mOYmGNPyWdZGVzSReOh3GMZmJndsYVkhoMrB1S9cG+Hra6QxwihmERdrYETMcZx9yR\r
+ //BFaI/LgnRNktwIwjYP9uJ4mHnKlv6XChQP93ZtPACQtU6P285TnkF9iTa3b/5sn1Vg\r
+ gXJXLM/aeohszPV7Is9G2PEI0S5jCZq/Tq7pjqPykI/2urUcLSuyZpVGiYPwFOiCVXor\r
+ zPJackD7I/L/kSNHaMI40Hpf1uxzV1DW0QYRNVh9Bxi8bARWldpcysTCgx8Wni6c9FRK\r
+ WFsDXetSlFNjOxyfHTREcueXbLW+ZOxiLGOdkbBDTMrrFchXdfrvjQsWe7DutSJUA7vc\r
+ 3AEw==\r
+MIME-Version: 1.0\r
+Received: by 10.68.236.5 with SMTP id uq5mr455946pbc.165.1336425256404; Mon,\r
+ 07 May 2012 14:14:16 -0700 (PDT)\r
+Received: by 10.68.52.33 with HTTP; Mon, 7 May 2012 14:14:16 -0700 (PDT)\r
+Received: by 10.68.52.33 with HTTP; Mon, 7 May 2012 14:14:16 -0700 (PDT)\r
+In-Reply-To:\r
+ <CAB+hUn_rcz00p4kobjcf57aiyqSAWSPGgRSCNNG0b5eSye=3DQ@mail.gmail.com>\r
+References: <1336414186-15293-1-git-send-email-amdragon@mit.edu>\r
+ <1336414186-15293-3-git-send-email-amdragon@mit.edu>\r
+ <CAB+hUn_rcz00p4kobjcf57aiyqSAWSPGgRSCNNG0b5eSye=3DQ@mail.gmail.com>\r
+Date: Tue, 8 May 2012 00:14:16 +0300\r
+Message-ID:\r
+ <CAB+hUn917qxCVDat64NWpc9r5+9g7yS3y4snkv_p8Oo6Vh7OyQ@mail.gmail.com>\r
+Subject: Re: [PATCH 2/2] new: Centralize file type stat-ing logic\r
+From: Jani Nikula <jani@nikula.org>\r
+To: Austin Clements <amdragon@mit.edu>\r
+Content-Type: multipart/alternative; boundary=047d7b33c9c6ad473d04bf78c08a\r
+X-Gm-Message-State:\r
+ ALoCoQko9LcsJsIgWSM/hFYSIQSv26f8OUh8J4IapJWfjPERaQjv4mx9V6NmvBdVtDh3XV80rQPa\r
+Cc: Notmuch Mail <notmuch@notmuchmail.org>, Vladimir Marek <vlmarek@volny.cz>\r
+X-BeenThere: notmuch@notmuchmail.org\r
+X-Mailman-Version: 2.1.13\r
+Precedence: list\r
+List-Id: "Use and development of the notmuch mail system."\r
+ <notmuch.notmuchmail.org>\r
+List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
+List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
+List-Post: <mailto:notmuch@notmuchmail.org>\r
+List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
+List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
+X-List-Received-Date: Mon, 07 May 2012 21:14:19 -0000\r
+\r
+--047d7b33c9c6ad473d04bf78c08a\r
+Content-Type: text/plain; charset=UTF-8\r
+\r
+On May 8, 2012 12:08 AM, "Jani Nikula" <jani@nikula.org> wrote:\r
+>\r
+>\r
+> On May 7, 2012 9:10 PM, "Austin Clements" <amdragon@mit.edu> wrote:\r
+> >\r
+> > This moves our logic to get a file's type into one function. This has\r
+> > several benefits: we can support OSes and file systems that do not\r
+> > provide dirent.d_type or always return DT_UNKNOWN, complex\r
+> > symlink-handling logic has been replaced by a simple stat fall-through\r
+> > in one place, and the error message for un-stat-able file is more\r
+> > accurate (previously, the error always mentioned directories, even\r
+> > though a broken symlink is not a directory).\r
+>\r
+> Please find some quick drive-by-review below.\r
+\r
+Forgot to add that the general approach seems sensible to me.\r
+\r
+>\r
+> J.\r
+>\r
+> > ---\r
+> > notmuch-new.c | 99\r
+++++++++++++++++++++++++++++++++++-----------------------\r
+> > test/new | 2 +-\r
+> > 2 files changed, 60 insertions(+), 41 deletions(-)\r
+> >\r
+> > diff --git a/notmuch-new.c b/notmuch-new.c\r
+> > index cb720cc..cf2580e 100644\r
+> > --- a/notmuch-new.c\r
+> > +++ b/notmuch-new.c\r
+> > @@ -154,6 +154,44 @@ dirent_sort_strcmp_name (const struct dirent **a,\r
+const struct dirent **b)\r
+> > return strcmp ((*a)->d_name, (*b)->d_name);\r
+> > }\r
+> >\r
+> > +/* Return the type of a directory entry relative to path as a stat(2)\r
+> > + * mode. Like stat, this follows symlinks. Returns -1 and sets errno\r
+> > + * if the file's type cannot be determined (which includes dangling\r
+> > + * symlinks).\r
+> > + */\r
+> > +static int\r
+> > +dirent_type (const char *path, const struct dirent *entry)\r
+> > +{\r
+> > + struct stat statbuf;\r
+> > + char *abspath;\r
+> > + int err;\r
+> > +\r
+> > +#ifdef _DIRENT_HAVE_D_TYPE\r
+> > + /* Mapping from d_type to stat mode_t. We omit DT_LNK so that\r
+> > + * we'll fall through to stat and get the real file type. */\r
+> > + static const mode_t modes[] = {\r
+> > + [DT_BLK] = S_IFBLK,\r
+> > + [DT_CHR] = S_IFCHR,\r
+> > + [DT_DIR] = S_IFDIR,\r
+> > + [DT_FIFO] = S_IFIFO,\r
+> > + [DT_REG] = S_IFREG,\r
+> > + [DT_SOCK] = S_IFSOCK\r
+> > + };\r
+> > + if (entry->d_type < sizeof(modes)/sizeof(modes[0]) &&\r
+>\r
+> ARRAY_SIZE()\r
+>\r
+> > + modes[entry->d_type])\r
+> > + return modes[entry->d_type];\r
+> > +#endif\r
+> > +\r
+> > + abspath = talloc_asprintf (NULL, "%s/%s", path, entry->d_name);\r
+> > + if (!abspath)\r
+> > + return -1;\r
+>\r
+> Does talloc set errno in this case? I suspect not.\r
+>\r
+> > + err = stat(abspath, &statbuf);\r
+> > + talloc_free (abspath);\r
+>\r
+> This likely breaks your promise about errno. You can't trust\r
+talloc_free() not calling some function that sets errno.\r
+>\r
+> > + if (err < 0)\r
+> > + return -1;\r
+> > + return statbuf.st_mode & S_IFMT;\r
+> > +}\r
+> > +\r
+> > /* Test if the directory looks like a Maildir directory.\r
+> > *\r
+> > * Search through the array of directory entries to see if we can find\r
+all\r
+> > @@ -162,12 +200,12 @@ dirent_sort_strcmp_name (const struct dirent **a,\r
+const struct dirent **b)\r
+> > * Return 1 if the directory looks like a Maildir and 0 otherwise.\r
+> > */\r
+> > static int\r
+> > -_entries_resemble_maildir (struct dirent **entries, int count)\r
+> > +_entries_resemble_maildir (const char *path, struct dirent **entries,\r
+int count)\r
+> > {\r
+> > int i, found = 0;\r
+> >\r
+> > for (i = 0; i < count; i++) {\r
+> > - if (entries[i]->d_type != DT_DIR && entries[i]->d_type !=\r
+DT_UNKNOWN)\r
+> > + if (dirent_type (path, entries[i]) != S_IFDIR)\r
+> > continue;\r
+> >\r
+> > if (strcmp(entries[i]->d_name, "new") == 0 ||\r
+> > @@ -250,7 +288,7 @@ add_files_recursive (notmuch_database_t *notmuch,\r
+> > notmuch_status_t status, ret = NOTMUCH_STATUS_SUCCESS;\r
+> > notmuch_message_t *message = NULL;\r
+> > struct dirent **fs_entries = NULL;\r
+> > - int i, num_fs_entries;\r
+> > + int i, num_fs_entries, entry_type;\r
+> > notmuch_directory_t *directory;\r
+> > notmuch_filenames_t *db_files = NULL;\r
+> > notmuch_filenames_t *db_subdirs = NULL;\r
+> > @@ -317,7 +355,7 @@ add_files_recursive (notmuch_database_t *notmuch,\r
+> > }\r
+> >\r
+> > /* Pass 1: Recurse into all sub-directories. */\r
+> > - is_maildir = _entries_resemble_maildir (fs_entries,\r
+num_fs_entries);\r
+> > + is_maildir = _entries_resemble_maildir (path, fs_entries,\r
+num_fs_entries);\r
+> >\r
+> > for (i = 0; i < num_fs_entries; i++) {\r
+> > if (interrupted)\r
+> > @@ -325,17 +363,16 @@ add_files_recursive (notmuch_database_t *notmuch,\r
+> >\r
+> > entry = fs_entries[i];\r
+> >\r
+> > - /* We only want to descend into directories.\r
+> > - * But symlinks can be to directories too, of course.\r
+> > - *\r
+> > - * And if the filesystem doesn't tell us the file type in the\r
+> > - * scandir results, then it might be a directory (and if not,\r
+> > - * then we'll stat and return immediately in the next level of\r
+> > - * recursion). */\r
+> > - if (entry->d_type != DT_DIR &&\r
+> > - entry->d_type != DT_LNK &&\r
+> > - entry->d_type != DT_UNKNOWN)\r
+> > - {\r
+> > + /* We only want to descend into directories (and symlinks to\r
+> > + * directories). */\r
+> > + entry_type = dirent_type (path, entry);\r
+> > + if (entry_type == -1) {\r
+> > + /* Be pessimistic, e.g. so we don't loose lots of mail\r
+>\r
+> s/loose/lose/ ?\r
+>\r
+> > + * just because a user broke a symlink. */\r
+> > + fprintf (stderr, "Error reading file %s/%s: %s\n",\r
+> > + path, entry->d_name, strerror (errno));\r
+>\r
+> You can't trust errno here, as explained above.\r
+>\r
+> > + return NOTMUCH_STATUS_FILE_ERROR;\r
+> > + } else if (entry_type != S_IFDIR) {\r
+> > continue;\r
+> > }\r
+> >\r
+> > @@ -425,31 +462,13 @@ add_files_recursive (notmuch_database_t *notmuch,\r
+> > notmuch_filenames_move_to_next (db_subdirs);\r
+> > }\r
+> >\r
+> > - /* If we're looking at a symlink, we only want to add it if it\r
+> > - * links to a regular file, (and not to a directory, say).\r
+> > - *\r
+> > - * Similarly, if the file is of unknown type (due to filesystem\r
+> > - * limitations), then we also need to look closer.\r
+> > - *\r
+> > - * In either case, a stat does the trick.\r
+> > - */\r
+> > - if (entry->d_type == DT_LNK || entry->d_type == DT_UNKNOWN) {\r
+> > - int err;\r
+> > -\r
+> > - next = talloc_asprintf (notmuch, "%s/%s", path,\r
+entry->d_name);\r
+> > - err = stat (next, &st);\r
+> > - talloc_free (next);\r
+> > - next = NULL;\r
+> > -\r
+> > - /* Don't emit an error for a link pointing nowhere, since\r
+> > - * the directory-traversal pass will have already done\r
+> > - * that. */\r
+> > - if (err)\r
+> > - continue;\r
+> > -\r
+> > - if (! S_ISREG (st.st_mode))\r
+> > - continue;\r
+> > - } else if (entry->d_type != DT_REG) {\r
+> > + /* Only add regular files (and symlinks to regular files). */\r
+> > + entry_type = dirent_type (path, entry);\r
+> > + if (entry_type == -1) {\r
+> > + fprintf (stderr, "Error reading file %s/%s: %s\n",\r
+> > + path, entry->d_name, strerror (errno));\r
+>\r
+> Ditto.\r
+>\r
+> > + return NOTMUCH_STATUS_FILE_ERROR;\r
+> > + } else if (entry_type != S_IFREG) {\r
+> > continue;\r
+> > }\r
+> >\r
+> > diff --git a/test/new b/test/new\r
+> > index 26253db..e3900f5 100755\r
+> > --- a/test/new\r
+> > +++ b/test/new\r
+> > @@ -140,7 +140,7 @@ test_begin_subtest "Broken symlink aborts"\r
+> > ln -s does-not-exist "${MAIL_DIR}/broken"\r
+> > output=$(NOTMUCH_NEW 2>&1)\r
+> > test_expect_equal "$output" \\r
+> > -"Error reading directory /run/shm/nm/tmp.new/mail/broken: No such file\r
+or directory\r
+> > +"Error reading file /run/shm/nm/tmp.new/mail/broken: No such file or\r
+directory\r
+> > Note: A fatal error was encountered: Something went wrong trying to\r
+read or write a file\r
+> > No new mail."\r
+> > rm "${MAIL_DIR}/broken"\r
+> > --\r
+> > 1.7.10\r
+> >\r
+> > _______________________________________________\r
+> > notmuch mailing list\r
+> > notmuch@notmuchmail.org\r
+> > http://notmuchmail.org/mailman/listinfo/notmuch\r
+\r
+--047d7b33c9c6ad473d04bf78c08a\r
+Content-Type: text/html; charset=UTF-8\r
+Content-Transfer-Encoding: quoted-printable\r
+\r
+<p><br>\r
+On May 8, 2012 12:08 AM, "Jani Nikula" <<a href=3D"mailto:jani=\r
+@nikula.org">jani@nikula.org</a>> wrote:<br>\r
+><br>\r
+><br>\r
+> On May 7, 2012 9:10 PM, "Austin Clements" <<a href=3D"mai=\r
+lto:amdragon@mit.edu">amdragon@mit.edu</a>> wrote:<br>\r
+> ><br>\r
+> > This moves our logic to get a file's type into one function. =\r
+=C2=A0This has<br>\r
+> > several benefits: we can support OSes and file systems that do no=\r
+t<br>\r
+> > provide dirent.d_type or always return DT_UNKNOWN, complex<br>\r
+> > symlink-handling logic has been replaced by a simple stat fall-th=\r
+rough<br>\r
+> > in one place, and the error message for un-stat-able file is more=\r
+<br>\r
+> > accurate (previously, the error always mentioned directories, eve=\r
+n<br>\r
+> > though a broken symlink is not a directory).<br>\r
+><br>\r
+> Please find some quick drive-by-review below.</p>\r
+<p>Forgot to add that the general approach seems sensible to me.<br></p>\r
+<p>><br>\r
+> J.<br>\r
+><br>\r
+> > ---<br>\r
+> > =C2=A0notmuch-new.c | =C2=A0 99 +++++++++++++++++++++++++++++++++=\r
++-----------------------<br>\r
+> > =C2=A0test/new =C2=A0 =C2=A0 =C2=A0| =C2=A0 =C2=A02 +-<br>\r
+> > =C2=A02 files changed, 60 insertions(+), 41 deletions(-)<br>\r
+> ><br>\r
+> > diff --git a/notmuch-new.c b/notmuch-new.c<br>\r
+> > index cb720cc..cf2580e 100644<br>\r
+> > --- a/notmuch-new.c<br>\r
+> > +++ b/notmuch-new.c<br>\r
+> > @@ -154,6 +154,44 @@ dirent_sort_strcmp_name (const struct dirent=\r
+ **a, const struct dirent **b)<br>\r
+> > =C2=A0 =C2=A0 return strcmp ((*a)->d_name, (*b)->d_name);<b=\r
+r>\r
+> > =C2=A0}<br>\r
+> ><br>\r
+> > +/* Return the type of a directory entry relative to path as a st=\r
+at(2)<br>\r
+> > + * mode. =C2=A0Like stat, this follows symlinks. =C2=A0Returns -=\r
+1 and sets errno<br>\r
+> > + * if the file's type cannot be determined (which includes d=\r
+angling<br>\r
+> > + * symlinks).<br>\r
+> > + */<br>\r
+> > +static int<br>\r
+> > +dirent_type (const char *path, const struct dirent *entry)<br>\r
+> > +{<br>\r
+> > + =C2=A0 =C2=A0struct stat statbuf;<br>\r
+> > + =C2=A0 =C2=A0char *abspath;<br>\r
+> > + =C2=A0 =C2=A0int err;<br>\r
+> > +<br>\r
+> > +#ifdef _DIRENT_HAVE_D_TYPE<br>\r
+> > + =C2=A0 =C2=A0/* Mapping from d_type to stat mode_t. =C2=A0We om=\r
+it DT_LNK so that<br>\r
+> > + =C2=A0 =C2=A0 * we'll fall through to stat and get the real=\r
+ file type. */<br>\r
+> > + =C2=A0 =C2=A0static const mode_t modes[] =3D {<br>\r
+> > + =C2=A0 =C2=A0 =C2=A0 [DT_BLK] =C2=A0=3D S_IFBLK,<br>\r
+> > + =C2=A0 =C2=A0 =C2=A0 [DT_CHR] =C2=A0=3D S_IFCHR,<br>\r
+> > + =C2=A0 =C2=A0 =C2=A0 [DT_DIR] =C2=A0=3D S_IFDIR,<br>\r
+> > + =C2=A0 =C2=A0 =C2=A0 [DT_FIFO] =3D S_IFIFO,<br>\r
+> > + =C2=A0 =C2=A0 =C2=A0 [DT_REG] =C2=A0=3D S_IFREG,<br>\r
+> > + =C2=A0 =C2=A0 =C2=A0 [DT_SOCK] =3D S_IFSOCK<br>\r
+> > + =C2=A0 =C2=A0};<br>\r
+> > + =C2=A0 =C2=A0if (entry->d_type < sizeof(modes)/sizeof(mod=\r
+es[0]) &&<br>\r
+><br>\r
+> ARRAY_SIZE()<br>\r
+><br>\r
+> > + =C2=A0 =C2=A0 =C2=A0 modes[entry->d_type])<br>\r
+> > + =C2=A0 =C2=A0 =C2=A0 return modes[entry->d_type];<br>\r
+> > +#endif<br>\r
+> > +<br>\r
+> > + =C2=A0 =C2=A0abspath =3D talloc_asprintf (NULL, "%s/%s&quo=\r
+t;, path, entry->d_name);<br>\r
+> > + =C2=A0 =C2=A0if (!abspath)<br>\r
+> > + =C2=A0 =C2=A0 =C2=A0 return -1;<br>\r
+><br>\r
+> Does talloc set errno in this case? I suspect not.<br>\r
+><br>\r
+> > + =C2=A0 =C2=A0err =3D stat(abspath, &statbuf);<br>\r
+> > + =C2=A0 =C2=A0talloc_free (abspath);<br>\r
+><br>\r
+> This likely breaks your promise about errno. You can't trust tallo=\r
+c_free() not calling some function that sets errno.<br>\r
+><br>\r
+> > + =C2=A0 =C2=A0if (err < 0)<br>\r
+> > + =C2=A0 =C2=A0 =C2=A0 return -1;<br>\r
+> > + =C2=A0 =C2=A0return statbuf.st_mode & S_IFMT;<br>\r
+> > +}<br>\r
+> > +<br>\r
+> > =C2=A0/* Test if the directory looks like a Maildir directory.<br=\r
+>\r
+> > =C2=A0*<br>\r
+> > =C2=A0* Search through the array of directory entries to see if w=\r
+e can find all<br>\r
+> > @@ -162,12 +200,12 @@ dirent_sort_strcmp_name (const struct diren=\r
+t **a, const struct dirent **b)<br>\r
+> > =C2=A0* Return 1 if the directory looks like a Maildir and 0 othe=\r
+rwise.<br>\r
+> > =C2=A0*/<br>\r
+> > =C2=A0static int<br>\r
+> > -_entries_resemble_maildir (struct dirent **entries, int count)<b=\r
+r>\r
+> > +_entries_resemble_maildir (const char *path, struct dirent **ent=\r
+ries, int count)<br>\r
+> > =C2=A0{<br>\r
+> > =C2=A0 =C2=A0 int i, found =3D 0;<br>\r
+> ><br>\r
+> > =C2=A0 =C2=A0 for (i =3D 0; i < count; i++) {<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 if (entries[i]->d_type !=3D DT_DIR &=\r
+;& entries[i]->d_type !=3D DT_UNKNOWN)<br>\r
+> > + =C2=A0 =C2=A0 =C2=A0 if (dirent_type (path, entries[i]) !=3D S_=\r
+IFDIR)<br>\r
+> > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0continue;<br>\r
+> ><br>\r
+> > =C2=A0 =C2=A0 =C2=A0 =C2=A0if (strcmp(entries[i]->d_name, &quo=\r
+t;new") =3D=3D 0 ||<br>\r
+> > @@ -250,7 +288,7 @@ add_files_recursive (notmuch_database_t *notm=\r
+uch,<br>\r
+> > =C2=A0 =C2=A0 notmuch_status_t status, ret =3D NOTMUCH_STATUS_SUC=\r
+CESS;<br>\r
+> > =C2=A0 =C2=A0 notmuch_message_t *message =3D NULL;<br>\r
+> > =C2=A0 =C2=A0 struct dirent **fs_entries =3D NULL;<br>\r
+> > - =C2=A0 =C2=A0int i, num_fs_entries;<br>\r
+> > + =C2=A0 =C2=A0int i, num_fs_entries, entry_type;<br>\r
+> > =C2=A0 =C2=A0 notmuch_directory_t *directory;<br>\r
+> > =C2=A0 =C2=A0 notmuch_filenames_t *db_files =3D NULL;<br>\r
+> > =C2=A0 =C2=A0 notmuch_filenames_t *db_subdirs =3D NULL;<br>\r
+> > @@ -317,7 +355,7 @@ add_files_recursive (notmuch_database_t *notm=\r
+uch,<br>\r
+> > =C2=A0 =C2=A0 }<br>\r
+> ><br>\r
+> > =C2=A0 =C2=A0 /* Pass 1: Recurse into all sub-directories. */<br>\r
+> > - =C2=A0 =C2=A0is_maildir =3D _entries_resemble_maildir (fs_entri=\r
+es, num_fs_entries);<br>\r
+> > + =C2=A0 =C2=A0is_maildir =3D _entries_resemble_maildir (path, fs=\r
+_entries, num_fs_entries);<br>\r
+> ><br>\r
+> > =C2=A0 =C2=A0 for (i =3D 0; i < num_fs_entries; i++) {<br>\r
+> > =C2=A0 =C2=A0 =C2=A0 =C2=A0if (interrupted)<br>\r
+> > @@ -325,17 +363,16 @@ add_files_recursive (notmuch_database_t *no=\r
+tmuch,<br>\r
+> ><br>\r
+> > =C2=A0 =C2=A0 =C2=A0 =C2=A0entry =3D fs_entries[i];<br>\r
+> ><br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 /* We only want to descend into directorie=\r
+s.<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0* But symlinks can be to directories=\r
+ too, of course.<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0*<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0* And if the filesystem doesn't =\r
+tell us the file type in the<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0* scandir results, then it might be =\r
+a directory (and if not,<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0* then we'll stat and return imm=\r
+ediately in the next level of<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0* recursion). */<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 if (entry->d_type !=3D DT_DIR &&=\r
+;<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 entry->d_type !=3D DT_LNK=\r
+ &&<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 entry->d_type !=3D DT_UNK=\r
+NOWN)<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 {<br>\r
+> > + =C2=A0 =C2=A0 =C2=A0 /* We only want to descend into directorie=\r
+s (and symlinks to<br>\r
+> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0* directories). */<br>\r
+> > + =C2=A0 =C2=A0 =C2=A0 entry_type =3D dirent_type (path, entry);<=\r
+br>\r
+> > + =C2=A0 =C2=A0 =C2=A0 if (entry_type =3D=3D -1) {<br>\r
+> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Be pessimistic, e.g. so w=\r
+e don't loose lots of mail<br>\r
+><br>\r
+> s/loose/lose/ ?<br>\r
+><br>\r
+> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* just because a user =\r
+broke a symlink. */<br>\r
+> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 fprintf (stderr, "Error=\r
+ reading file %s/%s: %s\n",<br>\r
+> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =\r
+=C2=A0path, entry->d_name, strerror (errno));<br>\r
+><br>\r
+> You can't trust errno here, as explained above.<br>\r
+><br>\r
+> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return NOTMUCH_STATUS_FILE_E=\r
+RROR;<br>\r
+> > + =C2=A0 =C2=A0 =C2=A0 } else if (entry_type !=3D S_IFDIR) {<br>\r
+> > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0continue;<br>\r
+> > =C2=A0 =C2=A0 =C2=A0 =C2=A0}<br>\r
+> ><br>\r
+> > @@ -425,31 +462,13 @@ add_files_recursive (notmuch_database_t *no=\r
+tmuch,<br>\r
+> > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0notmuch_filenames_move_t=\r
+o_next (db_subdirs);<br>\r
+> > =C2=A0 =C2=A0 =C2=A0 =C2=A0}<br>\r
+> ><br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 /* If we're looking at a symlink, we o=\r
+nly want to add it if it<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0* links to a regular file, (and not =\r
+to a directory, say).<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0*<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0* Similarly, if the file is of unkno=\r
+wn type (due to filesystem<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0* limitations), then we also need to=\r
+ look closer.<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0*<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0* In either case, a stat does the tr=\r
+ick.<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0*/<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 if (entry->d_type =3D=3D DT_LNK || entr=\r
+y->d_type =3D=3D DT_UNKNOWN) {<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 int err;<br>\r
+> > -<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 next =3D talloc_asprintf (no=\r
+tmuch, "%s/%s", path, entry->d_name);<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 err =3D stat (next, &st)=\r
+;<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 talloc_free (next);<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 next =3D NULL;<br>\r
+> > -<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Don't emit an error f=\r
+or a link pointing nowhere, since<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* the directory-traver=\r
+sal pass will have already done<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* that. */<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (err)<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 continue;<br>\r
+> > -<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (! S_ISREG (st.st_mode))<=\r
+br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 continue;<br>\r
+> > - =C2=A0 =C2=A0 =C2=A0 } else if (entry->d_type !=3D DT_REG) {=\r
+<br>\r
+> > + =C2=A0 =C2=A0 =C2=A0 /* Only add regular files (and symlinks to=\r
+ regular files). */<br>\r
+> > + =C2=A0 =C2=A0 =C2=A0 entry_type =3D dirent_type (path, entry);<=\r
+br>\r
+> > + =C2=A0 =C2=A0 =C2=A0 if (entry_type =3D=3D -1) {<br>\r
+> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 fprintf (stderr, "Error=\r
+ reading file %s/%s: %s\n",<br>\r
+> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =\r
+=C2=A0path, entry->d_name, strerror (errno));<br>\r
+><br>\r
+> Ditto.<br>\r
+><br>\r
+> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return NOTMUCH_STATUS_FILE_E=\r
+RROR;<br>\r
+> > + =C2=A0 =C2=A0 =C2=A0 } else if (entry_type !=3D S_IFREG) {<br>\r
+> > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0continue;<br>\r
+> > =C2=A0 =C2=A0 =C2=A0 =C2=A0}<br>\r
+> ><br>\r
+> > diff --git a/test/new b/test/new<br>\r
+> > index 26253db..e3900f5 100755<br>\r
+> > --- a/test/new<br>\r
+> > +++ b/test/new<br>\r
+> > @@ -140,7 +140,7 @@ test_begin_subtest "Broken symlink abort=\r
+s"<br>\r
+> > =C2=A0ln -s does-not-exist "${MAIL_DIR}/broken"<br>\r
+> > =C2=A0output=3D$(NOTMUCH_NEW 2>&1)<br>\r
+> > =C2=A0test_expect_equal "$output" \<br>\r
+> > -"Error reading directory /run/shm/nm/tmp.new/mail/broken: N=\r
+o such file or directory<br>\r
+> > +"Error reading file /run/shm/nm/tmp.new/mail/broken: No suc=\r
+h file or directory<br>\r
+> > =C2=A0Note: A fatal error was encountered: Something went wrong t=\r
+rying to read or write a file<br>\r
+> > =C2=A0No new mail."<br>\r
+> > =C2=A0rm "${MAIL_DIR}/broken"<br>\r
+> > --<br>\r
+> > 1.7.10<br>\r
+> ><br>\r
+> > _______________________________________________<br>\r
+> > notmuch mailing list<br>\r
+> > <a href=3D"mailto:notmuch@notmuchmail.org">notmuch@notmuchmail.or=\r
+g</a><br>\r
+> > <a href=3D"http://notmuchmail.org/mailman/listinfo/notmuch">http:=\r
+//notmuchmail.org/mailman/listinfo/notmuch</a><br>\r
+</p>\r
+\r
+--047d7b33c9c6ad473d04bf78c08a--\r
projects / notmuch-archives.git / commitdiff