Re: [PATCH v2 2/2] new: Centralize file type stat-ing logic
authorAustin Clements <amdragon@MIT.EDU>
Wed, 9 May 2012 18:27:04 +0000 (14:27 +2000)
committerW. Trevor King <wking@tremily.us>
Fri, 7 Nov 2014 17:47:01 +0000 (09:47 -0800)
07/69e7bfeaa4e49c3f72aad77b89e2b1c18d7297 [new file with mode: 0644]

diff --git a/07/69e7bfeaa4e49c3f72aad77b89e2b1c18d7297 b/07/69e7bfeaa4e49c3f72aad77b89e2b1c18d7297
new file mode 100644 (file)
index 0000000..001b616
--- /dev/null
@@ -0,0 +1,111 @@
+Return-Path: <amdragon@mit.edu>\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 90861431FAF\r
+       for <notmuch@notmuchmail.org>; Wed,  9 May 2012 11:27:09 -0700 (PDT)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: -0.7\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
+       tests=[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 h-5sASKm1vMf for <notmuch@notmuchmail.org>;\r
+       Wed,  9 May 2012 11:27:09 -0700 (PDT)\r
+Received: from dmz-mailsec-scanner-7.mit.edu (DMZ-MAILSEC-SCANNER-7.MIT.EDU\r
+       [18.7.68.36])\r
+       by olra.theworths.org (Postfix) with ESMTP id D3605431FAE\r
+       for <notmuch@notmuchmail.org>; Wed,  9 May 2012 11:27:08 -0700 (PDT)\r
+X-AuditID: 12074424-b7fae6d000000906-a6-4faab6fce94b\r
+Received: from mailhub-auth-3.mit.edu ( [18.9.21.43])\r
+       by dmz-mailsec-scanner-7.mit.edu (Symantec Messaging Gateway) with SMTP\r
+       id 5D.0B.02310.CF6BAAF4; Wed,  9 May 2012 14:27:08 -0400 (EDT)\r
+Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103])\r
+       by mailhub-auth-3.mit.edu (8.13.8/8.9.2) with ESMTP id q49IR7TF007049; \r
+       Wed, 9 May 2012 14:27:07 -0400\r
+Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91])\r
+       (authenticated bits=0)\r
+       (User authenticated as amdragon@ATHENA.MIT.EDU)\r
+       by outgoing.mit.edu (8.13.6/8.12.4) with ESMTP id q49IR5UK007115\r
+       (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NOT);\r
+       Wed, 9 May 2012 14:27:06 -0400 (EDT)\r
+Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.77)\r
+       (envelope-from <amdragon@mit.edu>)\r
+       id 1SSBbN-00074t-0L; Wed, 09 May 2012 14:27:05 -0400\r
+Date: Wed, 9 May 2012 14:27:04 -0400\r
+From: Austin Clements <amdragon@MIT.EDU>\r
+To: Jani Nikula <jani@nikula.org>\r
+Subject: Re: [PATCH v2 2/2] new: Centralize file type stat-ing logic\r
+Message-ID: <20120509182704.GC11804@mit.edu>\r
+References: <1336414186-15293-1-git-send-email-amdragon@mit.edu>\r
+       <1336429240-1114-1-git-send-email-amdragon@mit.edu>\r
+       <1336429240-1114-3-git-send-email-amdragon@mit.edu>\r
+       <87r4uvdryz.fsf@nikula.org> <87obpzdqcz.fsf@nikula.org>\r
+MIME-Version: 1.0\r
+Content-Type: text/plain; charset=us-ascii\r
+Content-Disposition: inline\r
+In-Reply-To: <87obpzdqcz.fsf@nikula.org>\r
+User-Agent: Mutt/1.5.21 (2010-09-15)\r
+X-Brightmail-Tracker:\r
+ H4sIAAAAAAAAA+NgFuphleLIzCtJLcpLzFFi42IR4hTV1v2zbZW/wa3f0hZN050trt+cyWwx\r
+       4/wuFgdmj1v3X7N7PFt1i9lj2dGfjAHMUVw2Kak5mWWpRfp2CVwZry5+Yy5o4K3Y/2U/ewPj\r
+       eq4uRk4OCQETiUW7L7FA2GISF+6tZwOxhQT2MUpcXMbYxcgFZK9nlFj8/hEThHOCSaLl3kE2\r
+       CGcJo8TV5n3sIC0sAioSf+7+YwKx2QQ0JLbtX84IYosIKEpsPrkfzGYWsJOY9uIYWI2wgIvE\r
+       vNO9QHEODl4BHYlF+2QhZr5glGg5+wTsDF4BQYmTM5+wQPRqSdz495IJpJ5ZQFpi+T8OkDAn\r
+       0KqH946DlYgCnTDl5Da2CYxCs5B0z0LSPQuhewEj8ypG2ZTcKt3cxMyc4tRk3eLkxLy81CJd\r
+       c73czBK91JTSTYygQGd3UdnB2HxI6RCjAAejEg+vVMsqfyHWxLLiytxDjJIcTEqivIc3A4X4\r
+       kvJTKjMSizPii0pzUosPMUpwMCuJ8N5dBZTjTUmsrEotyodJSXOwKInzami98xMSSE8sSc1O\r
+       TS1ILYLJynBwKEnw/t4K1ChYlJqeWpGWmVOCkGbi4AQZzgM0fDtIDW9xQWJucWY6RP4Uo6KU\r
+       OO8nkIQASCKjNA+uF5aIXjGKA70izMsMTEtCPMAkBtf9CmgwE9DgaYdXggwuSURISTUwVvd9\r
+       fFNpeW3C/zl3rzye9eBkruZcqWtKhopLPITdurqsG9oz2ef5ZnHd33l+b+O9Fj4ZQ9na3+W7\r
+       3p/QSrOLm/ZS3thZT+91HN/9GM1Dj19tVk5YbJHtWTeN6fWek7PEXbNPTH6i++fpnF2rXzfH\r
+       Gle6SRo5RoR+Wxm9Ke3w2ooix/miDXrXlFiKMxINtZiLihMBtNrY7R8DAAA=\r
+Cc: 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: Wed, 09 May 2012 18:27:09 -0000\r
+\r
+Quoth Jani Nikula on May 08 at  8:33 am:\r
+> On Tue, 08 May 2012 07:58:28 +0000, Jani Nikula <jani@nikula.org> wrote:\r
+> > On Mon,  7 May 2012 18:20:40 -0400, Austin Clements <amdragon@MIT.EDU> wrote:\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
+> > LGTM.\r
+> \r
+> Okay, it's good, but I think you can make it even better:\r
+> \r
+> add_files_recursive() has check for "! S_ISDIR (st.st_mode)" in the\r
+> beginning, returning silently in case it recursed based on a symlink to\r
+> regular file. IIUC, this will no longer happen with your patch, as\r
+> symlinks are resolved and stat'ed before recursing.\r
+> \r
+> add_files() exists to fail loudly in the same situation, and has\r
+> otherwise the same checks in the beginning. I think you could now use\r
+> the checks from add_files() to replace the ones in\r
+> add_files_recursive(), and get rid of add_files() altogether.\r
+> \r
+> Please double check my thinking. Also this should probably be a separate\r
+> patch, no need to change the current one.\r
+\r
+Excellent idea.  It works fantastically.  I'll wait to send the\r
+patches until this series gets pushed, both the avoid dependent series\r
+confusion and so I can refer to the appropriate commit ID in the\r
+commit message.\r