* [PATCH] Use std::string in substitute_path_component
@ 2023-04-04 18:52 Tom Tromey
2023-04-05 1:17 ` Simon Marchi
0 siblings, 1 reply; 3+ messages in thread
From: Tom Tromey @ 2023-04-04 18:52 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This changes substitute_path_component to use std::string, simplifying
it quite a bit, and allowing for the removal of an xfree as well.
Regression tested on x86-64 Fedora 36.
---
gdb/auto-load.c | 16 +++++------
gdb/unittests/utils-selftests.c | 6 ++---
gdb/utils.c | 47 ++++++++++++---------------------
gdb/utils.h | 2 +-
4 files changed, 26 insertions(+), 45 deletions(-)
diff --git a/gdb/auto-load.c b/gdb/auto-load.c
index 40b05fdc634..6f1548c00c6 100644
--- a/gdb/auto-load.c
+++ b/gdb/auto-load.c
@@ -179,18 +179,14 @@ static std::vector<gdb::unique_xmalloc_ptr<char>> auto_load_safe_path_vec;
static std::vector<gdb::unique_xmalloc_ptr<char>>
auto_load_expand_dir_vars (const char *string)
{
- char *s = xstrdup (string);
- substitute_path_component (&s, "$datadir", gdb_datadir.c_str ());
- substitute_path_component (&s, "$debugdir", debug_file_directory.c_str ());
+ std::string s = string;
+ substitute_path_component (s, "$datadir", gdb_datadir.c_str ());
+ substitute_path_component (s, "$debugdir", debug_file_directory.c_str ());
- if (debug_auto_load && strcmp (s, string) != 0)
- auto_load_debug_printf ("Expanded $-variables to \"%s\".", s);
+ if (debug_auto_load && s != string)
+ auto_load_debug_printf ("Expanded $-variables to \"%s\".", s.c_str ());
- std::vector<gdb::unique_xmalloc_ptr<char>> dir_vec
- = dirnames_to_char_ptr_vec (s);
- xfree(s);
-
- return dir_vec;
+ return dirnames_to_char_ptr_vec (s.c_str ());
}
/* Update auto_load_safe_path_vec from current AUTO_LOAD_SAFE_PATH. */
diff --git a/gdb/unittests/utils-selftests.c b/gdb/unittests/utils-selftests.c
index 70609aa4294..42424b709fe 100644
--- a/gdb/unittests/utils-selftests.c
+++ b/gdb/unittests/utils-selftests.c
@@ -30,10 +30,8 @@ test_substitute_path_component ()
auto test = [] (std::string s, const char *from, const char *to,
const char *expected)
{
- char *temp = xstrdup (s.c_str ());
- substitute_path_component (&temp, from, to);
- SELF_CHECK (strcmp (temp, expected) == 0);
- xfree (temp);
+ substitute_path_component (s, from, to);
+ SELF_CHECK (s == expected);
};
test ("/abc/$def/g", "abc", "xyz", "/xyz/$def/g");
diff --git a/gdb/utils.c b/gdb/utils.c
index 6ec1cc0d48d..6cadf64a7d2 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -3263,49 +3263,36 @@ parse_pid_to_attach (const char *args)
return pid;
}
-/* Substitute all occurrences 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. */
+/* Substitute all occurrences of string FROM by string TO in STRINGP.
+ STRINGP may be updated in place. FROM needs to be delimited by
+ IS_DIR_SEPARATOR or DIRNAME_SEPARATOR (or be located at the start
+ or end of STRINGP). */
void
-substitute_path_component (char **stringp, const char *from, const char *to)
+substitute_path_component (std::string &stringp, const char *from,
+ const char *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 = 0;;)
{
- s = strstr (s, from);
- if (s == NULL)
+ pos = stringp.find (from, pos);
+ if (pos == std::string::npos)
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))
+ if ((pos == 0 || IS_DIR_SEPARATOR (stringp[pos - 1])
+ || stringp[pos - 1] == DIRNAME_SEPARATOR)
+ && (pos + from_len == stringp.length ()
+ || IS_DIR_SEPARATOR (stringp[pos + from_len])
+ || stringp[pos + from_len] == 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;
+ stringp.replace (pos, from_len, to);
+ pos += to_len;
}
else
- s++;
+ ++pos;
}
-
- *stringp = string;
}
#ifdef HAVE_WAITPID
diff --git a/gdb/utils.h b/gdb/utils.h
index a383036bcfe..bd1daac2e44 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -136,7 +136,7 @@ struct set_batch_flag_and_restore_page_info
extern int gdb_filename_fnmatch (const char *pattern, const char *string,
int flags);
-extern void substitute_path_component (char **stringp, const char *from,
+extern void substitute_path_component (std::string &stringp, const char *from,
const char *to);
std::string ldirname (const char *filename);
--
2.39.2
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Use std::string in substitute_path_component
2023-04-04 18:52 [PATCH] Use std::string in substitute_path_component Tom Tromey
@ 2023-04-05 1:17 ` Simon Marchi
2023-04-07 18:09 ` Tom Tromey
0 siblings, 1 reply; 3+ messages in thread
From: Simon Marchi @ 2023-04-05 1:17 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 4/4/23 14:52, Tom Tromey wrote:
> This changes substitute_path_component to use std::string, simplifying
> it quite a bit, and allowing for the removal of an xfree as well.
Your patch LGTM.
But a random thought I had while reading it: the sole user of
substitute_path_component is auto_load_expand_dir_vars, which splits the
path in components just after that. It would seem much simpler to do
the $datadir / $debugdir replacement after the split, by iterating on
the vector and checking each element. Anyway, not a big deal.
Simon
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Use std::string in substitute_path_component
2023-04-05 1:17 ` Simon Marchi
@ 2023-04-07 18:09 ` Tom Tromey
0 siblings, 0 replies; 3+ messages in thread
From: Tom Tromey @ 2023-04-07 18:09 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, gdb-patches
Simon> But a random thought I had while reading it: the sole user of
Simon> substitute_path_component is auto_load_expand_dir_vars, which splits the
Simon> path in components just after that. It would seem much simpler to do
Simon> the $datadir / $debugdir replacement after the split, by iterating on
Simon> the vector and checking each element. Anyway, not a big deal.
Good idea, I sent a v2 that does this.
Tom
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-04-07 18:09 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-04 18:52 [PATCH] Use std::string in substitute_path_component Tom Tromey
2023-04-05 1:17 ` Simon Marchi
2023-04-07 18:09 ` Tom Tromey
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).