[PATCH v2 2/2] new: Centralize file type stat-ing logic

[Austin Clements]

Quoth Jani Nikula on May 08 at  8:33 am:
> On Tue, 08 May 2012 07:58:28 +0000, Jani Nikula <jani at nikula.org> wrote:
> > On Mon,  7 May 2012 18:20:40 -0400, Austin Clements <amdragon at MIT.EDU> 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.