On 2013/12/04 09:51, Denis Krjuchkov <de...@crazydev.net> wrote:
> I'm going to replace remaing GLib path management functions.
>
> However I have some thoughts at first:
>
> I think PathTraits type could be split into two types.
> One for native paths and one for UTF-8 paths.
> They could have similar symbol names (e.g. Build, IsAbsolute).
> This would allow writing more general path management algorithms using
> templates with specific traits template parameter.
> On contrary there would be only few functions so copy-and-paste solution
> might no so bad (easier to read, faster compile, etc).
Merging duplicate code is good, even if it's just about a few lines.
Compile times don't matter a lot. I chose C++ even though C++ is a
lot slower than C (at compile time, not at runtime).
We need to be careful when to merge code, because sometimes, there are
differences; for example, UTF-8 strings shall always use slash and
never backslash. That is not currently documented or enforced, but
may be a good idea to implement for better Windows compatibility.
UTF-8 paths shall be "portable" paths, while FS paths may have OS
specifics.
> This function looks suspicious for me:
>
> std::string
> PathTraits::GetParentUTF8(const char *p)
> {
> assert(p != nullptr);
>
> const char *slash = strrchr(p, SEPARATOR_UTF8);
> return slash != nullptr
> ? std::string(p, slash)
> : std::string(".");
> }
>
> If given rooted path (for example '/foo') it would return empty string
> which is not valid path. This might be OK for MPD virtual UTF-8 paths,
> however it's definitely not OK for real paths because empty string is
> never a valid directory. This leaves me in twofold situation.
> If I implement GetParentFS() to return '/' if given rooted path it would
> be inconsistent with GetParentUTF8(). If I change GetParentUTF8() to
> return '/' if given rooted path I'm not sure if this won't break
> anything in database operation and other places.
The root directory is some sort of special case that was not
considered here, you're right. It is special because it ends with a
slash, while "normal" directory names should not. Care must be taken
to avoid double slashes.
> One more suspicious thing is PathToUTF8() function:
> On Windows we accept both '/' and '\' as path separators.
> However UTF-8 paths always use '/'. This means if path that contains
> backslashes is converted to UTF-8 it can not be safely returned to
> client because it would treated incorrectly (I assume MPD protocol does
> not have special meaning of backslash). From what I saw in the code this
> functions is mostly used in logging, which is safe. But I'm not sure if
> there are no hidden bugs in this logic.
The backslash is the escape character for arguments in the MPD
protocol.
A client should never see a backslash as a path separator. So yes, it
should be converted (see above).
------------------------------------------------------------------------------
Sponsored by Intel(R) XDK
Develop, test and display web and hybrid apps with a single code base.
Download it for free now!
http://pubads.g.doubleclick.net/gampad/clk?id=111408631&iu=/4140/ostg.clktrk
_______________________________________________
Musicpd-dev-team mailing list
Musicpd-dev-team@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
