Re: [Musicpd-dev-team] FS improvements + refinements in PollGroupWinSelect
I've pushed implementation of standard directories API. It does not support non-POSIX getpwnam_r/getpwuid_r yet, because I don't know how to test it. Please take a look. CommandLine: refactor config search, use standard directory API Main: use standard directory API ConfigPath: use standard directory API fs: implemented standard directories API TextFile: move to fs subsystem -- 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=111408631iu=/4140/ostg.clktrk ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] FS improvements + refinements in PollGroupWinSelect
Few more improvements + fix for starting with missing db file on Windows fs/FileSystem.hxx: don't define CheckAccess() with mode on Windows db/SimpleDatabasePlugin.cxx: don't use CheckAccess with mode on Windows fs/FileSystem.hxx: add CheckAccess without mode parameter fs/Traits.cxx: don't return drive path without trailing separator fs/Traits.hxx: introduce PathTraitsXXX::IsDrive function fs/Charset.cxx: replace \ with / when converting path to UTF-8 on Windows PlaylistSong.cxx: don't use g_build_filename fs/Traits.hxx: add shorter forms of PathTraitsXXX::Build() fs/Traits.hxx: add PathTraitsUTF8::GetLength for consistency fs/AllocatedPath.cxx: don't use g_path_get_dirname fs/Traits: implement GetBase/GetParent/Build using templates fs/Traits.cxx: don't return empty string if parent dir is root fs/Traits.hxx: add FindLastSeparator function to PathTraitsXXX fs/Traits: improve compatibility between PathTraitsFS and PathTraitsUTF8 fs/Traits.hxx: add gcc_nonnull_all where applicable fs/Traits: split PathTraits type into PathTraitsFS and PathTraitsUTF8 -- Denis -- 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=111408631iu=/4140/ostg.clktrk ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] FS improvements + refinements in PollGroupWinSelect
Next part is reimplementing of what I call standard directories API. g_get_system_config_dirs g_get_user_config_dir g_get_home_dir g_get_user_special_dir(G_USER_DIRECTORY_MUSIC) Windows part looks easy there are plenty of standard functions with different levels of obsolescence for that. For Unix version I need some assistance. g_get_home_dir() works differently depending on GLib version. Newer GLib versions check $HOME first and if it's empty use getpwnam_r() to obtain home directory for currently user. Old versions go straight into using getpwnam_r(). What approach should we use? Second issue: GLib uses two flavors of getpwnam_r() (guarded by HAVE_POSIX_GETPWUID_R and HAVE_NONPOSIX_GETPWUID_R in the code). What should we do with that? https://developer.gnome.org/glib/2.38/glib-Miscellaneous-Utility-Functions.html#g-get-home-dir https://git.gnome.org/browse/glib/tree/glib/gutils.c For reimplementing g_get_user_special_dir(G_USER_DIRECTORY_MUSIC) I think we need to parse user-dirs.dirs file. I didn't any reference documentation on this. However I've found the source http://cgit.freedesktop.org/xdg/xdg-user-dirs/tree/xdg-user-dir-lookup.c I think this could be reused as-is, it does not look very efficient, but this function is going to be called at most once after all. One more question what encoding is this file supposed to have? Bonus question: In the MPD code AllocatedPath::FromUTF8() is called on the results of GLib calls, however I did not find any conversion of encoding in the GLib code (see gutils.c code above). Maybe I'm missing something or maybe this is a bug. -- Denis -- 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=111408631iu=/4140/ostg.clktrk ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] FS improvements + refinements in PollGroupWinSelect
On Thu, Dec 05, 2013 at 19:17:56 +0600, Denis Krjuchkov wrote: g_get_home_dir() works differently depending on GLib version. Newer GLib versions check $HOME first and if it's empty use getpwnam_r() to obtain home directory for currently user. Old versions go straight into using getpwnam_r(). What approach should we use? Please use $HOME then fall back to getpwnam_r(). I have a user with the same uid/gid, but a different name and $HOME (mainly to put games into a special root without a separate login), but getpwnam_r() gets my main user's $HOME because it's earlier in the passwd file. --Ben -- 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=111408631iu=/4140/ostg.clktrk ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] FS improvements + refinements in PollGroupWinSelect
On 2013/12/05 17:10, Ben Boeckel maths...@gmail.com wrote: On Thu, Dec 05, 2013 at 19:17:56 +0600, Denis Krjuchkov wrote: g_get_home_dir() works differently depending on GLib version. Newer GLib versions check $HOME first and if it's empty use getpwnam_r() to obtain home directory for currently user. Old versions go straight into using getpwnam_r(). What approach should we use? Please use $HOME then fall back to getpwnam_r(). Agree. $HOME is the nearest variable, one that can be customized easily by users. One can not override /etc/passwd entries easily. -- 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=111408631iu=/4140/ostg.clktrk ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] FS improvements + refinements in PollGroupWinSelect
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).
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.
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.
--
Denis
--
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=111408631iu=/4140/ostg.clktrk
___
Musicpd-dev-team mailing list
Musicpd-dev-team@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] FS improvements + refinements in PollGroupWinSelect
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=111408631iu=/4140/ostg.clktrk
___
Musicpd-dev-team mailing list
Musicpd-dev-team@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] FS improvements + refinements in PollGroupWinSelect
On 2013/12/03 09:58, Denis Krjuchkov de...@crazydev.net wrote: I think you're right. Pushed a fix for that. Merged. -- Rapidly troubleshoot problems before they affect your business. Most IT organizations don't have a clear picture of how application performance affects their revenue. With AppDynamics, you get 100% visibility into your Java,.NET, PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro! http://pubads.g.doubleclick.net/gampad/clk?id=84349351iu=/4140/ostg.clktrk ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
