This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFC 2/7] Add libiberty/concat styled concat_path function
On 01/12/2017 01:33 PM, Philipp Rudo wrote:
> Hi Pedro,
>
> i see your point.
>
> My goal here was to get rid of any C-string. While making this patch i
> also wanted to use it to get rid of all those
>
> concat (path, need_dirsep ? SLASH_STRING : "", NULL)
>
> or
>
> strcat (path, "/")
> strcat (path, file)
>
> constructs. I gave up when it repeatedly caused memory leaks and use
> after free errors because of the mixture of C and C++ strings. Fixing
> them made the code less readable than before. Thus you should only use
> one kind of string through out GDB, either char * or std::string. And
> as GDB decided to move to C++ for me std::string is the way you should
> go.
Even if we used std::string throughout, we should still be careful
with unnecessary string copying. "std::string" vs "const string &"
in function parameters (use the former only when the function already
needs to work with a copy).
Similarly, please don't write:
+ for (std::string arg: args)
+ {
Please write instead:
for (const std::string &arg : args)
Or
for (const auto &arg : args)
"for (std::string arg: args)" creates/destroys
one deep string copy on each iteration.
I hope it's obvious that I'm all for C++ conversion, but ...
> Even when it costs performance.
... not at any cost. startswith _is_ used in performance
critical paths.
BTW, I've been thinking that we may want to add our version
of C++17 std::string_view to avoid these kinds of problems.
Thanks,
Pedro Alves