[PATCH] Change target_bfd_reopen to take a gdb_bfd_ref_ptr

[PATCH] Change target_bfd_reopen to take a gdb_bfd_ref_ptr

[Tom Tromey]

While looking at Andrew's recent target sections series, I saw that
target_bfd_reopen took a "bfd *", leading to a call to new_reference.
However, because the only caller of target_bfd_reopen is already using
gdb_bfd_ref_ptr, this code can be simplified and the explicit call to
new_reference can be removed.

gdb/ChangeLog
2021-02-19  Tom Tromey  <tromey@adacore.com>

	* solib-svr4.c (enable_break): Update.
	* bfd-target.c (class target_bfd) <target_bfd>: Change parameter
	type.
	(target_bfd_reopen): Change parameter type.
	* bfd-target.h (target_bfd_reopen): Change parameter type.
---
 gdb/ChangeLog    |  8 ++++++++
 gdb/bfd-target.c | 10 +++++-----
 gdb/bfd-target.h |  5 +++--
 gdb/solib-svr4.c |  2 +-
 4 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/gdb/bfd-target.c b/gdb/bfd-target.c
index 90d247a981a..689dbb19e25 100644
--- a/gdb/bfd-target.c
+++ b/gdb/bfd-target.c
@@ -34,7 +34,7 @@ static const target_info target_bfd_target_info = {
 class target_bfd : public target_ops
 {
 public:
-  explicit target_bfd (struct bfd *bfd);
+  explicit target_bfd (const gdb_bfd_ref_ptr &bfd);
 
   const target_info &info () const override
   { return target_bfd_target_info; }
@@ -88,14 +88,14 @@ target_bfd::get_section_table ()
   return &m_table;
 }
 
-target_bfd::target_bfd (struct bfd *abfd)
-  : m_bfd (gdb_bfd_ref_ptr::new_reference (abfd)),
-    m_table (build_section_table (abfd))
+target_bfd::target_bfd (const gdb_bfd_ref_ptr &abfd)
+  : m_bfd (abfd),
+    m_table (build_section_table (abfd.get ()))
 {
 }
 
 target_ops *
-target_bfd_reopen (struct bfd *abfd)
+target_bfd_reopen (const gdb_bfd_ref_ptr &abfd)
 {
   return new target_bfd (abfd);
 }
diff --git a/gdb/bfd-target.h b/gdb/bfd-target.h
index 199ada8c73f..ff96100cae5 100644
--- a/gdb/bfd-target.h
+++ b/gdb/bfd-target.h
@@ -20,12 +20,13 @@
 #ifndef BFD_TARGET_H
 #define BFD_TARGET_H
 
-struct bfd;
+#include "gdb_bfd.h"
+
 struct target_ops;
 
 /* Given an existing BFD, re-open it as a "struct target_ops".  This
    acquires a new reference to the BFD.  This reference will be
    released when the target is closed.  */
-struct target_ops *target_bfd_reopen (struct bfd *bfd);
+struct target_ops *target_bfd_reopen (const gdb_bfd_ref_ptr &bfd);
 
 #endif
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 0416faa17c3..1331c5d5f56 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -2335,7 +2335,7 @@ enable_break (struct svr4_info *info, int from_tty)
       /* Now convert the TMP_BFD into a target.  That way target, as
 	 well as BFD operations can be used.  target_bfd_reopen
 	 acquires its own reference.  */
-      tmp_bfd_target = target_bfd_reopen (tmp_bfd.get ());
+      tmp_bfd_target = target_bfd_reopen (tmp_bfd);
 
       /* On a running target, we can get the dynamic linker's base
 	 address from the shared library table.  */
-- 
2.26.2

Re: [PATCH] Change target_bfd_reopen to take a gdb_bfd_ref_ptr

[Andrew Burgess]

* Tom Tromey <tromey@adacore.com> [2021-02-19 07:51:00 -0700]:

> While looking at Andrew's recent target sections series, I saw that
> target_bfd_reopen took a "bfd *", leading to a call to new_reference.
> However, because the only caller of target_bfd_reopen is already using
> gdb_bfd_ref_ptr, this code can be simplified and the explicit call to
> new_reference can be removed.
> 
> gdb/ChangeLog
> 2021-02-19  Tom Tromey  <tromey@adacore.com>
> 
> 	* solib-svr4.c (enable_break): Update.
> 	* bfd-target.c (class target_bfd) <target_bfd>: Change parameter
> 	type.
> 	(target_bfd_reopen): Change parameter type.
> 	* bfd-target.h (target_bfd_reopen): Change parameter type.
> ---
>  gdb/ChangeLog    |  8 ++++++++
>  gdb/bfd-target.c | 10 +++++-----
>  gdb/bfd-target.h |  5 +++--
>  gdb/solib-svr4.c |  2 +-
>  4 files changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/gdb/bfd-target.c b/gdb/bfd-target.c
> index 90d247a981a..689dbb19e25 100644
> --- a/gdb/bfd-target.c
> +++ b/gdb/bfd-target.c
> @@ -34,7 +34,7 @@ static const target_info target_bfd_target_info = {
>  class target_bfd : public target_ops
>  {
>  public:
> -  explicit target_bfd (struct bfd *bfd);
> +  explicit target_bfd (const gdb_bfd_ref_ptr &bfd);
>  
>    const target_info &info () const override
>    { return target_bfd_target_info; }
> @@ -88,14 +88,14 @@ target_bfd::get_section_table ()
>    return &m_table;
>  }
>  
> -target_bfd::target_bfd (struct bfd *abfd)
> -  : m_bfd (gdb_bfd_ref_ptr::new_reference (abfd)),
> -    m_table (build_section_table (abfd))
> +target_bfd::target_bfd (const gdb_bfd_ref_ptr &abfd)
> +  : m_bfd (abfd),
> +    m_table (build_section_table (abfd.get ()))
>  {
>  }
>  
>  target_ops *
> -target_bfd_reopen (struct bfd *abfd)
> +target_bfd_reopen (const gdb_bfd_ref_ptr &abfd)
>  {
>    return new target_bfd (abfd);
>  }
> diff --git a/gdb/bfd-target.h b/gdb/bfd-target.h
> index 199ada8c73f..ff96100cae5 100644
> --- a/gdb/bfd-target.h
> +++ b/gdb/bfd-target.h
> @@ -20,12 +20,13 @@
>  #ifndef BFD_TARGET_H
>  #define BFD_TARGET_H
>  
> -struct bfd;
> +#include "gdb_bfd.h"
> +
>  struct target_ops;
>  
>  /* Given an existing BFD, re-open it as a "struct target_ops".  This
>     acquires a new reference to the BFD.  This reference will be
>     released when the target is closed.  */
> -struct target_ops *target_bfd_reopen (struct bfd *bfd);
> +struct target_ops *target_bfd_reopen (const gdb_bfd_ref_ptr &bfd);
>  
>  #endif
> diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
> index 0416faa17c3..1331c5d5f56 100644
> --- a/gdb/solib-svr4.c
> +++ b/gdb/solib-svr4.c
> @@ -2335,7 +2335,7 @@ enable_break (struct svr4_info *info, int from_tty)
>        /* Now convert the TMP_BFD into a target.  That way target, as
>  	 well as BFD operations can be used.  target_bfd_reopen
>  	 acquires its own reference.  */
> -      tmp_bfd_target = target_bfd_reopen (tmp_bfd.get ());
> +      tmp_bfd_target = target_bfd_reopen (tmp_bfd);

Isn't the comment immediate above this about target_bfd_reopen
acquiring its own reference now out of date?

Otherwise, LGTM.

thanks,
Andrew

>  
>        /* On a running target, we can get the dynamic linker's base
>  	 address from the shared library table.  */
> -- 
> 2.26.2
> 

Re: [PATCH] Change target_bfd_reopen to take a gdb_bfd_ref_ptr

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

>> /* Now convert the TMP_BFD into a target.  That way target, as
>> well as BFD operations can be used.  target_bfd_reopen
>> acquires its own reference.  */
>> -      tmp_bfd_target = target_bfd_reopen (tmp_bfd.get ());
>> +      tmp_bfd_target = target_bfd_reopen (tmp_bfd);

Andrew> Isn't the comment immediate above this about target_bfd_reopen
Andrew> acquiring its own reference now out of date?

Yeah.  I'll update it and then check this in.

Tom