Re: [PATCH 1/7] Introduce type-safe variant of gdb_bfd_openr_iovec

[Lancelot SIX <lsix@lancelotsix.com>]

Hi Tom,

On Thu, Aug 24, 2023 at 11:12:18AM -0600, Tom Tromey via Gdb-patches wrote:
> This patch adds a new, type-safe variant of gdb_bfd_openr_iovec.  In
> this approach, the underlying user data is simply an object, the
> callbacks are methods, and the "open" function is a function view.
> Nothing uses this new code yet.
> ---
>  gdb/gdb_bfd.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>  gdb/gdb_bfd.h | 31 +++++++++++++++++++++++++++++++
>  2 files changed, 73 insertions(+)
> 
> diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
> index 3765561cbe9..0bb97460898 100644
> --- a/gdb/gdb_bfd.c
> +++ b/gdb/gdb_bfd.c
> @@ -907,6 +907,48 @@ gdb_bfd_openw (const char *filename, const char *target)
>  
>  /* See gdb_bfd.h.  */
>  
> +gdb_bfd_ref_ptr
> +gdb_bfd_openr_iovec (const char *filename, const char *target,
> +		     gdb_iovec_opener_ftype open_fn)
> +{
> +  auto do_open = [] (bfd *nbfd, void *closure) -> void *
> +  {
> +    auto real_opener = (gdb_iovec_opener_ftype *) closure;

I think I would have a personal preference toward using static_cast
here, but in this case the C-style cast and static cast are equivalent.

Other than this remark, this looks good to me.
Reviewed-By: Lancelot Six <lancelot.six@amd.com>

Best,
Lancelot.

> +    return (*real_opener) (nbfd);
> +  };
> +
> +  auto read_trampoline = [] (bfd *nbfd, void *stream, void *buf,
> +			     file_ptr nbytes, file_ptr offset) -> file_ptr
> +  {
> +    gdb_bfd_iovec_base *obj = (gdb_bfd_iovec_base *) stream;
> +    return obj->read (nbfd, buf, nbytes, offset);
> +  };
> +
> +  auto stat_trampoline = [] (struct bfd *abfd, void *stream,
> +			     struct stat *sb) -> int
> +  {
> +    gdb_bfd_iovec_base *obj = (gdb_bfd_iovec_base *) stream;
> +    return obj->stat (abfd, sb);
> +  };
> +
> +  auto close_trampoline = [] (struct bfd *nbfd, void *stream) -> int
> +  {
> +    gdb_bfd_iovec_base *obj = (gdb_bfd_iovec_base *) stream;
> +    delete obj;
> +    /* Success.  */
> +    return 0;
> +  };
> +
> +  bfd *result = bfd_openr_iovec (filename, target,
> +				 do_open, &open_fn,
> +				 read_trampoline, close_trampoline,
> +				 stat_trampoline);
> +
> +  return gdb_bfd_ref_ptr::new_reference (result);
> +}
> +
> +/* See gdb_bfd.h.  */
> +
>  gdb_bfd_ref_ptr
>  gdb_bfd_openr_iovec (const char *filename, const char *target,
>  		     void *(*open_func) (struct bfd *nbfd,
> diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h
> index d15b1106d9a..ae374f5d7ae 100644
> --- a/gdb/gdb_bfd.h
> +++ b/gdb/gdb_bfd.h
> @@ -22,6 +22,7 @@
>  
>  #include "registry.h"
>  #include "gdbsupport/byte-vector.h"
> +#include "gdbsupport/function-view.h"
>  #include "gdbsupport/gdb_ref_ptr.h"
>  #include "gdbsupport/iterator-range.h"
>  #include "gdbsupport/next-iterator.h"
> @@ -150,6 +151,36 @@ gdb_bfd_ref_ptr gdb_bfd_openr (const char *, const char *);
>  
>  gdb_bfd_ref_ptr gdb_bfd_openw (const char *, const char *);
>  
> +/* The base class for BFD "iovec" implementations.  This is used by
> +   gdb_bfd_openr_iovec and enables better type safety.  */
> +
> +class gdb_bfd_iovec_base
> +{
> +protected:
> +
> +  gdb_bfd_iovec_base () = default;
> +
> +public:
> +
> +  virtual ~gdb_bfd_iovec_base () = default;
> +
> +  /* The "read" callback.  */
> +  virtual file_ptr read (bfd *abfd, void *buffer, file_ptr nbytes,
> +			 file_ptr offset) = 0;
> +
> +  /* The "stat" callback.  */
> +  virtual int stat (struct bfd *abfd, struct stat *sb) = 0;
> +};
> +
> +/* The type of the function used to open a new iovec-based BFD.  */
> +using gdb_iovec_opener_ftype
> +     = gdb::function_view<gdb_bfd_iovec_base * (bfd *)>;
> +
> +/* A type-safe wrapper for bfd_openr_iovec.  */
> +
> +gdb_bfd_ref_ptr gdb_bfd_openr_iovec (const char *filename, const char *target,
> +				     gdb_iovec_opener_ftype open_fn);
> +
>  /* A wrapper for bfd_openr_iovec that initializes the gdb-specific
>     reference count.  */
>  
> 
> -- 
> 2.40.1
>