From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by olra.theworths.org (Postfix) with ESMTP id 90861431FAF for ; Wed, 9 May 2012 11:27:09 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at olra.theworths.org X-Spam-Flag: NO X-Spam-Score: -0.7 X-Spam-Level: X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5 tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled Received: from olra.theworths.org ([127.0.0.1]) by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id h-5sASKm1vMf for ; Wed, 9 May 2012 11:27:09 -0700 (PDT) Received: from dmz-mailsec-scanner-7.mit.edu (DMZ-MAILSEC-SCANNER-7.MIT.EDU [18.7.68.36]) by olra.theworths.org (Postfix) with ESMTP id D3605431FAE for ; Wed, 9 May 2012 11:27:08 -0700 (PDT) X-AuditID: 12074424-b7fae6d000000906-a6-4faab6fce94b Received: from mailhub-auth-3.mit.edu ( [18.9.21.43]) by dmz-mailsec-scanner-7.mit.edu (Symantec Messaging Gateway) with SMTP id 5D.0B.02310.CF6BAAF4; Wed, 9 May 2012 14:27:08 -0400 (EDT) Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103]) by mailhub-auth-3.mit.edu (8.13.8/8.9.2) with ESMTP id q49IR7TF007049; Wed, 9 May 2012 14:27:07 -0400 Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91]) (authenticated bits=0) (User authenticated as amdragon@ATHENA.MIT.EDU) by outgoing.mit.edu (8.13.6/8.12.4) with ESMTP id q49IR5UK007115 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NOT); Wed, 9 May 2012 14:27:06 -0400 (EDT) Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.77) (envelope-from ) id 1SSBbN-00074t-0L; Wed, 09 May 2012 14:27:05 -0400 Date: Wed, 9 May 2012 14:27:04 -0400 From: Austin Clements To: Jani Nikula Subject: Re: [PATCH v2 2/2] new: Centralize file type stat-ing logic Message-ID: <20120509182704.GC11804@mit.edu> References: <1336414186-15293-1-git-send-email-amdragon@mit.edu> <1336429240-1114-1-git-send-email-amdragon@mit.edu> <1336429240-1114-3-git-send-email-amdragon@mit.edu> <87r4uvdryz.fsf@nikula.org> <87obpzdqcz.fsf@nikula.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87obpzdqcz.fsf@nikula.org> User-Agent: Mutt/1.5.21 (2010-09-15) X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFuphleLIzCtJLcpLzFFi42IR4hTV1v2zbZW/wa3f0hZN050trt+cyWwx 4/wuFgdmj1v3X7N7PFt1i9lj2dGfjAHMUVw2Kak5mWWpRfp2CVwZry5+Yy5o4K3Y/2U/ewPj eq4uRk4OCQETiUW7L7FA2GISF+6tZwOxhQT2MUpcXMbYxcgFZK9nlFj8/hEThHOCSaLl3kE2 CGcJo8TV5n3sIC0sAioSf+7+YwKx2QQ0JLbtX84IYosIKEpsPrkfzGYWsJOY9uIYWI2wgIvE vNO9QHEODl4BHYlF+2QhZr5glGg5+wTsDF4BQYmTM5+wQPRqSdz495IJpJ5ZQFpi+T8OkDAn 0KqH946DlYgCnTDl5Da2CYxCs5B0z0LSPQuhewEj8ypG2ZTcKt3cxMyc4tRk3eLkxLy81CJd c73czBK91JTSTYygQGd3UdnB2HxI6RCjAAejEg+vVMsqfyHWxLLiytxDjJIcTEqivIc3A4X4 kvJTKjMSizPii0pzUosPMUpwMCuJ8N5dBZTjTUmsrEotyodJSXOwKInzami98xMSSE8sSc1O TS1ILYLJynBwKEnw/t4K1ChYlJqeWpGWmVOCkGbi4AQZzgM0fDtIDW9xQWJucWY6RP4Uo6KU OO8nkIQASCKjNA+uF5aIXjGKA70izMsMTEtCPMAkBtf9CmgwE9DgaYdXggwuSURISTUwVvd9 fFNpeW3C/zl3rzye9eBkruZcqWtKhopLPITdurqsG9oz2ef5ZnHd33l+b+O9Fj4ZQ9na3+W7 3p/QSrOLm/ZS3thZT+91HN/9GM1Dj19tVk5YbJHtWTeN6fWek7PEXbNPTH6i++fpnF2rXzfH Gle6SRo5RoR+Wxm9Ke3w2ooix/miDXrXlFiKMxINtZiLihMBtNrY7R8DAAA= Cc: notmuch@notmuchmail.org, Vladimir Marek X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 09 May 2012 18:27:09 -0000 Quoth Jani Nikula on May 08 at 8:33 am: > On Tue, 08 May 2012 07:58:28 +0000, Jani Nikula wrote: > > On Mon, 7 May 2012 18:20:40 -0400, Austin Clements wrote: > > > This moves our logic to get a file's type into one function. This has > > > several benefits: we can support OSes and file systems that do not > > > provide dirent.d_type or always return DT_UNKNOWN, complex > > > symlink-handling logic has been replaced by a simple stat fall-through > > > in one place, and the error message for un-stat-able file is more > > > accurate (previously, the error always mentioned directories, even > > > though a broken symlink is not a directory). > > > > LGTM. > > Okay, it's good, but I think you can make it even better: > > add_files_recursive() has check for "! S_ISDIR (st.st_mode)" in the > beginning, returning silently in case it recursed based on a symlink to > regular file. IIUC, this will no longer happen with your patch, as > symlinks are resolved and stat'ed before recursing. > > add_files() exists to fail loudly in the same situation, and has > otherwise the same checks in the beginning. I think you could now use > the checks from add_files() to replace the ones in > add_files_recursive(), and get rid of add_files() altogether. > > Please double check my thinking. Also this should probably be a separate > patch, no need to change the current one. Excellent idea. It works fantastically. I'll wait to send the patches until this series gets pushed, both the avoid dependent series confusion and so I can refer to the appropriate commit ID in the commit message.