From: Simon Marchi <simark@simark.ca>
To: Philipp Rudo <prudo@linux.vnet.ibm.com>, gdb-patches@sourceware.org
Cc: Omair Javaid <omair.javaid@linaro.org>,
Yao Qi <qiyaoltc@gmail.com>,
arnez@linux.vnet.ibm.com
Subject: Re: [RFC PATCH v5 2/9] Add libiberty/concat styled concat_path function
Date: Fri, 16 Mar 2018 02:47:00 -0000 [thread overview]
Message-ID: <edd1214c-2b95-b563-d371-2fe6918689f8@simark.ca> (raw)
In-Reply-To: <20180312153115.47321-3-prudo@linux.vnet.ibm.com>
On 2018-03-12 11:31 AM, Philipp Rudo wrote:
> This commit adds concat_path function to concatenate an arbitrary number of
> path elements. The function automatically adds an directory separator between
> two elements as needed.
>
> gdb/ChangeLog:
>
> * common/common-utils.h (endswith): New function.
> * utils.c (_concat_path, approx_path_length): New function.
> * utils.h (_concat_path): New export.
> (concat_path): New define.
> ---
> gdb/common/common-utils.h | 11 +++++++++++
> gdb/utils.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> gdb/utils.h | 16 ++++++++++++++++
> 3 files changed, 73 insertions(+)
>
> diff --git a/gdb/common/common-utils.h b/gdb/common/common-utils.h
> index 5408c35469..8e6eb05c2d 100644
> --- a/gdb/common/common-utils.h
> +++ b/gdb/common/common-utils.h
> @@ -109,6 +109,17 @@ startswith (const char *string, const char *pattern)
> return strncmp (string, pattern, strlen (pattern)) == 0;
> }
>
> +/* Return non-zero if the end of STRING matches PATTERN, zero
> + otherwise. */
> +
> +static inline int
> +endswith (const char *string, const char *pattern)
> +{
> + return (strlen (string) > strlen (pattern)
Should this be >=, so that endswith ("Hello", "Hello") returns true?
> + && strncmp (string + strlen (string) - strlen (pattern), pattern,
> + strlen (pattern)) == 0);
> +}
I believe you can use strcmp, which would avoid the last call to strlen.
> +
> ULONGEST strtoulst (const char *num, const char **trailer, int base);
>
> /* Skip leading whitespace characters in INP, returning an updated
> diff --git a/gdb/utils.c b/gdb/utils.c
> index d4f1398d14..4ce3909fca 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -3072,6 +3072,52 @@ substitute_path_component (std::string &str, const std::string &from,
> }
> }
>
> +/* Approximate length of final path. Helper for concat_path. */
> +
> +static inline unsigned long
Make the return type size_t too?
> +approx_path_length (const std::initializer_list<const std::string> args,
>
> + const std::string &dir_separator)
> +{
> + size_t length = 0;
> +
> + for (const std::string &arg: args)
> + length += arg.length () + dir_separator.length ();
One extra indentation level.
> +
> + return length;
> +}
> +
> +/* See utils.h. */
> +
> +std::string
> +_concat_path (const std::initializer_list<const std::string> args,
> + const std::string &dir_separator)
dit_separator should probably be a const char *, for the same reason
as mentioned in the previous patch.
> +{
> + std::string ret;
> + ret.reserve (approx_path_length (args, dir_separator));
> +
> + for (const std::string &arg : args)
> + {
> + if (arg.empty ())
> + continue;
> +
> + if (startswith (arg.c_str (), dir_separator.c_str ())
> + && endswith (ret.c_str (), dir_separator.c_str ()))
> + ret.erase (ret.length () - dir_separator.length (),
> + dir_separator.length ());
> +
> + else if (!ret.empty ()
> + && !startswith (arg.c_str (), dir_separator.c_str ())
> + && !endswith (ret.c_str (), dir_separator.c_str ())
> + && ret != TARGET_SYSROOT_PREFIX)
> + ret += dir_separator;
> +
> + ret += arg;
> + }
> +
> + ret.shrink_to_fit ();
> + return ret;
> +}
> +
> #ifdef HAVE_WAITPID
>
> #ifdef SIGALRM
> diff --git a/gdb/utils.h b/gdb/utils.h
> index 7e6a39ee82..aec9c6194d 100644
> --- a/gdb/utils.h
> +++ b/gdb/utils.h
> @@ -307,6 +307,22 @@ extern void substitute_path_component (std::string &str,
> const std::string &from,
> const std::string &to);
>
> +/* Concatenate an arbitrary number of path elements. Adds and removes
> + directory separators as needed.
Maybe mention the exception for TARGET_SYSROOT_PREFIX?
> +
> + concat_path (/first, second) => /first/second
> + concat_path (first, second) => first/second
> + concat_path (first/, second) => first/second
> + concat_path (first, /second) => first/second
> + concat_path (first/, /second) => first/second
> + concat_path (target:, second) => target:second
Should these examples include quotes, eg:
concat_path ("/first", "second") => /first/second
?
Also, those examples would be nice to convert to a selftest/unittest in the
newly created unittests/utils-selftests.c. :)
> + */
> +
> +extern std::string _concat_path (const std::initializer_list<const std::string> args,
> + const std::string &dir_separator);
> +
> +#define concat_path(...) _concat_path ({__VA_ARGS__}, SLASH_STRING)
> +
> std::string ldirname (const char *filename);
>
> extern int count_path_elements (const char *path);
>
Thanks,
Simon
next prev parent reply other threads:[~2018-03-16 2:47 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-12 15:31 [RFC v5 0/9] Add support for Linux kernel debugging Philipp Rudo
2018-03-12 15:31 ` [RFC PATCH v5 2/9] Add libiberty/concat styled concat_path function Philipp Rudo
2018-03-16 2:47 ` Simon Marchi [this message]
2018-03-12 15:31 ` [RFC PATCH v5 8/9] Link frame_info to thread_info Philipp Rudo
2018-03-12 15:31 ` [RFC PATCH v5 5/9] Add kernel module support for linux-kernel target Philipp Rudo
2018-03-12 15:31 ` [RFC PATCH v5 1/9] Convert substitute_path_component to C++ Philipp Rudo
2018-03-16 2:15 ` Simon Marchi
2018-03-17 20:11 ` Simon Marchi
2018-03-12 15:31 ` [RFC PATCH v5 9/9] Add S390 support for linux-kernel target Philipp Rudo
2018-03-12 15:31 ` [RFC PATCH v5 4/9] Add basic Linux kernel support Philipp Rudo
2018-03-13 14:09 ` Kamil Rytarowski
2018-03-14 9:48 ` Philipp Rudo
2018-03-14 23:38 ` Kamil Rytarowski
2018-03-19 0:11 ` Simon Marchi
2018-03-12 15:31 ` [RFC PATCH v5 7/9] Add privileged registers for s390x Philipp Rudo
2018-03-12 15:31 ` [RFC PATCH v5 3/9] Add scoped_restore_regcache_ptid Philipp Rudo
2018-03-17 18:08 ` Simon Marchi
2018-03-12 15:31 ` [RFC PATCH v5 6/9] Add commands for linux-kernel target Philipp Rudo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=edd1214c-2b95-b563-d371-2fe6918689f8@simark.ca \
--to=simark@simark.ca \
--cc=arnez@linux.vnet.ibm.com \
--cc=gdb-patches@sourceware.org \
--cc=omair.javaid@linaro.org \
--cc=prudo@linux.vnet.ibm.com \
--cc=qiyaoltc@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).