[PATCH 0/5] Some target_desc_info cleanups

[PATCH 0/5] Some target_desc_info cleanups

[Simon Marchi]

While reviewing another series, I noticed some things about
target_desc_info that could be cleanup or simplified.

Simon Marchi (5):
  gdb: move target_desc_info to inferior.h
  gdb: change inferior::tdesc_info to non-pointer
  gdb: remove get_tdesc_info
  gdb: remove copy_inferior_target_desc_info
  gdb: make target_desc_info_from_user_p a method of target_desc_info

 gdb/inferior.c            |  7 +---
 gdb/inferior.h            | 30 ++++++++++++++-
 gdb/infrun.c              |  4 +-
 gdb/target-descriptions.c | 81 ++++-----------------------------------
 gdb/target-descriptions.h | 19 ---------
 5 files changed, 41 insertions(+), 100 deletions(-)


base-commit: e3ee979c1f24984eb93b32822338aa425afbc2af
-- 
2.39.1

[PATCH 1/5] gdb: move target_desc_info to inferior.h

[Simon Marchi]

In preparation for the following patch, where struct inferior needs to
"see" struct target_desc_info, move target_desc_info to the header file.

I initially moved the structure to target-descriptions.h, and later made
inferior.h include target-descriptions.h.  This worked, but it then
occured to me that target_desc_info is really an inferior property that
involves a target description, so I think it makes sense to have it in
inferior.h.

Change-Id: I3e81d04faafcad431e294357389f3d4c601ee83d
---
 gdb/inferior.h            | 23 +++++++++++++++++++++++
 gdb/target-descriptions.c | 26 --------------------------
 2 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/gdb/inferior.h b/gdb/inferior.h
index 4d001b0ad50e..5b5eab00546c 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -340,6 +340,29 @@ extern void set_current_inferior (inferior *);
    selected.  */
 extern void switch_to_inferior_no_thread (inferior *inf);
 
+/* Info about an inferior's target description.  There's one of these
+   for each inferior.  */
+
+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 = false;
+
+  /* The description fetched from the target, or NULL if the target
+     did not supply any description.  Only valid when
+     FETCHED is set.  Only the description initialization
+     code should access this; normally, the description should be
+     accessed through the gdbarch object.  */
+  const struct target_desc *tdesc = nullptr;
+
+  /* If not empty, the filename to read a target description from, as set by
+     "set tdesc filename ...".
+
+     If empty, there is not filename specified by the user.  */
+  std::string filename;
+};
+
 /* GDB represents the state of each program execution with an object
    called an inferior.  An inferior typically corresponds to a process
    but is more general and applies also to targets that do not have a
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 1a451c79b824..076feed0008c 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -436,32 +436,6 @@ struct tdesc_arch_data
   gdbarch_register_reggroup_p_ftype *pseudo_register_reggroup_p = NULL;
 };
 
-/* Info about an inferior's target description.  There's one of these
-   for each inferior.  */
-
-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 = false;
-
-  /* The description fetched from the target, or NULL if the target
-     did not supply any description.  Only valid when
-     FETCHED is set.  Only the description initialization
-     code should access this; normally, the description should be
-     accessed through the gdbarch object.  */
-
-  const struct target_desc *tdesc = nullptr;
-
-  /* If not empty, the filename to read a target description from, as set by
-     "set tdesc filename ...".
-
-     If empty, there is not filename specified by the user.  */
-
-  std::string filename;
-};
-
 /* Get the inferior INF's target description info, allocating one on
    the stop if necessary.  */
 
-- 
2.39.1

[PATCH 2/5] gdb: change inferior::tdesc_info to non-pointer

[Simon Marchi]

I initially made this field a unique pointer, to have automatic memory
management.  But I then thought that the field didn't really need to be
allocated separately from struct inferior.  So make it a regular
non-pointer field of inferior.

Remove target_desc_info_free, as it's no longer needed.

Change-Id: Ica2b97071226f31c40e86222a2f6922454df1229
---
 gdb/inferior.c            |  5 +----
 gdb/inferior.h            |  2 +-
 gdb/target-descriptions.c | 16 ++--------------
 gdb/target-descriptions.h |  4 ----
 4 files changed, 4 insertions(+), 23 deletions(-)

diff --git a/gdb/inferior.c b/gdb/inferior.c
index b0ecca8b63a3..dfe523664de0 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -69,8 +69,6 @@ private_inferior::~private_inferior () = default;
 
 inferior::~inferior ()
 {
-  inferior *inf = this;
-
   /* Before the inferior is deleted, all target_ops should be popped from
      the target stack, this leaves just the dummy_target behind.  If this
      is not done, then any target left in the target stack will be left
@@ -81,7 +79,6 @@ inferior::~inferior ()
   gdb_assert (m_target_stack.top ()->stratum () == dummy_stratum);
 
   m_continuations.clear ();
-  target_desc_info_free (inf->tdesc_info);
 }
 
 inferior::inferior (int pid_)
@@ -964,7 +961,7 @@ clone_inferior_command (const char *args, int from_tty)
 
       /* If the original inferior had a user specified target
 	 description, make the clone use it too.  */
-      if (target_desc_info_from_user_p (inf->tdesc_info))
+      if (target_desc_info_from_user_p (&inf->tdesc_info))
 	copy_inferior_target_desc_info (inf, orginf);
 
       clone_program_space (pspace, orginf->pspace);
diff --git a/gdb/inferior.h b/gdb/inferior.h
index 5b5eab00546c..d902881bfe24 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -633,7 +633,7 @@ class inferior : public refcounted_object,
 
   /* Info about an inferior's target description (if it's fetched; the
      user supplied description's filename, if any; etc.).  */
-  target_desc_info *tdesc_info = NULL;
+  target_desc_info tdesc_info;
 
   /* The architecture associated with the inferior through the
      connection to the target.
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 076feed0008c..049e42c7ea77 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -436,16 +436,12 @@ struct tdesc_arch_data
   gdbarch_register_reggroup_p_ftype *pseudo_register_reggroup_p = NULL;
 };
 
-/* Get the inferior INF's target description info, allocating one on
-   the stop if necessary.  */
+/* Get the inferior INF's target description info.  */
 
 static struct target_desc_info *
 get_tdesc_info (struct inferior *inf)
 {
-  if (inf->tdesc_info == NULL)
-    inf->tdesc_info = new target_desc_info;
-
-  return inf->tdesc_info;
+  return &inf->tdesc_info;
 }
 
 /* A handle for architecture-specific data associated with the
@@ -482,14 +478,6 @@ copy_inferior_target_desc_info (struct inferior *destinf, struct inferior *srcin
   *dest = *src;
 }
 
-/* See target-descriptions.h.  */
-
-void
-target_desc_info_free (struct target_desc_info *tdesc_info)
-{
-  delete tdesc_info;
-}
-
 /* The string manipulated by the "set tdesc filename ..." command.  */
 
 static std::string tdesc_filename_cmd_string;
diff --git a/gdb/target-descriptions.h b/gdb/target-descriptions.h
index 93bf382a18ea..c337c177c8e8 100644
--- a/gdb/target-descriptions.h
+++ b/gdb/target-descriptions.h
@@ -55,10 +55,6 @@ const struct target_desc *target_current_description (void);
 void copy_inferior_target_desc_info (struct inferior *destinf,
 				     struct inferior *srcinf);
 
-/* Free a target_desc_info object.  */
-
-void target_desc_info_free (struct target_desc_info *tdesc_info);
-
 /* Returns true if INFO indicates the target description had been
    supplied by the user.  */
 
-- 
2.39.1

[PATCH 3/5] gdb: remove get_tdesc_info

[Simon Marchi]

Remove this function, since it's now a trivial access to
inferior::tdesc_info.

Change-Id: I3e88a8214034f1a4163420b434be11f51eef462c
---
 gdb/target-descriptions.c | 28 ++++++++++------------------
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 049e42c7ea77..0561a8098c5c 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -436,14 +436,6 @@ struct tdesc_arch_data
   gdbarch_register_reggroup_p_ftype *pseudo_register_reggroup_p = NULL;
 };
 
-/* Get the inferior INF's target description info.  */
-
-static struct target_desc_info *
-get_tdesc_info (struct inferior *inf)
-{
-  return &inf->tdesc_info;
-}
-
 /* A handle for architecture-specific data associated with the
    target description (see struct tdesc_arch_data).  */
 
@@ -472,8 +464,8 @@ target_desc_info_from_user_p (struct target_desc_info *info)
 void
 copy_inferior_target_desc_info (struct inferior *destinf, struct inferior *srcinf)
 {
-  struct target_desc_info *src = get_tdesc_info (srcinf);
-  struct target_desc_info *dest = get_tdesc_info (destinf);
+  struct target_desc_info *src = &srcinf->tdesc_info;
+  struct target_desc_info *dest = &destinf->tdesc_info;
 
   *dest = *src;
 }
@@ -488,7 +480,7 @@ static std::string tdesc_filename_cmd_string;
 void
 target_find_description (void)
 {
-  target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
+  target_desc_info *tdesc_info = &current_inferior ()->tdesc_info;
 
   /* If we've already fetched a description from the target, don't do
      it again.  This allows a target to fetch the description early,
@@ -551,7 +543,7 @@ target_find_description (void)
 void
 target_clear_description (void)
 {
-  target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
+  target_desc_info *tdesc_info = &current_inferior ()->tdesc_info;
 
   if (!tdesc_info->fetched)
     return;
@@ -571,7 +563,7 @@ target_clear_description (void)
 const struct target_desc *
 target_current_description (void)
 {
-  target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
+  target_desc_info *tdesc_info = &current_inferior ()->tdesc_info;
 
   if (tdesc_info->fetched)
     return tdesc_info->tdesc;
@@ -1246,7 +1238,7 @@ static void
 set_tdesc_filename_cmd (const char *args, int from_tty,
 			struct cmd_list_element *c)
 {
-  target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
+  target_desc_info *tdesc_info = &current_inferior ()->tdesc_info;
 
   tdesc_info->filename = tdesc_filename_cmd_string;
 
@@ -1259,7 +1251,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.data ();
+  value = current_inferior ()->tdesc_info.filename.data ();
 
   if (value != NULL && *value != '\0')
     gdb_printf (file,
@@ -1274,7 +1266,7 @@ show_tdesc_filename_cmd (struct ui_file *file, int from_tty,
 static void
 unset_tdesc_filename_cmd (const char *args, int from_tty)
 {
-  target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
+  target_desc_info *tdesc_info = &current_inferior ()->tdesc_info;
 
   tdesc_info->filename.clear ();
   target_clear_description ();
@@ -1730,7 +1722,7 @@ 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.  */
-      target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
+      target_desc_info *tdesc_info = &current_inferior ()->tdesc_info;
       tdesc = tdesc_info->tdesc;
       filename = tdesc_info->filename.data ();
     }
@@ -1803,7 +1795,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 = get_tdesc_info (current_inferior ())->tdesc;
+      tdesc = current_inferior ()->tdesc_info.tdesc;
     }
   else
     {
-- 
2.39.1

[PATCH 4/5] gdb: remove copy_inferior_target_desc_info

[Simon Marchi]

This function is now trivial, we can just copy inferior::tdesc_info
where needed.

Change-Id: I25185e2cd4ba1ef24a822d9e0eebec6e611d54d6
---
 gdb/inferior.c            |  2 +-
 gdb/infrun.c              |  4 ++--
 gdb/target-descriptions.c | 11 -----------
 gdb/target-descriptions.h |  7 -------
 4 files changed, 3 insertions(+), 21 deletions(-)

diff --git a/gdb/inferior.c b/gdb/inferior.c
index dfe523664de0..65863440b9c0 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -962,7 +962,7 @@ clone_inferior_command (const char *args, int from_tty)
       /* If the original inferior had a user specified target
 	 description, make the clone use it too.  */
       if (target_desc_info_from_user_p (&inf->tdesc_info))
-	copy_inferior_target_desc_info (inf, orginf);
+	inf->tdesc_info = orginf->tdesc_info;
 
       clone_program_space (pspace, orginf->pspace);
 
diff --git a/gdb/infrun.c b/gdb/infrun.c
index edfb5ab0a91f..87ab73c47a4f 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -478,7 +478,7 @@ holding the child stopped.  Try \"set detach-on-fork\" or \
 	  child_inf->attach_flag = parent_inf->attach_flag;
 	  copy_terminal_info (child_inf, parent_inf);
 	  child_inf->gdbarch = parent_inf->gdbarch;
-	  copy_inferior_target_desc_info (child_inf, parent_inf);
+	  child_inf->tdesc_info = parent_inf->tdesc_info;
 
 	  child_inf->symfile_flags = SYMFILE_NO_READ;
 
@@ -546,7 +546,7 @@ holding the child stopped.  Try \"set detach-on-fork\" or \
       child_inf->attach_flag = parent_inf->attach_flag;
       copy_terminal_info (child_inf, parent_inf);
       child_inf->gdbarch = parent_inf->gdbarch;
-      copy_inferior_target_desc_info (child_inf, parent_inf);
+      child_inf->tdesc_info = parent_inf->tdesc_info;
 
       if (has_vforked)
 	{
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 0561a8098c5c..6defd5bbe863 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -459,17 +459,6 @@ target_desc_info_from_user_p (struct target_desc_info *info)
   return info != nullptr && !info->filename.empty ();
 }
 
-/* See target-descriptions.h.  */
-
-void
-copy_inferior_target_desc_info (struct inferior *destinf, struct inferior *srcinf)
-{
-  struct target_desc_info *src = &srcinf->tdesc_info;
-  struct target_desc_info *dest = &destinf->tdesc_info;
-
-  *dest = *src;
-}
-
 /* The string manipulated by the "set tdesc filename ..." command.  */
 
 static std::string tdesc_filename_cmd_string;
diff --git a/gdb/target-descriptions.h b/gdb/target-descriptions.h
index c337c177c8e8..b835e144c680 100644
--- a/gdb/target-descriptions.h
+++ b/gdb/target-descriptions.h
@@ -48,13 +48,6 @@ void target_clear_description (void);
 
 const struct target_desc *target_current_description (void);
 
-/* Copy inferior target description data.  Used for example when
-   handling (v)forks, where child's description is the same as the
-   parent's, since the child really is a copy of the parent.  */
-
-void copy_inferior_target_desc_info (struct inferior *destinf,
-				     struct inferior *srcinf);
-
 /* Returns true if INFO indicates the target description had been
    supplied by the user.  */
 
-- 
2.39.1

[PATCH 5/5] gdb: make target_desc_info_from_user_p a method of target_desc_info

[Simon Marchi]

Move the implementation over to target_desc_info.  Remove the
target_desc_info forward declaration in target-descriptions.h, it's no
longer needed.

Change-Id: Ic95060341685afe0b73af591ca6efe32f5e7e892
---
 gdb/inferior.c            | 2 +-
 gdb/inferior.h            | 5 +++++
 gdb/target-descriptions.c | 8 --------
 gdb/target-descriptions.h | 8 --------
 4 files changed, 6 insertions(+), 17 deletions(-)

diff --git a/gdb/inferior.c b/gdb/inferior.c
index 65863440b9c0..a1e3c79d8a20 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -961,7 +961,7 @@ clone_inferior_command (const char *args, int from_tty)
 
       /* If the original inferior had a user specified target
 	 description, make the clone use it too.  */
-      if (target_desc_info_from_user_p (&inf->tdesc_info))
+      if (inf->tdesc_info.from_user_p ())
 	inf->tdesc_info = orginf->tdesc_info;
 
       clone_program_space (pspace, orginf->pspace);
diff --git a/gdb/inferior.h b/gdb/inferior.h
index d902881bfe24..bac483f0f78d 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -345,6 +345,11 @@ extern void switch_to_inferior_no_thread (inferior *inf);
 
 struct target_desc_info
 {
+  /* Returns true if INFO indicates the target description had been supplied by
+     the user.  */
+  bool from_user_p ()
+  { return !this->filename.empty (); }
+
   /* A flag indicating that a description has already been fetched
      from the target, so it should not be queried again.  */
   bool fetched = false;
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 6defd5bbe863..b08a185dfa21 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -451,14 +451,6 @@ get_arch_data (struct gdbarch *gdbarch)
   return result;
 }
 
-/* See target-descriptions.h.  */
-
-int
-target_desc_info_from_user_p (struct target_desc_info *info)
-{
-  return info != nullptr && !info->filename.empty ();
-}
-
 /* The string manipulated by the "set tdesc filename ..." command.  */
 
 static std::string tdesc_filename_cmd_string;
diff --git a/gdb/target-descriptions.h b/gdb/target-descriptions.h
index b835e144c680..ee48fdfaa0c2 100644
--- a/gdb/target-descriptions.h
+++ b/gdb/target-descriptions.h
@@ -26,9 +26,6 @@
 
 struct tdesc_arch_data;
 struct target_ops;
-/* An inferior's target description info is stored in this opaque
-   object.  There's one such object per inferior.  */
-struct target_desc_info;
 struct inferior;
 
 /* Fetch the current inferior's description, and switch its current
@@ -48,11 +45,6 @@ void target_clear_description (void);
 
 const struct target_desc *target_current_description (void);
 
-/* Returns true if INFO indicates the target description had been
-   supplied by the user.  */
-
-int target_desc_info_from_user_p (struct target_desc_info *info);
-
 /* Record architecture-specific functions to call for pseudo-register
    support.  If tdesc_use_registers is called and gdbarch_num_pseudo_regs
    is greater than zero, then these should be called as well.
-- 
2.39.1

Re: [PATCH 3/5] gdb: remove get_tdesc_info

[Andrew Burgess]

Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

> Remove this function, since it's now a trivial access to
> inferior::tdesc_info.
>
> Change-Id: I3e88a8214034f1a4163420b434be11f51eef462c
> ---
>  gdb/target-descriptions.c | 28 ++++++++++------------------
>  1 file changed, 10 insertions(+), 18 deletions(-)
>
> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
> index 049e42c7ea77..0561a8098c5c 100644
> --- a/gdb/target-descriptions.c
> +++ b/gdb/target-descriptions.c
> @@ -436,14 +436,6 @@ struct tdesc_arch_data
>    gdbarch_register_reggroup_p_ftype *pseudo_register_reggroup_p = NULL;
>  };
>  
> -/* Get the inferior INF's target description info.  */
> -
> -static struct target_desc_info *
> -get_tdesc_info (struct inferior *inf)
> -{
> -  return &inf->tdesc_info;
> -}
> -
>  /* A handle for architecture-specific data associated with the
>     target description (see struct tdesc_arch_data).  */
>  
> @@ -472,8 +464,8 @@ target_desc_info_from_user_p (struct target_desc_info *info)
>  void
>  copy_inferior_target_desc_info (struct inferior *destinf, struct inferior *srcinf)
>  {
> -  struct target_desc_info *src = get_tdesc_info (srcinf);
> -  struct target_desc_info *dest = get_tdesc_info (destinf);
> +  struct target_desc_info *src = &srcinf->tdesc_info;
> +  struct target_desc_info *dest = &destinf->tdesc_info;
>  
>    *dest = *src;

Would:

  destinf->tdesc_info = srcinf->tdesc_info;

not be better now?

Thanks,
Andrew


>  }
> @@ -488,7 +480,7 @@ static std::string tdesc_filename_cmd_string;
>  void
>  target_find_description (void)
>  {
> -  target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
> +  target_desc_info *tdesc_info = &current_inferior ()->tdesc_info;
>  
>    /* If we've already fetched a description from the target, don't do
>       it again.  This allows a target to fetch the description early,
> @@ -551,7 +543,7 @@ target_find_description (void)
>  void
>  target_clear_description (void)
>  {
> -  target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
> +  target_desc_info *tdesc_info = &current_inferior ()->tdesc_info;
>  
>    if (!tdesc_info->fetched)
>      return;
> @@ -571,7 +563,7 @@ target_clear_description (void)
>  const struct target_desc *
>  target_current_description (void)
>  {
> -  target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
> +  target_desc_info *tdesc_info = &current_inferior ()->tdesc_info;
>  
>    if (tdesc_info->fetched)
>      return tdesc_info->tdesc;
> @@ -1246,7 +1238,7 @@ static void
>  set_tdesc_filename_cmd (const char *args, int from_tty,
>  			struct cmd_list_element *c)
>  {
> -  target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
> +  target_desc_info *tdesc_info = &current_inferior ()->tdesc_info;
>  
>    tdesc_info->filename = tdesc_filename_cmd_string;
>  
> @@ -1259,7 +1251,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.data ();
> +  value = current_inferior ()->tdesc_info.filename.data ();
>  
>    if (value != NULL && *value != '\0')
>      gdb_printf (file,
> @@ -1274,7 +1266,7 @@ show_tdesc_filename_cmd (struct ui_file *file, int from_tty,
>  static void
>  unset_tdesc_filename_cmd (const char *args, int from_tty)
>  {
> -  target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
> +  target_desc_info *tdesc_info = &current_inferior ()->tdesc_info;
>  
>    tdesc_info->filename.clear ();
>    target_clear_description ();
> @@ -1730,7 +1722,7 @@ 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.  */
> -      target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
> +      target_desc_info *tdesc_info = &current_inferior ()->tdesc_info;
>        tdesc = tdesc_info->tdesc;
>        filename = tdesc_info->filename.data ();
>      }
> @@ -1803,7 +1795,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 = get_tdesc_info (current_inferior ())->tdesc;
> +      tdesc = current_inferior ()->tdesc_info.tdesc;
>      }
>    else
>      {
> -- 
> 2.39.1

Re: [PATCH 4/5] gdb: remove copy_inferior_target_desc_info

[Andrew Burgess]

Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

> This function is now trivial, we can just copy inferior::tdesc_info
> where needed.

I should look ahead before commenting on earlier patches :)

Thanks,
Andrew


>
> Change-Id: I25185e2cd4ba1ef24a822d9e0eebec6e611d54d6
> ---
>  gdb/inferior.c            |  2 +-
>  gdb/infrun.c              |  4 ++--
>  gdb/target-descriptions.c | 11 -----------
>  gdb/target-descriptions.h |  7 -------
>  4 files changed, 3 insertions(+), 21 deletions(-)
>
> diff --git a/gdb/inferior.c b/gdb/inferior.c
> index dfe523664de0..65863440b9c0 100644
> --- a/gdb/inferior.c
> +++ b/gdb/inferior.c
> @@ -962,7 +962,7 @@ clone_inferior_command (const char *args, int from_tty)
>        /* If the original inferior had a user specified target
>  	 description, make the clone use it too.  */
>        if (target_desc_info_from_user_p (&inf->tdesc_info))
> -	copy_inferior_target_desc_info (inf, orginf);
> +	inf->tdesc_info = orginf->tdesc_info;
>  
>        clone_program_space (pspace, orginf->pspace);
>  
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index edfb5ab0a91f..87ab73c47a4f 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -478,7 +478,7 @@ holding the child stopped.  Try \"set detach-on-fork\" or \
>  	  child_inf->attach_flag = parent_inf->attach_flag;
>  	  copy_terminal_info (child_inf, parent_inf);
>  	  child_inf->gdbarch = parent_inf->gdbarch;
> -	  copy_inferior_target_desc_info (child_inf, parent_inf);
> +	  child_inf->tdesc_info = parent_inf->tdesc_info;
>  
>  	  child_inf->symfile_flags = SYMFILE_NO_READ;
>  
> @@ -546,7 +546,7 @@ holding the child stopped.  Try \"set detach-on-fork\" or \
>        child_inf->attach_flag = parent_inf->attach_flag;
>        copy_terminal_info (child_inf, parent_inf);
>        child_inf->gdbarch = parent_inf->gdbarch;
> -      copy_inferior_target_desc_info (child_inf, parent_inf);
> +      child_inf->tdesc_info = parent_inf->tdesc_info;
>  
>        if (has_vforked)
>  	{
> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
> index 0561a8098c5c..6defd5bbe863 100644
> --- a/gdb/target-descriptions.c
> +++ b/gdb/target-descriptions.c
> @@ -459,17 +459,6 @@ target_desc_info_from_user_p (struct target_desc_info *info)
>    return info != nullptr && !info->filename.empty ();
>  }
>  
> -/* See target-descriptions.h.  */
> -
> -void
> -copy_inferior_target_desc_info (struct inferior *destinf, struct inferior *srcinf)
> -{
> -  struct target_desc_info *src = &srcinf->tdesc_info;
> -  struct target_desc_info *dest = &destinf->tdesc_info;
> -
> -  *dest = *src;
> -}
> -
>  /* The string manipulated by the "set tdesc filename ..." command.  */
>  
>  static std::string tdesc_filename_cmd_string;
> diff --git a/gdb/target-descriptions.h b/gdb/target-descriptions.h
> index c337c177c8e8..b835e144c680 100644
> --- a/gdb/target-descriptions.h
> +++ b/gdb/target-descriptions.h
> @@ -48,13 +48,6 @@ void target_clear_description (void);
>  
>  const struct target_desc *target_current_description (void);
>  
> -/* Copy inferior target description data.  Used for example when
> -   handling (v)forks, where child's description is the same as the
> -   parent's, since the child really is a copy of the parent.  */
> -
> -void copy_inferior_target_desc_info (struct inferior *destinf,
> -				     struct inferior *srcinf);
> -
>  /* Returns true if INFO indicates the target description had been
>     supplied by the user.  */
>  
> -- 
> 2.39.1

Re: [PATCH 0/5] Some target_desc_info cleanups

[Andrew Burgess]

Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

> While reviewing another series, I noticed some things about
> target_desc_info that could be cleanup or simplified.

Took a look through the series.  Everything looks like a good cleanup to
me.

Thanks,
Andrew

>
> Simon Marchi (5):
>   gdb: move target_desc_info to inferior.h
>   gdb: change inferior::tdesc_info to non-pointer
>   gdb: remove get_tdesc_info
>   gdb: remove copy_inferior_target_desc_info
>   gdb: make target_desc_info_from_user_p a method of target_desc_info
>
>  gdb/inferior.c            |  7 +---
>  gdb/inferior.h            | 30 ++++++++++++++-
>  gdb/infrun.c              |  4 +-
>  gdb/target-descriptions.c | 81 ++++-----------------------------------
>  gdb/target-descriptions.h | 19 ---------
>  5 files changed, 41 insertions(+), 100 deletions(-)
>
>
> base-commit: e3ee979c1f24984eb93b32822338aa425afbc2af
> -- 
> 2.39.1

Re: [PATCH 3/5] gdb: remove get_tdesc_info

[Simon Marchi]

On 2/3/23 10:53, Andrew Burgess wrote:
> Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
>> Remove this function, since it's now a trivial access to
>> inferior::tdesc_info.
>>
>> Change-Id: I3e88a8214034f1a4163420b434be11f51eef462c
>> ---
>>  gdb/target-descriptions.c | 28 ++++++++++------------------
>>  1 file changed, 10 insertions(+), 18 deletions(-)
>>
>> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
>> index 049e42c7ea77..0561a8098c5c 100644
>> --- a/gdb/target-descriptions.c
>> +++ b/gdb/target-descriptions.c
>> @@ -436,14 +436,6 @@ struct tdesc_arch_data
>>    gdbarch_register_reggroup_p_ftype *pseudo_register_reggroup_p = NULL;
>>  };
>>  
>> -/* Get the inferior INF's target description info.  */
>> -
>> -static struct target_desc_info *
>> -get_tdesc_info (struct inferior *inf)
>> -{
>> -  return &inf->tdesc_info;
>> -}
>> -
>>  /* A handle for architecture-specific data associated with the
>>     target description (see struct tdesc_arch_data).  */
>>  
>> @@ -472,8 +464,8 @@ target_desc_info_from_user_p (struct target_desc_info *info)
>>  void
>>  copy_inferior_target_desc_info (struct inferior *destinf, struct inferior *srcinf)
>>  {
>> -  struct target_desc_info *src = get_tdesc_info (srcinf);
>> -  struct target_desc_info *dest = get_tdesc_info (destinf);
>> +  struct target_desc_info *src = &srcinf->tdesc_info;
>> +  struct target_desc_info *dest = &destinf->tdesc_info;
>>  
>>    *dest = *src;
> 
> Would:
> 
>   destinf->tdesc_info = srcinf->tdesc_info;
> 
> not be better now?

For others: this disappears in the next patch anyway.

Simon

Re: [PATCH 4/5] gdb: remove copy_inferior_target_desc_info

[Simon Marchi]

On 2/3/23 10:54, Andrew Burgess wrote:
> Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
>> This function is now trivial, we can just copy inferior::tdesc_info
>> where needed.
> 
> I should look ahead before commenting on earlier patches :)

Yeah, it happens to me all the time :).

Simon

Re: [PATCH 0/5] Some target_desc_info cleanups

[Simon Marchi]

On 2/3/23 10:56, Andrew Burgess wrote:
> Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
>> While reviewing another series, I noticed some things about
>> target_desc_info that could be cleanup or simplified.
> 
> Took a look through the series.  Everything looks like a good cleanup to
> me.

Thanks, will push.

Simon

Re: [PATCH 5/5] gdb: make target_desc_info_from_user_p a method of target_desc_info

[Pedro Alves]

On 2023-02-03 2:21 p.m., Simon Marchi via Gdb-patches wrote:
> --- a/gdb/inferior.h
> +++ b/gdb/inferior.h
> @@ -345,6 +345,11 @@ extern void switch_to_inferior_no_thread (inferior *inf);
>  
>  struct target_desc_info
>  {
> +  /* Returns true if INFO indicates the target description had been supplied by
> +     the user.  */

There is no INFO anymore.

> +  bool from_user_p ()
> +  { return !this->filename.empty (); }
> +

Re: [PATCH 5/5] gdb: make target_desc_info_from_user_p a method of target_desc_info

[Simon Marchi]

On 2/6/23 13:41, Pedro Alves wrote:
> On 2023-02-03 2:21 p.m., Simon Marchi via Gdb-patches wrote:
>> --- a/gdb/inferior.h
>> +++ b/gdb/inferior.h
>> @@ -345,6 +345,11 @@ extern void switch_to_inferior_no_thread (inferior *inf);
>>  
>>  struct target_desc_info
>>  {
>> +  /* Returns true if INFO indicates the target description had been supplied by
>> +     the user.  */
> 
> There is no INFO anymore.

Ah, thanks.  I pushed this fix.


From f9b677528fdde6b1d5f402b75db2efcbe62c93d8 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Mon, 6 Feb 2023 14:12:27 -0500
Subject: [PATCH] gdb: adjust comment on target_desc_info::from_user_p

Remove the stale reference to INFO, which is now "this target
description info" now.

Change-Id: I35dbdb089048ed7cfffe730d3134ee391b176abf
---
 gdb/inferior.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/inferior.h b/gdb/inferior.h
index bac483f0f78d..72034cc4ffbc 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -345,7 +345,7 @@ extern void switch_to_inferior_no_thread (inferior *inf);
 
 struct target_desc_info
 {
-  /* Returns true if INFO indicates the target description had been supplied by
+  /* Returns true if this target description information has been supplied by
      the user.  */
   bool from_user_p ()
   { return !this->filename.empty (); }

base-commit: 85df9457b72e4a198b4edfc908017612fca59509
-- 
2.39.1