[RFC,v3,1/8] Convert substitute_path_component to C++
Commit Message
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
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,
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,
>
@@ -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;
}
@@ -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
@@ -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);