diff mbox

[RFC,v3,1/8] Convert substitute_path_component to C++

Message ID 20170316165739.88524-2-prudo@linux.vnet.ibm.com
State New, archived
Headers

Commit Message

Philipp Rudo March 16, 2017, 4:57 p.m. UTC 
  Simplify the code of utils.c:substiute_path_component by converting it to C++.

gdb/ChangeLog:

	* utils.c (substitute_path_component): Convert to C++.
	* utils.h (substitute_path_componetn): Adjust declatation.
	* auto-load.c (auto_load_expand_dir_vars): Adjust.
---
 gdb/auto-load.c | 18 +++++++++---------
 gdb/utils.c     | 50 ++++++++++++++------------------------------------
 gdb/utils.h     |  9 +++++++--
 3 files changed, 30 insertions(+), 47 deletions(-)

Comments

Sergio Durigan Junior April 20, 2017, 8:02 p.m. UTC | #1 
On Thursday, March 16 2017, Philipp Rudo wrote:

> Simplify the code of utils.c:substiute_path_component by converting it to C++.

Thanks for the patch, Philipp.  Just a minor nit.

> gdb/ChangeLog:
>
> 	* utils.c (substitute_path_component): Convert to C++.
> 	* utils.h (substitute_path_componetn): Adjust declatation.
> 	* auto-load.c (auto_load_expand_dir_vars): Adjust.
> ---
>  gdb/auto-load.c | 18 +++++++++---------
>  gdb/utils.c     | 50 ++++++++++++++------------------------------------
>  gdb/utils.h     |  9 +++++++--
>  3 files changed, 30 insertions(+), 47 deletions(-)
>
> diff --git a/gdb/auto-load.c b/gdb/auto-load.c
> index 56914c8..c84fee1 100644
> --- a/gdb/auto-load.c
> +++ b/gdb/auto-load.c
> @@ -40,6 +40,7 @@
>  #include "filestuff.h"
>  #include "extension.h"
>  #include "gdb/section-scripts.h"
> +#include <string>
>  
>  /* The section to look in for auto-loaded scripts (in file formats that
>     support sections).
> @@ -175,21 +176,20 @@ static VEC (char_ptr) *auto_load_safe_path_vec;
>     this vector must be freed by free_char_ptr_vec by the caller.  */
>  
>  static VEC (char_ptr) *
> -auto_load_expand_dir_vars (const char *string)
> +auto_load_expand_dir_vars (std::string orig)
>  {
>    VEC (char_ptr) *dir_vec;
> -  char *s;
> +  std::string str = orig;
>  
> -  s = xstrdup (string);
> -  substitute_path_component (&s, "$datadir", gdb_datadir);
> -  substitute_path_component (&s, "$debugdir", debug_file_directory);
> +  substitute_path_component (str, "$datadir", gdb_datadir);
> +  substitute_path_component (str, "$debugdir", debug_file_directory);
>  
> -  if (debug_auto_load && strcmp (s, string) != 0)
> +  if (debug_auto_load && str.compare (orig) != 0)
>      fprintf_unfiltered (gdb_stdlog,
> -			_("auto-load: Expanded $-variables to \"%s\".\n"), s);
> +			_("auto-load: Expanded $-variables to \"%s\".\n"),
> +			str.c_str ());
>  
> -  dir_vec = dirnames_to_char_ptr_vec (s);
> -  xfree(s);
> +  dir_vec = dirnames_to_char_ptr_vec (str.c_str ());
>  
>    return dir_vec;
>  }
> diff --git a/gdb/utils.c b/gdb/utils.c
> index 27021a1..bef619a 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -66,6 +66,8 @@
>  #include "interps.h"
>  #include "gdb_regex.h"
>  
> +#include <string>
> +
>  #if !HAVE_DECL_MALLOC
>  extern PTR malloc ();		/* ARI: PTR */
>  #endif
> @@ -3158,49 +3160,25 @@ make_cleanup_free_char_ptr_vec (VEC (char_ptr) *char_ptr_vec)
>    return make_cleanup (do_free_char_ptr_vec, char_ptr_vec);
>  }
>  
> -/* Substitute all occurences of string FROM by string TO in *STRINGP.  *STRINGP
> -   must come from xrealloc-compatible allocator and it may be updated.  FROM
> -   needs to be delimited by IS_DIR_SEPARATOR or DIRNAME_SEPARATOR (or be
> -   located at the start or end of *STRINGP.  */
> +/* See utils.h.  */
>  
>  void
> -substitute_path_component (char **stringp, const char *from, const char *to)
> +substitute_path_component (std::string &str, const std::string &from,
> +			   const std::string &to)
>  {
> -  char *string = *stringp, *s;
> -  const size_t from_len = strlen (from);
> -  const size_t to_len = strlen (to);
> -
> -  for (s = string;;)
> +  for (size_t pos = str.find (from); pos != std::string::npos;
> +       pos = str.find (from, pos + 1))
>      {
> -      s = strstr (s, from);
> -      if (s == NULL)
> -	break;
> -
> -      if ((s == string || IS_DIR_SEPARATOR (s[-1])
> -	   || s[-1] == DIRNAME_SEPARATOR)
> -          && (s[from_len] == '\0' || IS_DIR_SEPARATOR (s[from_len])
> -	      || s[from_len] == DIRNAME_SEPARATOR))
> +      char start, end;

Missing blank like between the declaration of the variables and the
assignment.

> +      start = str[pos - 1];
> +      end = str[pos + from.length ()];
> +      if ((pos == 0 || IS_DIR_SEPARATOR (start) || start == DIRNAME_SEPARATOR)
> +	  && (end == '\0' || IS_DIR_SEPARATOR (end)
> +	      || end == DIRNAME_SEPARATOR))
>  	{
> -	  char *string_new;
> -
> -	  string_new
> -	    = (char *) xrealloc (string, (strlen (string) + to_len + 1));
> -
> -	  /* Relocate the current S pointer.  */
> -	  s = s - string + string_new;
> -	  string = string_new;
> -
> -	  /* Replace from by to.  */
> -	  memmove (&s[to_len], &s[from_len], strlen (&s[from_len]) + 1);
> -	  memcpy (s, to, to_len);
> -
> -	  s += to_len;
> +	  str.replace (pos, from.length (), to);
>  	}
> -      else
> -	s++;
>      }
> -
> -  *stringp = string;
>  }
>  
>  #ifdef HAVE_WAITPID
> diff --git a/gdb/utils.h b/gdb/utils.h
> index f138702..d32114e 100644
> --- a/gdb/utils.h
> +++ b/gdb/utils.h
> @@ -132,8 +132,13 @@ extern char *gdb_abspath (const char *);
>  extern int gdb_filename_fnmatch (const char *pattern, const char *string,
>  				 int flags);
>  
> -extern void substitute_path_component (char **stringp, const char *from,
> -				       const char *to);
> +/* Substitute all occurences of string FROM by string TO in STR.  FROM
> +   needs to be delimited by IS_DIR_SEPARATOR or DIRNAME_SEPARATOR (or be
> +   located at the start or end of STR).  */
> +
> +extern void substitute_path_component (std::string &str,
> +				       const std::string &from,
> +				       const std::string &to);
>  
>  char *ldirname (const char *filename);
>  
> -- 
> 2.8.4

Other than that, LGTM.

Thanks,
Philipp Rudo May 3, 2017, 4:20 p.m. UTC | #2 
Hi Sergio,

sorry for the late answer but I was sick the last two weeks.

Thanks for the hint.  I fixed it locally.

Thanks
Philipp

On Thu, 20 Apr 2017 16:02:11 -0400
Sergio Durigan Junior <sergiodj@redhat.com> wrote:

> On Thursday, March 16 2017, Philipp Rudo wrote:
> 
> > Simplify the code of utils.c:substiute_path_component by converting it to
> > C++.  
> 
> Thanks for the patch, Philipp.  Just a minor nit.
> 
> > gdb/ChangeLog:
> >
> > 	* utils.c (substitute_path_component): Convert to C++.
> > 	* utils.h (substitute_path_componetn): Adjust declatation.
> > 	* auto-load.c (auto_load_expand_dir_vars): Adjust.
> > ---
> >  gdb/auto-load.c | 18 +++++++++---------
> >  gdb/utils.c     | 50 ++++++++++++++------------------------------------
> >  gdb/utils.h     |  9 +++++++--
> >  3 files changed, 30 insertions(+), 47 deletions(-)
> >
> > diff --git a/gdb/auto-load.c b/gdb/auto-load.c
> > index 56914c8..c84fee1 100644
> > --- a/gdb/auto-load.c
> > +++ b/gdb/auto-load.c
> > @@ -40,6 +40,7 @@
> >  #include "filestuff.h"
> >  #include "extension.h"
> >  #include "gdb/section-scripts.h"
> > +#include <string>
> >  
> >  /* The section to look in for auto-loaded scripts (in file formats that
> >     support sections).
> > @@ -175,21 +176,20 @@ static VEC (char_ptr) *auto_load_safe_path_vec;
> >     this vector must be freed by free_char_ptr_vec by the caller.  */
> >  
> >  static VEC (char_ptr) *
> > -auto_load_expand_dir_vars (const char *string)
> > +auto_load_expand_dir_vars (std::string orig)
> >  {
> >    VEC (char_ptr) *dir_vec;
> > -  char *s;
> > +  std::string str = orig;
> >  
> > -  s = xstrdup (string);
> > -  substitute_path_component (&s, "$datadir", gdb_datadir);
> > -  substitute_path_component (&s, "$debugdir", debug_file_directory);
> > +  substitute_path_component (str, "$datadir", gdb_datadir);
> > +  substitute_path_component (str, "$debugdir", debug_file_directory);
> >  
> > -  if (debug_auto_load && strcmp (s, string) != 0)
> > +  if (debug_auto_load && str.compare (orig) != 0)
> >      fprintf_unfiltered (gdb_stdlog,
> > -			_("auto-load: Expanded $-variables to \"%s\".\n"),
> > s);
> > +			_("auto-load: Expanded $-variables to \"%s\".\n"),
> > +			str.c_str ());
> >  
> > -  dir_vec = dirnames_to_char_ptr_vec (s);
> > -  xfree(s);
> > +  dir_vec = dirnames_to_char_ptr_vec (str.c_str ());
> >  
> >    return dir_vec;
> >  }
> > diff --git a/gdb/utils.c b/gdb/utils.c
> > index 27021a1..bef619a 100644
> > --- a/gdb/utils.c
> > +++ b/gdb/utils.c
> > @@ -66,6 +66,8 @@
> >  #include "interps.h"
> >  #include "gdb_regex.h"
> >  
> > +#include <string>
> > +
> >  #if !HAVE_DECL_MALLOC
> >  extern PTR malloc ();		/* ARI: PTR */
> >  #endif
> > @@ -3158,49 +3160,25 @@ make_cleanup_free_char_ptr_vec (VEC (char_ptr)
> > *char_ptr_vec) return make_cleanup (do_free_char_ptr_vec, char_ptr_vec);
> >  }
> >  
> > -/* Substitute all occurences of string FROM by string TO in *STRINGP.
> > *STRINGP
> > -   must come from xrealloc-compatible allocator and it may be updated.
> > FROM
> > -   needs to be delimited by IS_DIR_SEPARATOR or DIRNAME_SEPARATOR (or be
> > -   located at the start or end of *STRINGP.  */
> > +/* See utils.h.  */
> >  
> >  void
> > -substitute_path_component (char **stringp, const char *from, const char
> > *to) +substitute_path_component (std::string &str, const std::string &from,
> > +			   const std::string &to)
> >  {
> > -  char *string = *stringp, *s;
> > -  const size_t from_len = strlen (from);
> > -  const size_t to_len = strlen (to);
> > -
> > -  for (s = string;;)
> > +  for (size_t pos = str.find (from); pos != std::string::npos;
> > +       pos = str.find (from, pos + 1))
> >      {
> > -      s = strstr (s, from);
> > -      if (s == NULL)
> > -	break;
> > -
> > -      if ((s == string || IS_DIR_SEPARATOR (s[-1])
> > -	   || s[-1] == DIRNAME_SEPARATOR)
> > -          && (s[from_len] == '\0' || IS_DIR_SEPARATOR (s[from_len])
> > -	      || s[from_len] == DIRNAME_SEPARATOR))
> > +      char start, end;  
> 
> Missing blank like between the declaration of the variables and the
> assignment.
> 
> > +      start = str[pos - 1];
> > +      end = str[pos + from.length ()];
> > +      if ((pos == 0 || IS_DIR_SEPARATOR (start) || start ==
> > DIRNAME_SEPARATOR)
> > +	  && (end == '\0' || IS_DIR_SEPARATOR (end)
> > +	      || end == DIRNAME_SEPARATOR))
> >  	{
> > -	  char *string_new;
> > -
> > -	  string_new
> > -	    = (char *) xrealloc (string, (strlen (string) + to_len + 1));
> > -
> > -	  /* Relocate the current S pointer.  */
> > -	  s = s - string + string_new;
> > -	  string = string_new;
> > -
> > -	  /* Replace from by to.  */
> > -	  memmove (&s[to_len], &s[from_len], strlen (&s[from_len]) + 1);
> > -	  memcpy (s, to, to_len);
> > -
> > -	  s += to_len;
> > +	  str.replace (pos, from.length (), to);
> >  	}
> > -      else
> > -	s++;
> >      }
> > -
> > -  *stringp = string;
> >  }
> >  
> >  #ifdef HAVE_WAITPID
> > diff --git a/gdb/utils.h b/gdb/utils.h
> > index f138702..d32114e 100644
> > --- a/gdb/utils.h
> > +++ b/gdb/utils.h
> > @@ -132,8 +132,13 @@ extern char *gdb_abspath (const char *);
> >  extern int gdb_filename_fnmatch (const char *pattern, const char *string,
> >  				 int flags);
> >  
> > -extern void substitute_path_component (char **stringp, const char *from,
> > -				       const char *to);
> > +/* Substitute all occurences of string FROM by string TO in STR.  FROM
> > +   needs to be delimited by IS_DIR_SEPARATOR or DIRNAME_SEPARATOR (or be
> > +   located at the start or end of STR).  */
> > +
> > +extern void substitute_path_component (std::string &str,
> > +				       const std::string &from,
> > +				       const std::string &to);
> >  
> >  char *ldirname (const char *filename);
> >  
> > -- 
> > 2.8.4  
> 
> Other than that, LGTM.
> 
> Thanks,
>
diff mbox

Patch

diff --git a/gdb/auto-load.c b/gdb/auto-load.c
index 56914c8..c84fee1 100644
--- a/gdb/auto-load.c
+++ b/gdb/auto-load.c
@@ -40,6 +40,7 @@ 
 #include "filestuff.h"
 #include "extension.h"
 #include "gdb/section-scripts.h"
+#include <string>
 
 /* The section to look in for auto-loaded scripts (in file formats that
    support sections).
@@ -175,21 +176,20 @@  static VEC (char_ptr) *auto_load_safe_path_vec;
    this vector must be freed by free_char_ptr_vec by the caller.  */
 
 static VEC (char_ptr) *
-auto_load_expand_dir_vars (const char *string)
+auto_load_expand_dir_vars (std::string orig)
 {
   VEC (char_ptr) *dir_vec;
-  char *s;
+  std::string str = orig;
 
-  s = xstrdup (string);
-  substitute_path_component (&s, "$datadir", gdb_datadir);
-  substitute_path_component (&s, "$debugdir", debug_file_directory);
+  substitute_path_component (str, "$datadir", gdb_datadir);
+  substitute_path_component (str, "$debugdir", debug_file_directory);
 
-  if (debug_auto_load && strcmp (s, string) != 0)
+  if (debug_auto_load && str.compare (orig) != 0)
     fprintf_unfiltered (gdb_stdlog,
-			_("auto-load: Expanded $-variables to \"%s\".\n"), s);
+			_("auto-load: Expanded $-variables to \"%s\".\n"),
+			str.c_str ());
 
-  dir_vec = dirnames_to_char_ptr_vec (s);
-  xfree(s);
+  dir_vec = dirnames_to_char_ptr_vec (str.c_str ());
 
   return dir_vec;
 }
diff --git a/gdb/utils.c b/gdb/utils.c
index 27021a1..bef619a 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -66,6 +66,8 @@ 
 #include "interps.h"
 #include "gdb_regex.h"
 
+#include <string>
+
 #if !HAVE_DECL_MALLOC
 extern PTR malloc ();		/* ARI: PTR */
 #endif
@@ -3158,49 +3160,25 @@  make_cleanup_free_char_ptr_vec (VEC (char_ptr) *char_ptr_vec)
   return make_cleanup (do_free_char_ptr_vec, char_ptr_vec);
 }
 
-/* Substitute all occurences of string FROM by string TO in *STRINGP.  *STRINGP
-   must come from xrealloc-compatible allocator and it may be updated.  FROM
-   needs to be delimited by IS_DIR_SEPARATOR or DIRNAME_SEPARATOR (or be
-   located at the start or end of *STRINGP.  */
+/* See utils.h.  */
 
 void
-substitute_path_component (char **stringp, const char *from, const char *to)
+substitute_path_component (std::string &str, const std::string &from,
+			   const std::string &to)
 {
-  char *string = *stringp, *s;
-  const size_t from_len = strlen (from);
-  const size_t to_len = strlen (to);
-
-  for (s = string;;)
+  for (size_t pos = str.find (from); pos != std::string::npos;
+       pos = str.find (from, pos + 1))
     {
-      s = strstr (s, from);
-      if (s == NULL)
-	break;
-
-      if ((s == string || IS_DIR_SEPARATOR (s[-1])
-	   || s[-1] == DIRNAME_SEPARATOR)
-          && (s[from_len] == '\0' || IS_DIR_SEPARATOR (s[from_len])
-	      || s[from_len] == DIRNAME_SEPARATOR))
+      char start, end;
+      start = str[pos - 1];
+      end = str[pos + from.length ()];
+      if ((pos == 0 || IS_DIR_SEPARATOR (start) || start == DIRNAME_SEPARATOR)
+	  && (end == '\0' || IS_DIR_SEPARATOR (end)
+	      || end == DIRNAME_SEPARATOR))
 	{
-	  char *string_new;
-
-	  string_new
-	    = (char *) xrealloc (string, (strlen (string) + to_len + 1));
-
-	  /* Relocate the current S pointer.  */
-	  s = s - string + string_new;
-	  string = string_new;
-
-	  /* Replace from by to.  */
-	  memmove (&s[to_len], &s[from_len], strlen (&s[from_len]) + 1);
-	  memcpy (s, to, to_len);
-
-	  s += to_len;
+	  str.replace (pos, from.length (), to);
 	}
-      else
-	s++;
     }
-
-  *stringp = string;
 }
 
 #ifdef HAVE_WAITPID
diff --git a/gdb/utils.h b/gdb/utils.h
index f138702..d32114e 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -132,8 +132,13 @@  extern char *gdb_abspath (const char *);
 extern int gdb_filename_fnmatch (const char *pattern, const char *string,
 				 int flags);
 
-extern void substitute_path_component (char **stringp, const char *from,
-				       const char *to);
+/* Substitute all occurences of string FROM by string TO in STR.  FROM
+   needs to be delimited by IS_DIR_SEPARATOR or DIRNAME_SEPARATOR (or be
+   located at the start or end of STR).  */
+
+extern void substitute_path_component (std::string &str,
+				       const std::string &from,
+				       const std::string &to);
 
 char *ldirname (const char *filename);