20.01.2013 22:14, Max Kellermann пишет:
>
> - FileSystem.hxx includes all relevant system headers, and I would
>    like to reduce header dependencies as much as possible; include
>    special things like dirent.h only if you really want to read
>    directory contents

Agree, I think we could extract DirectoryReader to the separate header 
and also introduce new subdir in source tree, i.e.:

src/
        fs/
                FileSystem.hxx
                FileSystem.cxx
                DirectoryReader.hxx

>
> - ReadLink() should better return a "Path" object, and Path::Null() on
>    error

I've done it so initially, but what would happen in the following situation:

// Initializing p1 with some non-null value
Path p1 = ...;

// Reading the link and suppose ReadLink returns Path::Null()
p1 = ReadLink(...);

In this case old value of p1 would destroyed via free(), which might 
under some condition overwrite our errno.

> - CheckIsRegular(): for me, a name like IsRegularFile() sounds better

Agree, those names are invented under effects of reading g_file_test() 
documentation :-)

Couldn't it be something like the following?

   PathExists()/FileExists()/DirectoryExists()

Looks clean and symmetric.

> - slightly disagree with Ben (but thanks for pointing out that
>    potential problem): there should be some way to specify whether
>    symbolic links shall be followed.  Some callers may want to follow
>    symlinks, some do not, and the database update has that configurable
>    at runtime.

Approach with extra parameter (i.e. follow_symlinks) seems OK for me.
However I'm not sure which variant should be default.

-- 
Denis

------------------------------------------------------------------------------
Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
MVPs and experts. ON SALE this month only -- learn more at:
http://p.sf.net/sfu/learnmore_123012
_______________________________________________
Musicpd-dev-team mailing list
Musicpd-dev-team@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team

Reply via email to