[RFC,v5,2/9] Add libiberty/concat styled concat_path function

Message ID 20180312153115.47321-3-prudo@linux.vnet.ibm.com
State New
Headers show
Series
  • Add support for Linux kernel debugging
Related show

Commit Message

Philipp Rudo March 12, 2018, 3:31 p.m.
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(+)

-- 
2.13.5

Comments

Simon Marchi March 16, 2018, 2:47 a.m. | #1
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

Patch

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)
+	  && strncmp (string + strlen (string) - strlen (pattern), pattern,
+		      strlen (pattern)) == 0);
+}
+
 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
+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 ();
+
+  return length;
+}
+
+/* See utils.h.  */
+
+std::string
+_concat_path (const std::initializer_list<const std::string> args,
+	      const std::string &dir_separator)
+{
+  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.
+
+   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
+   */
+
+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);