63/b90bc460f5ae7dae1b02ed7ca3656962073221

   1 Return-Path: <amthrax@drake.mit.edu>\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 1B1A9431FD8\r
   6         for <notmuch@notmuchmail.org>; Mon,  7 May 2012 15:20:49 -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.7\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
  12         tests=[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 pI5an2aEpY-n for <notmuch@notmuchmail.org>;\r
  16         Mon,  7 May 2012 15:20:48 -0700 (PDT)\r
  17 Received: from dmz-mailsec-scanner-5.mit.edu (DMZ-MAILSEC-SCANNER-5.MIT.EDU\r
  18         [18.7.68.34])\r
  19         by olra.theworths.org (Postfix) with ESMTP id 26825431FAE\r
  20         for <notmuch@notmuchmail.org>; Mon,  7 May 2012 15:20:48 -0700 (PDT)\r
  21 X-AuditID: 12074422-b7fd66d0000008f9-e1-4fa84abfe988\r
  22 Received: from mailhub-auth-2.mit.edu ( [18.7.62.36])\r
  23         by dmz-mailsec-scanner-5.mit.edu (Symantec Messaging Gateway) with SMTP\r
  24         id 0D.2D.02297.FBA48AF4; Mon,  7 May 2012 18:20:47 -0400 (EDT)\r
  25 Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103])\r
  26         by mailhub-auth-2.mit.edu (8.13.8/8.9.2) with ESMTP id q47MKld0013329; \r
  27         Mon, 7 May 2012 18:20:47 -0400\r
  28 Received: from drake.mit.edu (26-4-166.dynamic.csail.mit.edu [18.26.4.166])\r
  29         (authenticated bits=0)\r
  30         (User authenticated as amdragon@ATHENA.MIT.EDU)\r
  31         by outgoing.mit.edu (8.13.6/8.12.4) with ESMTP id q47MKkvs006042\r
  32         (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NOT);\r
  33         Mon, 7 May 2012 18:20:46 -0400 (EDT)\r
  34 Received: from amthrax by drake.mit.edu with local (Exim 4.77)\r
  35         (envelope-from <amthrax@drake.mit.edu>)\r
  36         id 1SRWIP-0000Ja-UO; Mon, 07 May 2012 18:20:45 -0400\r
  37 From: Austin Clements <amdragon@MIT.EDU>\r
  38 To: notmuch@notmuchmail.org\r
  39 Subject: [PATCH v2 2/2] new: Centralize file type stat-ing logic\r
  40 Date: Mon,  7 May 2012 18:20:40 -0400\r
  41 Message-Id: <1336429240-1114-3-git-send-email-amdragon@mit.edu>\r
  42 X-Mailer: git-send-email 1.7.10\r
  43 In-Reply-To: <1336429240-1114-1-git-send-email-amdragon@mit.edu>\r
  44 References: <1336414186-15293-1-git-send-email-amdragon@mit.edu>\r
  45         <1336429240-1114-1-git-send-email-amdragon@mit.edu>\r
  46 X-Brightmail-Tracker:\r
  47  H4sIAAAAAAAAA+NgFtrDIsWRmVeSWpSXmKPExsUixG6norvfa4W/warpihbXb85ktphxfheL\r
  48         A5PHs1W3mD2WHf3JGMAUxWWTkpqTWZZapG+XwJXx/eUGtoInRhV3l91mbGDcrt7FyMkhIWAi\r
  49         ce7ZdxYIW0ziwr31bF2MXBxCAvsYJebMWwLlrGeUaPp4H8o5xiSxb99rZghnPqPEj64GRpB+\r
  50         NgENiW37l4PZIgLSEjvvzmYFsZkFPCUOfJsGFhcWcJD4/OMAM4jNIqAqcXH9OSCbg4NXwF7i\r
  51         1zE/iDPkJZ7e72MDCXMClb+/ZA8SFhIok3h7+R/TBEb+BYwMqxhlU3KrdHMTM3OKU5N1i5MT\r
  52         8/JSi3RN9XIzS/RSU0o3MYIDyUVpB+PPg0qHGAU4GJV4eJVeLPcXYk0sK67MPcQoycGkJMp7\r
  53         0nOFvxBfUn5KZUZicUZ8UWlOavEhRgkOZiUR3jYxoBxvSmJlVWpRPkxKmoNFSZxXXeudn5BA\r
  54         emJJanZqakFqEUxWhoNDSYL3HMhQwaLU9NSKtMycEoQ0EwcnyHAeoOFHQWp4iwsSc4sz0yHy\r
  55         pxgVpcR5+0ASAiCJjNI8uF5YpL9iFAd6RZg3D6SKB5gk4LpfAQ1mAhq8+dkykMEliQgpqQbG\r
  56         Vf/7p7BHnHmRcSpE+O6T/XdXMFj/2by46pvi2ZYit7KskpkS2srrb56aeMGo98TG/4ydxu1P\r
  57         P8fZcetKXl18dT1D8g9pblXPV5cb909+7KbWviX/U/z3P+KZnbO/3xboe7DGX+zCfrl5G1jL\r
  58         mVbc2/fjj1Xwwd/Pvgs+j+wvX6h03bK83Px8iRJLcUaioRZzUXEiAJANzZHPAgAA\r
  59 Cc: Vladimir Marek <vlmarek@volny.cz>\r
  60 X-BeenThere: notmuch@notmuchmail.org\r
  61 X-Mailman-Version: 2.1.13\r
  62 Precedence: list\r
  63 List-Id: "Use and development of the notmuch mail system."\r
  64         <notmuch.notmuchmail.org>\r
  65 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  66         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  67 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  68 List-Post: <mailto:notmuch@notmuchmail.org>\r
  69 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  70 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  71         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  72 X-List-Received-Date: Mon, 07 May 2012 22:20:49 -0000\r
  73 \r
  74 This moves our logic to get a file's type into one function.  This has\r
  75 several benefits: we can support OSes and file systems that do not\r
  76 provide dirent.d_type or always return DT_UNKNOWN, complex\r
  77 symlink-handling logic has been replaced by a simple stat fall-through\r
  78 in one place, and the error message for un-stat-able file is more\r
  79 accurate (previously, the error always mentioned directories, even\r
  80 though a broken symlink is not a directory).\r
  81 ---\r
  82  notmuch-new.c |  103 +++++++++++++++++++++++++++++++++++----------------------\r
  83  test/new      |    2 +-\r
  84  2 files changed, 64 insertions(+), 41 deletions(-)\r
  85 \r
  86 diff --git a/notmuch-new.c b/notmuch-new.c\r
  87 index cb720cc..8955677 100644\r
  88 --- a/notmuch-new.c\r
  89 +++ b/notmuch-new.c\r
  90 @@ -154,6 +154,48 @@ dirent_sort_strcmp_name (const struct dirent **a, const struct dirent **b)\r
  91      return strcmp ((*a)->d_name, (*b)->d_name);\r
  92  }\r
  93  \r
  94 +/* Return the type of a directory entry relative to path as a stat(2)\r
  95 + * mode.  Like stat, this follows symlinks.  Returns -1 and sets errno\r
  96 + * if the file's type cannot be determined (which includes dangling\r
  97 + * symlinks).\r
  98 + */\r
  99 +static int\r
 100 +dirent_type (const char *path, const struct dirent *entry)\r
 101 +{\r
 102 +    struct stat statbuf;\r
 103 +    char *abspath;\r
 104 +    int err, saved_errno;\r
 105 +\r
 106 +#ifdef _DIRENT_HAVE_D_TYPE\r
 107 +    /* Mapping from d_type to stat mode_t.  We omit DT_LNK so that\r
 108 +     * we'll fall through to stat and get the real file type. */\r
 109 +    static const mode_t modes[] = {\r
 110 +       [DT_BLK]  = S_IFBLK,\r
 111 +       [DT_CHR]  = S_IFCHR,\r
 112 +       [DT_DIR]  = S_IFDIR,\r
 113 +       [DT_FIFO] = S_IFIFO,\r
 114 +       [DT_REG]  = S_IFREG,\r
 115 +       [DT_SOCK] = S_IFSOCK\r
 116 +    };\r
 117 +    if (entry->d_type < ARRAY_SIZE(modes) && modes[entry->d_type])\r
 118 +       return modes[entry->d_type];\r
 119 +#endif\r
 120 +\r
 121 +    abspath = talloc_asprintf (NULL, "%s/%s", path, entry->d_name);\r
 122 +    if (!abspath) {\r
 123 +       errno = ENOMEM;\r
 124 +       return -1;\r
 125 +    }\r
 126 +    err = stat(abspath, &statbuf);\r
 127 +    saved_errno = errno;\r
 128 +    talloc_free (abspath);\r
 129 +    if (err < 0) {\r
 130 +       errno = saved_errno;\r
 131 +       return -1;\r
 132 +    }\r
 133 +    return statbuf.st_mode & S_IFMT;\r
 134 +}\r
 135 +\r
 136  /* Test if the directory looks like a Maildir directory.\r
 137   *\r
 138   * Search through the array of directory entries to see if we can find all\r
 139 @@ -162,12 +204,12 @@ dirent_sort_strcmp_name (const struct dirent **a, const struct dirent **b)\r
 140   * Return 1 if the directory looks like a Maildir and 0 otherwise.\r
 141   */\r
 142  static int\r
 143 -_entries_resemble_maildir (struct dirent **entries, int count)\r
 144 +_entries_resemble_maildir (const char *path, struct dirent **entries, int count)\r
 145  {\r
 146      int i, found = 0;\r
 147  \r
 148      for (i = 0; i < count; i++) {\r
 149 -       if (entries[i]->d_type != DT_DIR && entries[i]->d_type != DT_UNKNOWN)\r
 150 +       if (dirent_type (path, entries[i]) != S_IFDIR)\r
 151             continue;\r
 152  \r
 153         if (strcmp(entries[i]->d_name, "new") == 0 ||\r
 154 @@ -250,7 +292,7 @@ add_files_recursive (notmuch_database_t *notmuch,\r
 155      notmuch_status_t status, ret = NOTMUCH_STATUS_SUCCESS;\r
 156      notmuch_message_t *message = NULL;\r
 157      struct dirent **fs_entries = NULL;\r
 158 -    int i, num_fs_entries;\r
 159 +    int i, num_fs_entries, entry_type;\r
 160      notmuch_directory_t *directory;\r
 161      notmuch_filenames_t *db_files = NULL;\r
 162      notmuch_filenames_t *db_subdirs = NULL;\r
 163 @@ -317,7 +359,7 @@ add_files_recursive (notmuch_database_t *notmuch,\r
 164      }\r
 165  \r
 166      /* Pass 1: Recurse into all sub-directories. */\r
 167 -    is_maildir = _entries_resemble_maildir (fs_entries, num_fs_entries);\r
 168 +    is_maildir = _entries_resemble_maildir (path, fs_entries, num_fs_entries);\r
 169  \r
 170      for (i = 0; i < num_fs_entries; i++) {\r
 171         if (interrupted)\r
 172 @@ -325,17 +367,16 @@ add_files_recursive (notmuch_database_t *notmuch,\r
 173  \r
 174         entry = fs_entries[i];\r
 175  \r
 176 -       /* We only want to descend into directories.\r
 177 -        * But symlinks can be to directories too, of course.\r
 178 -        *\r
 179 -        * And if the filesystem doesn't tell us the file type in the\r
 180 -        * scandir results, then it might be a directory (and if not,\r
 181 -        * then we'll stat and return immediately in the next level of\r
 182 -        * recursion). */\r
 183 -       if (entry->d_type != DT_DIR &&\r
 184 -           entry->d_type != DT_LNK &&\r
 185 -           entry->d_type != DT_UNKNOWN)\r
 186 -       {\r
 187 +       /* We only want to descend into directories (and symlinks to\r
 188 +        * directories). */\r
 189 +       entry_type = dirent_type (path, entry);\r
 190 +       if (entry_type == -1) {\r
 191 +           /* Be pessimistic, e.g. so we don't lose lots of mail just\r
 192 +            * because a user broke a symlink. */\r
 193 +           fprintf (stderr, "Error reading file %s/%s: %s\n",\r
 194 +                    path, entry->d_name, strerror (errno));\r
 195 +           return NOTMUCH_STATUS_FILE_ERROR;\r
 196 +       } else if (entry_type != S_IFDIR) {\r
 197             continue;\r
 198         }\r
 199  \r
 200 @@ -425,31 +466,13 @@ add_files_recursive (notmuch_database_t *notmuch,\r
 201             notmuch_filenames_move_to_next (db_subdirs);\r
 202         }\r
 203  \r
 204 -       /* If we're looking at a symlink, we only want to add it if it\r
 205 -        * links to a regular file, (and not to a directory, say).\r
 206 -        *\r
 207 -        * Similarly, if the file is of unknown type (due to filesystem\r
 208 -        * limitations), then we also need to look closer.\r
 209 -        *\r
 210 -        * In either case, a stat does the trick.\r
 211 -        */\r
 212 -       if (entry->d_type == DT_LNK || entry->d_type == DT_UNKNOWN) {\r
 213 -           int err;\r
 214 -\r
 215 -           next = talloc_asprintf (notmuch, "%s/%s", path, entry->d_name);\r
 216 -           err = stat (next, &st);\r
 217 -           talloc_free (next);\r
 218 -           next = NULL;\r
 219 -\r
 220 -           /* Don't emit an error for a link pointing nowhere, since\r
 221 -            * the directory-traversal pass will have already done\r
 222 -            * that. */\r
 223 -           if (err)\r
 224 -               continue;\r
 225 -\r
 226 -           if (! S_ISREG (st.st_mode))\r
 227 -               continue;\r
 228 -       } else if (entry->d_type != DT_REG) {\r
 229 +       /* Only add regular files (and symlinks to regular files). */\r
 230 +       entry_type = dirent_type (path, entry);\r
 231 +       if (entry_type == -1) {\r
 232 +           fprintf (stderr, "Error reading file %s/%s: %s\n",\r
 233 +                    path, entry->d_name, strerror (errno));\r
 234 +           return NOTMUCH_STATUS_FILE_ERROR;\r
 235 +       } else if (entry_type != S_IFREG) {\r
 236             continue;\r
 237         }\r
 238  \r
 239 diff --git a/test/new b/test/new\r
 240 index 26253db..e3900f5 100755\r
 241 --- a/test/new\r
 242 +++ b/test/new\r
 243 @@ -140,7 +140,7 @@ test_begin_subtest "Broken symlink aborts"\r
 244  ln -s does-not-exist "${MAIL_DIR}/broken"\r
 245  output=$(NOTMUCH_NEW 2>&1)\r
 246  test_expect_equal "$output" \\r
 247 -"Error reading directory /run/shm/nm/tmp.new/mail/broken: No such file or directory\r
 248 +"Error reading file /run/shm/nm/tmp.new/mail/broken: No such file or directory\r
 249  Note: A fatal error was encountered: Something went wrong trying to read or write a file\r
 250  No new mail."\r
 251  rm "${MAIL_DIR}/broken"\r
 252 -- \r
 253 1.7.10\r
 254 \r