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
- From: Pedro Alves <palves at redhat dot com>
- To: Philipp Rudo <prudo at linux dot vnet dot ibm dot com>, gdb-patches at sourceware dot org
- Cc: peter dot griffin at linaro dot org, yao dot qi at arm dot com, arnez at linux dot vnet dot ibm dot com
- Date: Thu, 12 Jan 2017 12:00:22 +0000
- Subject: Re: [RFC 2/7] Add libiberty/concat styled concat_path function
- Authentication-results: sourceware.org; auth=none
- References: <20170112113217.48852-1-prudo@linux.vnet.ibm.com> <20170112113217.48852-3-prudo@linux.vnet.ibm.com>
On 01/12/2017 11:32 AM, Philipp Rudo wrote:
> static inline int
> -startswith (const char *string, const char *pattern)
> +startswith (std::string str, std::string pattern)
NAK.
This is passing by copy, so will force unnecessary deep
string copies all over the place.
> {
> - return strncmp (string, pattern, strlen (pattern)) == 0;
> + return (str.find (pattern) == 0);
> +}
> +
I.e., before, this caused 0 copies:
startswith ("foo, "f");
After, you force a deep string copy for "foo", and another
for "f". It's as if you wrote:
startswith (xstrdup ("foo), xstrdup ("f"));
Also, this function is a static inline in a header so that
the compiler can see when "pattern" is a string literal, and
thus strlen can be optimized to a plain 'sizeof (pattern) - 1',
which is very frequent.
If you want to add overloads that can take "const std::string &"
for convenience, to avoid str.c_str(), that's maybe fine,
but you'd have to add all the combinations of
'const char *' x 'const std::string &' in the parameters, I
suppose.
Thanks,
Pedro Alves