* [PATCH 1/4] gdb: remove target description macros
@ 2021-05-06 16:49 Simon Marchi
2021-05-06 16:49 ` [PATCH 2/4] gdb: change target_desc_info::fetched to bool Simon Marchi
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Simon Marchi @ 2021-05-06 16:49 UTC (permalink / raw)
To: gdb-patches
In my opinion, the target_desc_fetched, current_target_desc and
target_description_filename macros in target-descriptions.c are not very
useful. I don't think it's useful to hide that they operate on the
current inferior, as everything currently works under the assumption
that the various tdesc commands operate on the current inferior, and I
don't see that changing in the foreseeable future.
This change also avoids having multiple unnecessary calls to
current_inferior and get_tdesc_info per function.
gdb/ChangeLog:
* target-descriptions.c (struct target_desc_info) <tdesc>:
Adjust doc.
(target_desc_fetched): Remove.
(current_target_desc): Remove.
(target_description_filename): Remove.
(target_find_description): Adjust.
(target_clear_description): Adjust.
(target_current_description): Adjust.
(set_tdesc_filename_cmd): Adjust.
(show_tdesc_filename_cmd): Adjust.
(unset_tdesc_filename_cmd): Adjust.
(maint_print_c_tdesc_cmd): Adjust.
(maint_print_xml_tdesc_cmd): Adjust.
Change-Id: Ibfb581490e949c16d59924e2cac633ede5c26c5b
---
gdb/target-descriptions.c | 75 +++++++++++++++++++--------------------
1 file changed, 37 insertions(+), 38 deletions(-)
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index c0e798a3530a..fd9e5a9d115d 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -450,7 +450,7 @@ struct target_desc_info
/* The description fetched from the target, or NULL if the target
did not supply any description. Only valid when
- target_desc_fetched is set. Only the description initialization
+ FETCHED is set. Only the description initialization
code should access this; normally, the description should be
accessed through the gdbarch object. */
@@ -511,15 +511,6 @@ target_desc_info_free (struct target_desc_info *tdesc_info)
}
}
-/* Convenience helper macros. */
-
-#define target_desc_fetched \
- get_tdesc_info (current_inferior ())->fetched
-#define current_target_desc \
- get_tdesc_info (current_inferior ())->tdesc
-#define target_description_filename \
- get_tdesc_info (current_inferior ())->filename
-
/* The string manipulated by the "set tdesc filename ..." command. */
static char *tdesc_filename_cmd_string;
@@ -530,11 +521,13 @@ static char *tdesc_filename_cmd_string;
void
target_find_description (void)
{
+ target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
+
/* If we've already fetched a description from the target, don't do
it again. This allows a target to fetch the description early,
during its to_open or to_create_inferior, if it needs extra
information about the target to initialize. */
- if (target_desc_fetched)
+ if (tdesc_info->fetched)
return;
/* The current architecture should not have any target description
@@ -544,31 +537,29 @@ target_find_description (void)
/* First try to fetch an XML description from the user-specified
file. */
- current_target_desc = NULL;
- if (target_description_filename != NULL
- && *target_description_filename != '\0')
- current_target_desc
- = file_read_description_xml (target_description_filename);
+ tdesc_info->tdesc = nullptr;
+ if (tdesc_info->filename != nullptr && *tdesc_info->filename != '\0')
+ tdesc_info->tdesc= file_read_description_xml (tdesc_info->filename);
/* Next try to read the description from the current target using
target objects. */
- if (current_target_desc == NULL)
- current_target_desc = target_read_description_xml
+ if (tdesc_info->tdesc == nullptr)
+ tdesc_info->tdesc = target_read_description_xml
(current_inferior ()->top_target ());
/* If that failed try a target-specific hook. */
- if (current_target_desc == NULL)
- current_target_desc = target_read_description
+ if (tdesc_info->tdesc == nullptr)
+ tdesc_info->tdesc = target_read_description
(current_inferior ()->top_target ());
/* If a non-NULL description was returned, then update the current
architecture. */
- if (current_target_desc)
+ if (tdesc_info->tdesc != nullptr)
{
struct gdbarch_info info;
gdbarch_info_init (&info);
- info.target_desc = current_target_desc;
+ info.target_desc = tdesc_info->tdesc;
if (!gdbarch_update_p (info))
warning (_("Architecture rejected target-supplied description"));
else
@@ -577,7 +568,7 @@ target_find_description (void)
data = ((struct tdesc_arch_data *)
gdbarch_data (target_gdbarch (), tdesc_data));
- if (tdesc_has_registers (current_target_desc)
+ if (tdesc_has_registers (tdesc_info->tdesc)
&& data->arch_regs.empty ())
warning (_("Target-supplied registers are not supported "
"by the current architecture"));
@@ -586,7 +577,7 @@ target_find_description (void)
/* Now that we know this description is usable, record that we
fetched it. */
- target_desc_fetched = 1;
+ tdesc_info->fetched = 1;
}
/* Discard any description fetched from the current target, and switch
@@ -595,14 +586,15 @@ target_find_description (void)
void
target_clear_description (void)
{
- struct gdbarch_info info;
+ target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
- if (!target_desc_fetched)
+ if (!tdesc_info->fetched)
return;
- target_desc_fetched = 0;
- current_target_desc = NULL;
+ tdesc_info->fetched = 0;
+ tdesc_info->tdesc = nullptr;
+ gdbarch_info info;
gdbarch_info_init (&info);
if (!gdbarch_update_p (info))
internal_error (__FILE__, __LINE__,
@@ -616,8 +608,10 @@ target_clear_description (void)
const struct target_desc *
target_current_description (void)
{
- if (target_desc_fetched)
- return current_target_desc;
+ target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
+
+ if (tdesc_info->fetched)
+ return tdesc_info->tdesc;
return NULL;
}
@@ -1298,8 +1292,10 @@ static void
set_tdesc_filename_cmd (const char *args, int from_tty,
struct cmd_list_element *c)
{
- xfree (target_description_filename);
- target_description_filename = xstrdup (tdesc_filename_cmd_string);
+ target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
+
+ xfree (tdesc_info->filename);
+ tdesc_info->filename = xstrdup (tdesc_filename_cmd_string);
target_clear_description ();
target_find_description ();
@@ -1310,7 +1306,7 @@ show_tdesc_filename_cmd (struct ui_file *file, int from_tty,
struct cmd_list_element *c,
const char *value)
{
- value = target_description_filename;
+ value = get_tdesc_info (current_inferior ())->filename;
if (value != NULL && *value != '\0')
printf_filtered (_("The target description will be read from \"%s\".\n"),
@@ -1323,8 +1319,10 @@ show_tdesc_filename_cmd (struct ui_file *file, int from_tty,
static void
unset_tdesc_filename_cmd (const char *args, int from_tty)
{
- xfree (target_description_filename);
- target_description_filename = NULL;
+ target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
+
+ xfree (tdesc_info->filename);
+ tdesc_info->filename = nullptr;
target_clear_description ();
target_find_description ();
}
@@ -1778,8 +1776,9 @@ maint_print_c_tdesc_cmd (const char *args, int from_tty)
architecture's. This lets a GDB for one architecture generate C
for another architecture's description, even though the gdbarch
initialization code will reject the new description. */
- tdesc = current_target_desc;
- filename = target_description_filename;
+ target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
+ tdesc = tdesc_info->tdesc;
+ filename = tdesc_info->filename;
}
else
{
@@ -1850,7 +1849,7 @@ maint_print_xml_tdesc_cmd (const char *args, int from_tty)
architecture's. This lets a GDB for one architecture generate XML
for another architecture's description, even though the gdbarch
initialization code will reject the new description. */
- tdesc = current_target_desc;
+ tdesc = get_tdesc_info (current_inferior ())->tdesc;
}
else
{
--
2.30.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/4] gdb: change target_desc_info::fetched to bool
2021-05-06 16:49 [PATCH 1/4] gdb: remove target description macros Simon Marchi
@ 2021-05-06 16:49 ` Simon Marchi
2021-05-07 10:18 ` Andrew Burgess
2021-05-06 16:49 ` [PATCH 3/4] gdb: (de-)allocate target_desc_info with new/delete Simon Marchi
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Simon Marchi @ 2021-05-06 16:49 UTC (permalink / raw)
To: gdb-patches
gdb/ChangeLog:
* target-descriptions.c (struct target_desc_info) <fetched>:
bool.
(target_find_description): Adjust.
(target_clear_description): Adjust.
Change-Id: Ib69e097b38cf270e674f1249105d535a312954e1
---
gdb/target-descriptions.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index fd9e5a9d115d..aee1478301b6 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -446,7 +446,7 @@ struct target_desc_info
/* A flag indicating that a description has already been fetched
from the target, so it should not be queried again. */
- int fetched;
+ bool fetched;
/* The description fetched from the target, or NULL if the target
did not supply any description. Only valid when
@@ -577,7 +577,7 @@ target_find_description (void)
/* Now that we know this description is usable, record that we
fetched it. */
- tdesc_info->fetched = 1;
+ tdesc_info->fetched = true;
}
/* Discard any description fetched from the current target, and switch
@@ -591,7 +591,7 @@ target_clear_description (void)
if (!tdesc_info->fetched)
return;
- tdesc_info->fetched = 0;
+ tdesc_info->fetched = false;
tdesc_info->tdesc = nullptr;
gdbarch_info info;
--
2.30.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/4] gdb: (de-)allocate target_desc_info with new/delete
2021-05-06 16:49 [PATCH 1/4] gdb: remove target description macros Simon Marchi
2021-05-06 16:49 ` [PATCH 2/4] gdb: change target_desc_info::fetched to bool Simon Marchi
@ 2021-05-06 16:49 ` Simon Marchi
2021-05-07 10:20 ` Andrew Burgess
2021-05-06 16:49 ` [PATCH 4/4] gdb: make target_desc_info::filename an std::string Simon Marchi
2021-05-07 10:17 ` [PATCH 1/4] gdb: remove target description macros Andrew Burgess
3 siblings, 1 reply; 10+ messages in thread
From: Simon Marchi @ 2021-05-06 16:49 UTC (permalink / raw)
To: gdb-patches
In preparation for using non-POD types in the struct.
gdb/ChangeLog:
* target-descriptions.c (struct target_desc_info): Initialize
fields.
(get_tdesc_info): Use new.
(target_desc_info_free): Use delete.
Change-Id: I10fdaeeae7cdbd7930ae7adeeb13f7f363c67c7a
---
gdb/target-descriptions.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index aee1478301b6..341020fed762 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -446,7 +446,7 @@ struct target_desc_info
/* A flag indicating that a description has already been fetched
from the target, so it should not be queried again. */
- bool fetched;
+ bool fetched = false;
/* The description fetched from the target, or NULL if the target
did not supply any description. Only valid when
@@ -454,12 +454,12 @@ struct target_desc_info
code should access this; normally, the description should be
accessed through the gdbarch object. */
- const struct target_desc *tdesc;
+ const struct target_desc *tdesc = nullptr;
/* The filename to read a target description from, as set by "set
tdesc filename ..." */
- char *filename;
+ char *filename = nullptr;
};
/* Get the inferior INF's target description info, allocating one on
@@ -469,7 +469,8 @@ static struct target_desc_info *
get_tdesc_info (struct inferior *inf)
{
if (inf->tdesc_info == NULL)
- inf->tdesc_info = XCNEW (struct target_desc_info);
+ inf->tdesc_info = new target_desc_info;
+
return inf->tdesc_info;
}
@@ -507,7 +508,7 @@ target_desc_info_free (struct target_desc_info *tdesc_info)
if (tdesc_info != NULL)
{
xfree (tdesc_info->filename);
- xfree (tdesc_info);
+ delete tdesc_info;
}
}
--
2.30.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/4] gdb: make target_desc_info::filename an std::string
2021-05-06 16:49 [PATCH 1/4] gdb: remove target description macros Simon Marchi
2021-05-06 16:49 ` [PATCH 2/4] gdb: change target_desc_info::fetched to bool Simon Marchi
2021-05-06 16:49 ` [PATCH 3/4] gdb: (de-)allocate target_desc_info with new/delete Simon Marchi
@ 2021-05-06 16:49 ` Simon Marchi
2021-05-07 10:22 ` Andrew Burgess
2021-05-07 10:17 ` [PATCH 1/4] gdb: remove target description macros Andrew Burgess
3 siblings, 1 reply; 10+ messages in thread
From: Simon Marchi @ 2021-05-06 16:49 UTC (permalink / raw)
To: gdb-patches
To make the management of memory automatic.
As to why I chose to make this an std::string and not an
std::unique_xmalloc_ptr<char>: some parts of the code consider both a
NULL value and an empty string value to mean "no filename".
target_desc_info_from_user_p, however, doesn't check for a non-NULL but
empty string value. So it seems like having two ways of denoting "no
filename" can lead to these kinds of inconsistencies. Using
std::string, "no filename" is only represented by an empty value.
As a bonus, using an std::string lets us copy target_desc_info objects
using the default assignment operator.
gdb/ChangeLog:
* target-descriptions.c (struct target_desc_info) <filename>:
Make std::string.
(copy_inferior_target_desc_info): Adjust.
(target_desc_info_free): Adjust.
(target_find_description): Adjust.
(set_tdesc_filename_cmd): Adjust.
(show_tdesc_filename_cmd): Adjust.
(unset_tdesc_filename_cmd): Adjust.
(maint_print_c_tdesc_cmd): Adjust.
Change-Id: I4e3a6ad8ccda2b88c202471d4f54249753cad127
---
gdb/target-descriptions.c | 34 ++++++++++++++--------------------
1 file changed, 14 insertions(+), 20 deletions(-)
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 341020fed762..053864dec58b 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -456,10 +456,12 @@ struct target_desc_info
const struct target_desc *tdesc = nullptr;
- /* The filename to read a target description from, as set by "set
- tdesc filename ..." */
+ /* If not empty, the filename to read a target description from, as set by
+ "set tdesc filename ...".
- char *filename = nullptr;
+ If empty, there is not filename specified by the user. */
+
+ std::string filename;
};
/* Get the inferior INF's target description info, allocating one on
@@ -484,7 +486,7 @@ static struct gdbarch_data *tdesc_data;
int
target_desc_info_from_user_p (struct target_desc_info *info)
{
- return info != NULL && info->filename != NULL;
+ return info != nullptr && !info->filename.empty ();
}
/* See target-descriptions.h. */
@@ -495,9 +497,7 @@ copy_inferior_target_desc_info (struct inferior *destinf, struct inferior *srcin
struct target_desc_info *src = get_tdesc_info (srcinf);
struct target_desc_info *dest = get_tdesc_info (destinf);
- dest->fetched = src->fetched;
- dest->tdesc = src->tdesc;
- dest->filename = src->filename != NULL ? xstrdup (src->filename) : NULL;
+ *dest = *src;
}
/* See target-descriptions.h. */
@@ -505,11 +505,7 @@ copy_inferior_target_desc_info (struct inferior *destinf, struct inferior *srcin
void
target_desc_info_free (struct target_desc_info *tdesc_info)
{
- if (tdesc_info != NULL)
- {
- xfree (tdesc_info->filename);
- delete tdesc_info;
- }
+ delete tdesc_info;
}
/* The string manipulated by the "set tdesc filename ..." command. */
@@ -539,8 +535,8 @@ target_find_description (void)
/* First try to fetch an XML description from the user-specified
file. */
tdesc_info->tdesc = nullptr;
- if (tdesc_info->filename != nullptr && *tdesc_info->filename != '\0')
- tdesc_info->tdesc= file_read_description_xml (tdesc_info->filename);
+ if (!tdesc_info->filename.empty ())
+ tdesc_info->tdesc = file_read_description_xml (tdesc_info->filename.data ());
/* Next try to read the description from the current target using
target objects. */
@@ -1295,8 +1291,7 @@ set_tdesc_filename_cmd (const char *args, int from_tty,
{
target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
- xfree (tdesc_info->filename);
- tdesc_info->filename = xstrdup (tdesc_filename_cmd_string);
+ tdesc_info->filename = tdesc_filename_cmd_string;
target_clear_description ();
target_find_description ();
@@ -1307,7 +1302,7 @@ show_tdesc_filename_cmd (struct ui_file *file, int from_tty,
struct cmd_list_element *c,
const char *value)
{
- value = get_tdesc_info (current_inferior ())->filename;
+ value = get_tdesc_info (current_inferior ())->filename.data ();
if (value != NULL && *value != '\0')
printf_filtered (_("The target description will be read from \"%s\".\n"),
@@ -1322,8 +1317,7 @@ unset_tdesc_filename_cmd (const char *args, int from_tty)
{
target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
- xfree (tdesc_info->filename);
- tdesc_info->filename = nullptr;
+ tdesc_info->filename.clear ();
target_clear_description ();
target_find_description ();
}
@@ -1779,7 +1773,7 @@ maint_print_c_tdesc_cmd (const char *args, int from_tty)
initialization code will reject the new description. */
target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
tdesc = tdesc_info->tdesc;
- filename = tdesc_info->filename;
+ filename = tdesc_info->filename.data ();
}
else
{
--
2.30.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] gdb: remove target description macros
2021-05-06 16:49 [PATCH 1/4] gdb: remove target description macros Simon Marchi
` (2 preceding siblings ...)
2021-05-06 16:49 ` [PATCH 4/4] gdb: make target_desc_info::filename an std::string Simon Marchi
@ 2021-05-07 10:17 ` Andrew Burgess
2021-05-07 19:42 ` Simon Marchi
3 siblings, 1 reply; 10+ messages in thread
From: Andrew Burgess @ 2021-05-07 10:17 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
* Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> [2021-05-06 12:49:54 -0400]:
> In my opinion, the target_desc_fetched, current_target_desc and
> target_description_filename macros in target-descriptions.c are not very
> useful. I don't think it's useful to hide that they operate on the
> current inferior, as everything currently works under the assumption
> that the various tdesc commands operate on the current inferior, and I
> don't see that changing in the foreseeable future.
>
> This change also avoids having multiple unnecessary calls to
> current_inferior and get_tdesc_info per function.
Awesome! One minor nit inline below.
>
> gdb/ChangeLog:
>
> * target-descriptions.c (struct target_desc_info) <tdesc>:
> Adjust doc.
> (target_desc_fetched): Remove.
> (current_target_desc): Remove.
> (target_description_filename): Remove.
> (target_find_description): Adjust.
> (target_clear_description): Adjust.
> (target_current_description): Adjust.
> (set_tdesc_filename_cmd): Adjust.
> (show_tdesc_filename_cmd): Adjust.
> (unset_tdesc_filename_cmd): Adjust.
> (maint_print_c_tdesc_cmd): Adjust.
> (maint_print_xml_tdesc_cmd): Adjust.
>
> Change-Id: Ibfb581490e949c16d59924e2cac633ede5c26c5b
> ---
> gdb/target-descriptions.c | 75 +++++++++++++++++++--------------------
> 1 file changed, 37 insertions(+), 38 deletions(-)
>
> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
> index c0e798a3530a..fd9e5a9d115d 100644
> --- a/gdb/target-descriptions.c
> +++ b/gdb/target-descriptions.c
> @@ -450,7 +450,7 @@ struct target_desc_info
>
> /* The description fetched from the target, or NULL if the target
> did not supply any description. Only valid when
> - target_desc_fetched is set. Only the description initialization
> + FETCHED is set. Only the description initialization
> code should access this; normally, the description should be
> accessed through the gdbarch object. */
>
> @@ -511,15 +511,6 @@ target_desc_info_free (struct target_desc_info *tdesc_info)
> }
> }
>
> -/* Convenience helper macros. */
> -
> -#define target_desc_fetched \
> - get_tdesc_info (current_inferior ())->fetched
> -#define current_target_desc \
> - get_tdesc_info (current_inferior ())->tdesc
> -#define target_description_filename \
> - get_tdesc_info (current_inferior ())->filename
> -
> /* The string manipulated by the "set tdesc filename ..." command. */
>
> static char *tdesc_filename_cmd_string;
> @@ -530,11 +521,13 @@ static char *tdesc_filename_cmd_string;
> void
> target_find_description (void)
> {
> + target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
> +
> /* If we've already fetched a description from the target, don't do
> it again. This allows a target to fetch the description early,
> during its to_open or to_create_inferior, if it needs extra
> information about the target to initialize. */
> - if (target_desc_fetched)
> + if (tdesc_info->fetched)
> return;
>
> /* The current architecture should not have any target description
> @@ -544,31 +537,29 @@ target_find_description (void)
>
> /* First try to fetch an XML description from the user-specified
> file. */
> - current_target_desc = NULL;
> - if (target_description_filename != NULL
> - && *target_description_filename != '\0')
> - current_target_desc
> - = file_read_description_xml (target_description_filename);
> + tdesc_info->tdesc = nullptr;
> + if (tdesc_info->filename != nullptr && *tdesc_info->filename != '\0')
> + tdesc_info->tdesc= file_read_description_xml (tdesc_info->filename);
Missed a space before '=' here.
Otherwise, looks great.
Thanks,
Andrew
>
> /* Next try to read the description from the current target using
> target objects. */
> - if (current_target_desc == NULL)
> - current_target_desc = target_read_description_xml
> + if (tdesc_info->tdesc == nullptr)
> + tdesc_info->tdesc = target_read_description_xml
> (current_inferior ()->top_target ());
>
> /* If that failed try a target-specific hook. */
> - if (current_target_desc == NULL)
> - current_target_desc = target_read_description
> + if (tdesc_info->tdesc == nullptr)
> + tdesc_info->tdesc = target_read_description
> (current_inferior ()->top_target ());
>
> /* If a non-NULL description was returned, then update the current
> architecture. */
> - if (current_target_desc)
> + if (tdesc_info->tdesc != nullptr)
> {
> struct gdbarch_info info;
>
> gdbarch_info_init (&info);
> - info.target_desc = current_target_desc;
> + info.target_desc = tdesc_info->tdesc;
> if (!gdbarch_update_p (info))
> warning (_("Architecture rejected target-supplied description"));
> else
> @@ -577,7 +568,7 @@ target_find_description (void)
>
> data = ((struct tdesc_arch_data *)
> gdbarch_data (target_gdbarch (), tdesc_data));
> - if (tdesc_has_registers (current_target_desc)
> + if (tdesc_has_registers (tdesc_info->tdesc)
> && data->arch_regs.empty ())
> warning (_("Target-supplied registers are not supported "
> "by the current architecture"));
> @@ -586,7 +577,7 @@ target_find_description (void)
>
> /* Now that we know this description is usable, record that we
> fetched it. */
> - target_desc_fetched = 1;
> + tdesc_info->fetched = 1;
> }
>
> /* Discard any description fetched from the current target, and switch
> @@ -595,14 +586,15 @@ target_find_description (void)
> void
> target_clear_description (void)
> {
> - struct gdbarch_info info;
> + target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
>
> - if (!target_desc_fetched)
> + if (!tdesc_info->fetched)
> return;
>
> - target_desc_fetched = 0;
> - current_target_desc = NULL;
> + tdesc_info->fetched = 0;
> + tdesc_info->tdesc = nullptr;
>
> + gdbarch_info info;
> gdbarch_info_init (&info);
> if (!gdbarch_update_p (info))
> internal_error (__FILE__, __LINE__,
> @@ -616,8 +608,10 @@ target_clear_description (void)
> const struct target_desc *
> target_current_description (void)
> {
> - if (target_desc_fetched)
> - return current_target_desc;
> + target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
> +
> + if (tdesc_info->fetched)
> + return tdesc_info->tdesc;
>
> return NULL;
> }
> @@ -1298,8 +1292,10 @@ static void
> set_tdesc_filename_cmd (const char *args, int from_tty,
> struct cmd_list_element *c)
> {
> - xfree (target_description_filename);
> - target_description_filename = xstrdup (tdesc_filename_cmd_string);
> + target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
> +
> + xfree (tdesc_info->filename);
> + tdesc_info->filename = xstrdup (tdesc_filename_cmd_string);
>
> target_clear_description ();
> target_find_description ();
> @@ -1310,7 +1306,7 @@ show_tdesc_filename_cmd (struct ui_file *file, int from_tty,
> struct cmd_list_element *c,
> const char *value)
> {
> - value = target_description_filename;
> + value = get_tdesc_info (current_inferior ())->filename;
>
> if (value != NULL && *value != '\0')
> printf_filtered (_("The target description will be read from \"%s\".\n"),
> @@ -1323,8 +1319,10 @@ show_tdesc_filename_cmd (struct ui_file *file, int from_tty,
> static void
> unset_tdesc_filename_cmd (const char *args, int from_tty)
> {
> - xfree (target_description_filename);
> - target_description_filename = NULL;
> + target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
> +
> + xfree (tdesc_info->filename);
> + tdesc_info->filename = nullptr;
> target_clear_description ();
> target_find_description ();
> }
> @@ -1778,8 +1776,9 @@ maint_print_c_tdesc_cmd (const char *args, int from_tty)
> architecture's. This lets a GDB for one architecture generate C
> for another architecture's description, even though the gdbarch
> initialization code will reject the new description. */
> - tdesc = current_target_desc;
> - filename = target_description_filename;
> + target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
> + tdesc = tdesc_info->tdesc;
> + filename = tdesc_info->filename;
> }
> else
> {
> @@ -1850,7 +1849,7 @@ maint_print_xml_tdesc_cmd (const char *args, int from_tty)
> architecture's. This lets a GDB for one architecture generate XML
> for another architecture's description, even though the gdbarch
> initialization code will reject the new description. */
> - tdesc = current_target_desc;
> + tdesc = get_tdesc_info (current_inferior ())->tdesc;
> }
> else
> {
> --
> 2.30.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/4] gdb: change target_desc_info::fetched to bool
2021-05-06 16:49 ` [PATCH 2/4] gdb: change target_desc_info::fetched to bool Simon Marchi
@ 2021-05-07 10:18 ` Andrew Burgess
0 siblings, 0 replies; 10+ messages in thread
From: Andrew Burgess @ 2021-05-07 10:18 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
* Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> [2021-05-06 12:49:55 -0400]:
> gdb/ChangeLog:
>
> * target-descriptions.c (struct target_desc_info) <fetched>:
> bool.
> (target_find_description): Adjust.
> (target_clear_description): Adjust.
LGTM.
Thanks,
Andrew
>
> Change-Id: Ib69e097b38cf270e674f1249105d535a312954e1
> ---
> gdb/target-descriptions.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
> index fd9e5a9d115d..aee1478301b6 100644
> --- a/gdb/target-descriptions.c
> +++ b/gdb/target-descriptions.c
> @@ -446,7 +446,7 @@ struct target_desc_info
> /* A flag indicating that a description has already been fetched
> from the target, so it should not be queried again. */
>
> - int fetched;
> + bool fetched;
>
> /* The description fetched from the target, or NULL if the target
> did not supply any description. Only valid when
> @@ -577,7 +577,7 @@ target_find_description (void)
>
> /* Now that we know this description is usable, record that we
> fetched it. */
> - tdesc_info->fetched = 1;
> + tdesc_info->fetched = true;
> }
>
> /* Discard any description fetched from the current target, and switch
> @@ -591,7 +591,7 @@ target_clear_description (void)
> if (!tdesc_info->fetched)
> return;
>
> - tdesc_info->fetched = 0;
> + tdesc_info->fetched = false;
> tdesc_info->tdesc = nullptr;
>
> gdbarch_info info;
> --
> 2.30.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/4] gdb: (de-)allocate target_desc_info with new/delete
2021-05-06 16:49 ` [PATCH 3/4] gdb: (de-)allocate target_desc_info with new/delete Simon Marchi
@ 2021-05-07 10:20 ` Andrew Burgess
0 siblings, 0 replies; 10+ messages in thread
From: Andrew Burgess @ 2021-05-07 10:20 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
* Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> [2021-05-06 12:49:56 -0400]:
> In preparation for using non-POD types in the struct.
>
> gdb/ChangeLog:
>
> * target-descriptions.c (struct target_desc_info): Initialize
> fields.
> (get_tdesc_info): Use new.
> (target_desc_info_free): Use delete.
LGTM.
Thanks,
Andrew
>
> Change-Id: I10fdaeeae7cdbd7930ae7adeeb13f7f363c67c7a
> ---
> gdb/target-descriptions.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
> index aee1478301b6..341020fed762 100644
> --- a/gdb/target-descriptions.c
> +++ b/gdb/target-descriptions.c
> @@ -446,7 +446,7 @@ struct target_desc_info
> /* A flag indicating that a description has already been fetched
> from the target, so it should not be queried again. */
>
> - bool fetched;
> + bool fetched = false;
>
> /* The description fetched from the target, or NULL if the target
> did not supply any description. Only valid when
> @@ -454,12 +454,12 @@ struct target_desc_info
> code should access this; normally, the description should be
> accessed through the gdbarch object. */
>
> - const struct target_desc *tdesc;
> + const struct target_desc *tdesc = nullptr;
>
> /* The filename to read a target description from, as set by "set
> tdesc filename ..." */
>
> - char *filename;
> + char *filename = nullptr;
> };
>
> /* Get the inferior INF's target description info, allocating one on
> @@ -469,7 +469,8 @@ static struct target_desc_info *
> get_tdesc_info (struct inferior *inf)
> {
> if (inf->tdesc_info == NULL)
> - inf->tdesc_info = XCNEW (struct target_desc_info);
> + inf->tdesc_info = new target_desc_info;
> +
> return inf->tdesc_info;
> }
>
> @@ -507,7 +508,7 @@ target_desc_info_free (struct target_desc_info *tdesc_info)
> if (tdesc_info != NULL)
> {
> xfree (tdesc_info->filename);
> - xfree (tdesc_info);
> + delete tdesc_info;
> }
> }
>
> --
> 2.30.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] gdb: make target_desc_info::filename an std::string
2021-05-06 16:49 ` [PATCH 4/4] gdb: make target_desc_info::filename an std::string Simon Marchi
@ 2021-05-07 10:22 ` Andrew Burgess
2021-05-07 20:31 ` Simon Marchi
0 siblings, 1 reply; 10+ messages in thread
From: Andrew Burgess @ 2021-05-07 10:22 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
* Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> [2021-05-06 12:49:57 -0400]:
> To make the management of memory automatic.
>
> As to why I chose to make this an std::string and not an
> std::unique_xmalloc_ptr<char>: some parts of the code consider both a
> NULL value and an empty string value to mean "no filename".
> target_desc_info_from_user_p, however, doesn't check for a non-NULL but
> empty string value. So it seems like having two ways of denoting "no
> filename" can lead to these kinds of inconsistencies. Using
> std::string, "no filename" is only represented by an empty value.
>
> As a bonus, using an std::string lets us copy target_desc_info objects
> using the default assignment operator.
>
> gdb/ChangeLog:
>
> * target-descriptions.c (struct target_desc_info) <filename>:
> Make std::string.
> (copy_inferior_target_desc_info): Adjust.
> (target_desc_info_free): Adjust.
> (target_find_description): Adjust.
> (set_tdesc_filename_cmd): Adjust.
> (show_tdesc_filename_cmd): Adjust.
> (unset_tdesc_filename_cmd): Adjust.
> (maint_print_c_tdesc_cmd): Adjust.
LGTM.
Thanks,
Andrew
>
> Change-Id: I4e3a6ad8ccda2b88c202471d4f54249753cad127
> ---
> gdb/target-descriptions.c | 34 ++++++++++++++--------------------
> 1 file changed, 14 insertions(+), 20 deletions(-)
>
> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
> index 341020fed762..053864dec58b 100644
> --- a/gdb/target-descriptions.c
> +++ b/gdb/target-descriptions.c
> @@ -456,10 +456,12 @@ struct target_desc_info
>
> const struct target_desc *tdesc = nullptr;
>
> - /* The filename to read a target description from, as set by "set
> - tdesc filename ..." */
> + /* If not empty, the filename to read a target description from, as set by
> + "set tdesc filename ...".
>
> - char *filename = nullptr;
> + If empty, there is not filename specified by the user. */
> +
> + std::string filename;
> };
>
> /* Get the inferior INF's target description info, allocating one on
> @@ -484,7 +486,7 @@ static struct gdbarch_data *tdesc_data;
> int
> target_desc_info_from_user_p (struct target_desc_info *info)
> {
> - return info != NULL && info->filename != NULL;
> + return info != nullptr && !info->filename.empty ();
> }
>
> /* See target-descriptions.h. */
> @@ -495,9 +497,7 @@ copy_inferior_target_desc_info (struct inferior *destinf, struct inferior *srcin
> struct target_desc_info *src = get_tdesc_info (srcinf);
> struct target_desc_info *dest = get_tdesc_info (destinf);
>
> - dest->fetched = src->fetched;
> - dest->tdesc = src->tdesc;
> - dest->filename = src->filename != NULL ? xstrdup (src->filename) : NULL;
> + *dest = *src;
> }
>
> /* See target-descriptions.h. */
> @@ -505,11 +505,7 @@ copy_inferior_target_desc_info (struct inferior *destinf, struct inferior *srcin
> void
> target_desc_info_free (struct target_desc_info *tdesc_info)
> {
> - if (tdesc_info != NULL)
> - {
> - xfree (tdesc_info->filename);
> - delete tdesc_info;
> - }
> + delete tdesc_info;
> }
>
> /* The string manipulated by the "set tdesc filename ..." command. */
> @@ -539,8 +535,8 @@ target_find_description (void)
> /* First try to fetch an XML description from the user-specified
> file. */
> tdesc_info->tdesc = nullptr;
> - if (tdesc_info->filename != nullptr && *tdesc_info->filename != '\0')
> - tdesc_info->tdesc= file_read_description_xml (tdesc_info->filename);
> + if (!tdesc_info->filename.empty ())
> + tdesc_info->tdesc = file_read_description_xml (tdesc_info->filename.data ());
>
> /* Next try to read the description from the current target using
> target objects. */
> @@ -1295,8 +1291,7 @@ set_tdesc_filename_cmd (const char *args, int from_tty,
> {
> target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
>
> - xfree (tdesc_info->filename);
> - tdesc_info->filename = xstrdup (tdesc_filename_cmd_string);
> + tdesc_info->filename = tdesc_filename_cmd_string;
>
> target_clear_description ();
> target_find_description ();
> @@ -1307,7 +1302,7 @@ show_tdesc_filename_cmd (struct ui_file *file, int from_tty,
> struct cmd_list_element *c,
> const char *value)
> {
> - value = get_tdesc_info (current_inferior ())->filename;
> + value = get_tdesc_info (current_inferior ())->filename.data ();
>
> if (value != NULL && *value != '\0')
> printf_filtered (_("The target description will be read from \"%s\".\n"),
> @@ -1322,8 +1317,7 @@ unset_tdesc_filename_cmd (const char *args, int from_tty)
> {
> target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
>
> - xfree (tdesc_info->filename);
> - tdesc_info->filename = nullptr;
> + tdesc_info->filename.clear ();
> target_clear_description ();
> target_find_description ();
> }
> @@ -1779,7 +1773,7 @@ maint_print_c_tdesc_cmd (const char *args, int from_tty)
> initialization code will reject the new description. */
> target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
> tdesc = tdesc_info->tdesc;
> - filename = tdesc_info->filename;
> + filename = tdesc_info->filename.data ();
> }
> else
> {
> --
> 2.30.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] gdb: remove target description macros
2021-05-07 10:17 ` [PATCH 1/4] gdb: remove target description macros Andrew Burgess
@ 2021-05-07 19:42 ` Simon Marchi
0 siblings, 0 replies; 10+ messages in thread
From: Simon Marchi @ 2021-05-07 19:42 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
On 2021-05-07 6:17 a.m., Andrew Burgess wrote:
>> + if (tdesc_info->filename != nullptr && *tdesc_info->filename != '\0')
>> + tdesc_info->tdesc= file_read_description_xml (tdesc_info->filename);
> Missed a space before '=' here.
>
> Otherwise, looks great.
>
> Thanks,
> Andrew
>
Fixed, thanks!
Simon
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] gdb: make target_desc_info::filename an std::string
2021-05-07 10:22 ` Andrew Burgess
@ 2021-05-07 20:31 ` Simon Marchi
0 siblings, 0 replies; 10+ messages in thread
From: Simon Marchi @ 2021-05-07 20:31 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
On 2021-05-07 6:22 a.m., Andrew Burgess wrote:
> * Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> [2021-05-06 12:49:57 -0400]:
>
>> To make the management of memory automatic.
>>
>> As to why I chose to make this an std::string and not an
>> std::unique_xmalloc_ptr<char>: some parts of the code consider both a
>> NULL value and an empty string value to mean "no filename".
>> target_desc_info_from_user_p, however, doesn't check for a non-NULL but
>> empty string value. So it seems like having two ways of denoting "no
>> filename" can lead to these kinds of inconsistencies. Using
>> std::string, "no filename" is only represented by an empty value.
>>
>> As a bonus, using an std::string lets us copy target_desc_info objects
>> using the default assignment operator.
>>
>> gdb/ChangeLog:
>>
>> * target-descriptions.c (struct target_desc_info) <filename>:
>> Make std::string.
>> (copy_inferior_target_desc_info): Adjust.
>> (target_desc_info_free): Adjust.
>> (target_find_description): Adjust.
>> (set_tdesc_filename_cmd): Adjust.
>> (show_tdesc_filename_cmd): Adjust.
>> (unset_tdesc_filename_cmd): Adjust.
>> (maint_print_c_tdesc_cmd): Adjust.
>
> LGTM.
>
> Thanks,
> Andrew
Thanks, all pushed.
Simon
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-05-07 20:32 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-06 16:49 [PATCH 1/4] gdb: remove target description macros Simon Marchi
2021-05-06 16:49 ` [PATCH 2/4] gdb: change target_desc_info::fetched to bool Simon Marchi
2021-05-07 10:18 ` Andrew Burgess
2021-05-06 16:49 ` [PATCH 3/4] gdb: (de-)allocate target_desc_info with new/delete Simon Marchi
2021-05-07 10:20 ` Andrew Burgess
2021-05-06 16:49 ` [PATCH 4/4] gdb: make target_desc_info::filename an std::string Simon Marchi
2021-05-07 10:22 ` Andrew Burgess
2021-05-07 20:31 ` Simon Marchi
2021-05-07 10:17 ` [PATCH 1/4] gdb: remove target description macros Andrew Burgess
2021-05-07 19:42 ` Simon Marchi
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).