--- /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 460FC431FB6\r
+ for <notmuch@notmuchmail.org>; Mon, 7 May 2012 14:08:39 -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 v9+Gi4YCfqE6 for <notmuch@notmuchmail.org>;\r
+ Mon, 7 May 2012 14:08:37 -0700 (PDT)\r
+Received: from mail-pb0-f53.google.com (mail-pb0-f53.google.com\r
+ [209.85.160.53]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
+ (No client certificate requested)\r
+ by olra.theworths.org (Postfix) with ESMTPS id AF375431FAE\r
+ for <notmuch@notmuchmail.org>; Mon, 7 May 2012 14:08:37 -0700 (PDT)\r
+Received: by pbbrr13 with SMTP id rr13so8240660pbb.26\r
+ for <notmuch@notmuchmail.org>; Mon, 07 May 2012 14:08:35 -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=vmHcNGrU4I/7z3kpCwJhVirRk0Hap1yMXRTbQFRFyhE=;\r
+ b=DPr1y4CbTOPTtEvpeHD99HPfPhoTveicyQjj6Bg4OkOBvUowBKdOQMBa02qjd4z07R\r
+ R4mU/CesDgYezoLZgg9E5u0IEnC1GIqMX6b7L6mRzGLZrLtfyRixYs3F17RzlQtwoBNc\r
+ 1V1eH7WMdOGHDjJq6GJoIjoGSpJETl6eoac5umml1eiWdswcWZvNkpDdmSVlZUChb5Zo\r
+ /3pcTk00NO0NoqJLfeIOVU7agItvYeJ87AgAbWJShTP+q/Brz643VLNre3gPcYFn23CS\r
+ rQqdzTH8fh7L/w+pgMRfZHmSgPGfLc/2C+rRv3DvUCT4mSZirmGaq0zD2MbXaZMJOphx\r
+ eakQ==\r
+MIME-Version: 1.0\r
+Received: by 10.68.236.5 with SMTP id uq5mr410171pbc.165.1336424915761; Mon,\r
+ 07 May 2012 14:08:35 -0700 (PDT)\r
+Received: by 10.68.52.33 with HTTP; Mon, 7 May 2012 14:08:35 -0700 (PDT)\r
+Received: by 10.68.52.33 with HTTP; Mon, 7 May 2012 14:08:35 -0700 (PDT)\r
+In-Reply-To: <1336414186-15293-3-git-send-email-amdragon@mit.edu>\r
+References: <1336414186-15293-1-git-send-email-amdragon@mit.edu>\r
+ <1336414186-15293-3-git-send-email-amdragon@mit.edu>\r
+Date: Tue, 8 May 2012 00:08:35 +0300\r
+Message-ID:\r
+ <CAB+hUn_rcz00p4kobjcf57aiyqSAWSPGgRSCNNG0b5eSye=3DQ@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=047d7b33c9c65f7b8204bf78ac10\r
+X-Gm-Message-State:\r
+ ALoCoQmhKRKJHFxNKNd4Q3OgSi6Wk0CGmXs1VXfUKMYwCjTunscnVZ4zVse3K3U19uflXQ4VgCwf\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:08:39 -0000\r
+\r
+--047d7b33c9c65f7b8204bf78ac10\r
+Content-Type: text/plain; charset=UTF-8\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
+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 talloc_free()\r
+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 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, 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 read\r
+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
+--047d7b33c9c65f7b8204bf78ac10\r
+Content-Type: text/html; charset=UTF-8\r
+Content-Transfer-Encoding: quoted-printable\r
+\r
+<p><br>\r
+On May 7, 2012 9:10 PM, "Austin Clements" <<a href=3D"mailto:a=\r
+mdragon@mit.edu">amdragon@mit.edu</a>> wrote:<br>\r
+><br>\r
+> This moves our logic to get a file's type into one function. =C2=\r
+=A0This has<br>\r
+> several benefits: we can support OSes and file systems that do not<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-through=\r
+<br>\r
+> in one place, and the error message for un-stat-able file is more<br>\r
+> accurate (previously, the error always mentioned directories, even<br>\r
+> though a broken symlink is not a directory).</p>\r
+<p>Please find some quick drive-by-review below.</p>\r
+<p>J.</p>\r
+<p>> ---<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 **a,=\r
+ const struct dirent **b)<br>\r
+> =C2=A0 =C2=A0 return strcmp ((*a)->d_name, (*b)->d_name);<br>\r
+> =C2=A0}<br>\r
+><br>\r
+> +/* Return the type of a directory entry relative to path as a stat(2)=\r
+<br>\r
+> + * mode. =C2=A0Like stat, this follows symlinks. =C2=A0Returns -1 and=\r
+ sets errno<br>\r
+> + * if the file's type cannot be determined (which includes dangli=\r
+ng<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 omit DT=\r
+_LNK so that<br>\r
+> + =C2=A0 =C2=A0 * we'll fall through to stat and get the real file=\r
+ 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(modes[0]=\r
+) &&</p>\r
+<p>ARRAY_SIZE()</p>\r
+<p>> + =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", p=\r
+ath, entry->d_name);<br>\r
+> + =C2=A0 =C2=A0if (!abspath)<br>\r
+> + =C2=A0 =C2=A0 =C2=A0 return -1;</p>\r
+<p>Does talloc set errno in this case? I suspect not.</p>\r
+<p>> + =C2=A0 =C2=A0err =3D stat(abspath, &statbuf);<br>\r
+> + =C2=A0 =C2=A0talloc_free (abspath);</p>\r
+<p>This likely breaks your promise about errno. You can't trust talloc_=\r
+free() not calling some function that sets errno.</p>\r
+<p>> + =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
+> =C2=A0*<br>\r
+> =C2=A0* Search through the array of directory entries to see if we can=\r
+ find all<br>\r
+> @@ -162,12 +200,12 @@ dirent_sort_strcmp_name (const struct dirent **a=\r
+, const struct dirent **b)<br>\r
+> =C2=A0* Return 1 if the directory looks like a Maildir and 0 otherwise=\r
+.<br>\r
+> =C2=A0*/<br>\r
+> =C2=A0static int<br>\r
+> -_entries_resemble_maildir (struct dirent **entries, int count)<br>\r
+> +_entries_resemble_maildir (const char *path, struct dirent **entries,=\r
+ 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_IFDIR=\r
+)<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, "new=\r
+") =3D=3D 0 ||<br>\r
+> @@ -250,7 +288,7 @@ add_files_recursive (notmuch_database_t *notmuch,<=\r
+br>\r
+> =C2=A0 =C2=A0 notmuch_status_t status, ret =3D NOTMUCH_STATUS_SUCCESS;=\r
+<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 *notmuch,<=\r
+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_entries, n=\r
+um_fs_entries);<br>\r
+> + =C2=A0 =C2=A0is_maildir =3D _entries_resemble_maildir (path, fs_entr=\r
+ies, 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 *notmuch=\r
+,<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 directories.<br=\r
+>\r
+> - =C2=A0 =C2=A0 =C2=A0 =C2=A0* But symlinks can be to directories too,=\r
+ 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 tell =\r
+us the file type in the<br>\r
+> - =C2=A0 =C2=A0 =C2=A0 =C2=A0* scandir results, then it might be a dir=\r
+ectory (and if not,<br>\r
+> - =C2=A0 =C2=A0 =C2=A0 =C2=A0* then we'll stat and return immediat=\r
+ely 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 &&<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_UNKNOWN)=\r
+<br>\r
+> - =C2=A0 =C2=A0 =C2=A0 {<br>\r
+> + =C2=A0 =C2=A0 =C2=A0 /* We only want to descend into directories (an=\r
+d 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);<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 we don=\r
+'t loose lots of mail</p>\r
+<p>s/loose/lose/ ?</p>\r
+<p>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* just because a user br=\r
+oke a symlink. */<br>\r
+> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 fprintf (stderr, "Error read=\r
+ing 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 =C2=\r
+=A0path, entry->d_name, strerror (errno));</p>\r
+<p>You can't trust errno here, as explained above.</p>\r
+<p>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return NOTMUCH_STATUS_FILE_ERR=\r
+OR;<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 *notmuch=\r
+,<br>\r
+> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0notmuch_filenames_move_to_nex=\r
+t (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 only w=\r
+ant to add it if it<br>\r
+> - =C2=A0 =C2=A0 =C2=A0 =C2=A0* links to a regular file, (and not to a =\r
+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 unknown ty=\r
+pe (due to filesystem<br>\r
+> - =C2=A0 =C2=A0 =C2=A0 =C2=A0* limitations), then we also need to look=\r
+ 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 trick.<=\r
+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 || entry->=\r
+;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 (notmuch=\r
+, "%s/%s", path, entry->d_name);<br>\r
+> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 err =3D stat (next, &st);<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 for a =\r
+link pointing nowhere, since<br>\r
+> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* the directory-traversal p=\r
+ass 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))<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) {<br>\r
+> + =C2=A0 =C2=A0 =C2=A0 /* Only add regular files (and symlinks to regu=\r
+lar files). */<br>\r
+> + =C2=A0 =C2=A0 =C2=A0 entry_type =3D dirent_type (path, entry);<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 read=\r
+ing 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 =C2=\r
+=A0path, entry->d_name, strerror (errno));</p>\r
+<p>Ditto.</p>\r
+<p>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return NOTMUCH_STATUS_FILE_ERR=\r
+OR;<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 aborts&quo=\r
+t;<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: No suc=\r
+h file or directory<br>\r
+> +"Error reading file /run/shm/nm/tmp.new/mail/broken: No such fil=\r
+e or directory<br>\r
+> =C2=A0Note: A fatal error was encountered: Something went wrong trying=\r
+ 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.org</a>=\r
+<br>\r
+> <a href=3D"http://notmuchmail.org/mailman/listinfo/notmuch">http://not=\r
+muchmail.org/mailman/listinfo/notmuch</a><br>\r
+</p>\r
+\r
+--047d7b33c9c65f7b8204bf78ac10--\r
projects / notmuch-archives.git / commitdiff