07/69e7bfeaa4e49c3f72aad77b89e2b1c18d7297

   1 Return-Path: <amdragon@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 90861431FAF\r
   6         for <notmuch@notmuchmail.org>; Wed,  9 May 2012 11:27:09 -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 h-5sASKm1vMf for <notmuch@notmuchmail.org>;\r
  16         Wed,  9 May 2012 11:27:09 -0700 (PDT)\r
  17 Received: from dmz-mailsec-scanner-7.mit.edu (DMZ-MAILSEC-SCANNER-7.MIT.EDU\r
  18         [18.7.68.36])\r
  19         by olra.theworths.org (Postfix) with ESMTP id D3605431FAE\r
  20         for <notmuch@notmuchmail.org>; Wed,  9 May 2012 11:27:08 -0700 (PDT)\r
  21 X-AuditID: 12074424-b7fae6d000000906-a6-4faab6fce94b\r
  22 Received: from mailhub-auth-3.mit.edu ( [18.9.21.43])\r
  23         by dmz-mailsec-scanner-7.mit.edu (Symantec Messaging Gateway) with SMTP\r
  24         id 5D.0B.02310.CF6BAAF4; Wed,  9 May 2012 14:27:08 -0400 (EDT)\r
  25 Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103])\r
  26         by mailhub-auth-3.mit.edu (8.13.8/8.9.2) with ESMTP id q49IR7TF007049; \r
  27         Wed, 9 May 2012 14:27:07 -0400\r
  28 Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91])\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 q49IR5UK007115\r
  32         (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NOT);\r
  33         Wed, 9 May 2012 14:27:06 -0400 (EDT)\r
  34 Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.77)\r
  35         (envelope-from <amdragon@mit.edu>)\r
  36         id 1SSBbN-00074t-0L; Wed, 09 May 2012 14:27:05 -0400\r
  37 Date: Wed, 9 May 2012 14:27:04 -0400\r
  38 From: Austin Clements <amdragon@MIT.EDU>\r
  39 To: Jani Nikula <jani@nikula.org>\r
  40 Subject: Re: [PATCH v2 2/2] new: Centralize file type stat-ing logic\r
  41 Message-ID: <20120509182704.GC11804@mit.edu>\r
  42 References: <1336414186-15293-1-git-send-email-amdragon@mit.edu>\r
  43         <1336429240-1114-1-git-send-email-amdragon@mit.edu>\r
  44         <1336429240-1114-3-git-send-email-amdragon@mit.edu>\r
  45         <87r4uvdryz.fsf@nikula.org> <87obpzdqcz.fsf@nikula.org>\r
  46 MIME-Version: 1.0\r
  47 Content-Type: text/plain; charset=us-ascii\r
  48 Content-Disposition: inline\r
  49 In-Reply-To: <87obpzdqcz.fsf@nikula.org>\r
  50 User-Agent: Mutt/1.5.21 (2010-09-15)\r
  51 X-Brightmail-Tracker:\r
  52  H4sIAAAAAAAAA+NgFuphleLIzCtJLcpLzFFi42IR4hTV1v2zbZW/wa3f0hZN050trt+cyWwx\r
  53         4/wuFgdmj1v3X7N7PFt1i9lj2dGfjAHMUVw2Kak5mWWpRfp2CVwZry5+Yy5o4K3Y/2U/ewPj\r
  54         eq4uRk4OCQETiUW7L7FA2GISF+6tZwOxhQT2MUpcXMbYxcgFZK9nlFj8/hEThHOCSaLl3kE2\r
  55         CGcJo8TV5n3sIC0sAioSf+7+YwKx2QQ0JLbtX84IYosIKEpsPrkfzGYWsJOY9uIYWI2wgIvE\r
  56         vNO9QHEODl4BHYlF+2QhZr5glGg5+wTsDF4BQYmTM5+wQPRqSdz495IJpJ5ZQFpi+T8OkDAn\r
  57         0KqH946DlYgCnTDl5Da2CYxCs5B0z0LSPQuhewEj8ypG2ZTcKt3cxMyc4tRk3eLkxLy81CJd\r
  58         c73czBK91JTSTYygQGd3UdnB2HxI6RCjAAejEg+vVMsqfyHWxLLiytxDjJIcTEqivIc3A4X4\r
  59         kvJTKjMSizPii0pzUosPMUpwMCuJ8N5dBZTjTUmsrEotyodJSXOwKInzami98xMSSE8sSc1O\r
  60         TS1ILYLJynBwKEnw/t4K1ChYlJqeWpGWmVOCkGbi4AQZzgM0fDtIDW9xQWJucWY6RP4Uo6KU\r
  61         OO8nkIQASCKjNA+uF5aIXjGKA70izMsMTEtCPMAkBtf9CmgwE9DgaYdXggwuSURISTUwVvd9\r
  62         fFNpeW3C/zl3rzye9eBkruZcqWtKhopLPITdurqsG9oz2ef5ZnHd33l+b+O9Fj4ZQ9na3+W7\r
  63         3p/QSrOLm/ZS3thZT+91HN/9GM1Dj19tVk5YbJHtWTeN6fWek7PEXbNPTH6i++fpnF2rXzfH\r
  64         Gle6SRo5RoR+Wxm9Ke3w2ooix/miDXrXlFiKMxINtZiLihMBtNrY7R8DAAA=\r
  65 Cc: notmuch@notmuchmail.org, Vladimir Marek <vlmarek@volny.cz>\r
  66 X-BeenThere: notmuch@notmuchmail.org\r
  67 X-Mailman-Version: 2.1.13\r
  68 Precedence: list\r
  69 List-Id: "Use and development of the notmuch mail system."\r
  70         <notmuch.notmuchmail.org>\r
  71 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  72         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  73 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  74 List-Post: <mailto:notmuch@notmuchmail.org>\r
  75 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  76 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  77         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  78 X-List-Received-Date: Wed, 09 May 2012 18:27:09 -0000\r
  79 \r
  80 Quoth Jani Nikula on May 08 at  8:33 am:\r
  81 > On Tue, 08 May 2012 07:58:28 +0000, Jani Nikula <jani@nikula.org> wrote:\r
  82 > > On Mon,  7 May 2012 18:20:40 -0400, Austin Clements <amdragon@MIT.EDU> wrote:\r
  83 > > > This moves our logic to get a file's type into one function.  This has\r
  84 > > > several benefits: we can support OSes and file systems that do not\r
  85 > > > provide dirent.d_type or always return DT_UNKNOWN, complex\r
  86 > > > symlink-handling logic has been replaced by a simple stat fall-through\r
  87 > > > in one place, and the error message for un-stat-able file is more\r
  88 > > > accurate (previously, the error always mentioned directories, even\r
  89 > > > though a broken symlink is not a directory).\r
  90 > > \r
  91 > > LGTM.\r
  92 > \r
  93 > Okay, it's good, but I think you can make it even better:\r
  94 > \r
  95 > add_files_recursive() has check for "! S_ISDIR (st.st_mode)" in the\r
  96 > beginning, returning silently in case it recursed based on a symlink to\r
  97 > regular file. IIUC, this will no longer happen with your patch, as\r
  98 > symlinks are resolved and stat'ed before recursing.\r
  99 > \r
 100 > add_files() exists to fail loudly in the same situation, and has\r
 101 > otherwise the same checks in the beginning. I think you could now use\r
 102 > the checks from add_files() to replace the ones in\r
 103 > add_files_recursive(), and get rid of add_files() altogether.\r
 104 > \r
 105 > Please double check my thinking. Also this should probably be a separate\r
 106 > patch, no need to change the current one.\r
 107 \r
 108 Excellent idea.  It works fantastically.  I'll wait to send the\r
 109 patches until this series gets pushed, both the avoid dependent series\r
 110 confusion and so I can refer to the appropriate commit ID in the\r
 111 commit message.\r