I included two separate patches, one that fixes the bug and one that adds
functionality. They're in two different files with different names so I
hope that clears up the separation between the bug fix and the extra
feature.
Anyway, std::string doesn't have to determine it's size, it knows it
already. There's no real difference between empty() and !size() except that
empty() is slightly more readable, so I agree I should have used that.
However, I don't see why you would use glib for string manipulation when
there are perfectly fine <string> and <algorithm> headers that comes with
C++. These don't require you to call g_free manually and don't cause memory
leaks when you forget to anyway. That's one of the advantages of using C++.
Besides, I would hardly call using the standard library "extra bloat". And
if using standard C++ headers can lead to phasing out glib completely, the
result is actually less bloat.
As for extra overhead, std::string is usually very efficient and ovoids
actual copying unless it actually has to. More importantly though, we're
talking about reading the configuration file which happens once at startup
(with about 6 paths in the config file), and the length of the string
involved is probably around 20-30 characters.
Of course, if you want to fix it your way, who am I to protest? But that
still leaves the second patch which only allows tildes in socket paths.
What about that?
Kind regards,
Maarten de Vries
On 12 September 2013 08:32, Max Kellermann <m...@duempel.org> wrote:
> On 2013/09/05 21:01, Maarten de Vries <maar...@de-vri.es> wrote:
> > While patching this in, I also noticed that tilde paths weren't properly
> > parsed in the latest git version. For example: `~/.mpd/database' became
> > `/home/maarten/~/.mpd/database'. I also fixed that by mostly replacing
> the
> > old function. The version in my patch uses std::string and iterators
> rather
> > than old C strings and C functions. It's C++ now after all and I found it
> > much easier that way.
>
> This patch looks VERY confusing because you mixed two different things
> in one patch. One is the rewrite, the other is the bug fix.
>
> I don't even see an advantage of using std::string here (the
> disadvantage is increased bloat). The code doesn't get smaller or
> more readable by using std::string, it gets only smaller because you
> merged the default user / explicit user branches.
>
> This line:
>
> std::string user(user_start, user_end);
>
> is the equivalent of the old:
>
> g_strndup(path, slash - path)
>
> No difference in readability.
>
> What I don't understand is why you allocate memory for a second copy
> of "path". The original code avoids that.
>
> Btw., this:
>
> if (!user.size()) {
>
> is more bloated and less readable than:
>
> if (user.empty()) {
>
> .. because it forces the std::string to determine the size, and the
> latter does not.
>
> Max
>
------------------------------------------------------------------------------
How ServiceNow helps IT people transform IT departments:
1. Consolidate legacy IT systems to a single system of record for IT
2. Standardize and globalize service processes across IT
3. Implement zero-touch automation to replace manual, redundant tasks
http://pubads.g.doubleclick.net/gampad/clk?id=51271111&iu=/4140/ostg.clktrk
_______________________________________________
Musicpd-dev-team mailing list
Musicpd-dev-team@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
