Re: [RFA 2/8] Use class to manage BFD reference counts

[Pedro Alves <palves at redhat dot com>]

> @@ -10535,7 +10526,7 @@ open_dwo_file (const char *file_name, const char *comp_dir)
>       is a list of paths.  */
>  
>    if (*debug_file_directory == '\0')
> -    return NULL;
> +    return gdb_bfd_ref_ptr ();

This provides a good reason to have an implicit construction from nullptr_t.
You had it in the original gdbpy_reference submission, but I had asked
to remove it.  If we add it back, these cases could be more clearly
written as "return NULL/nullptr".  Could you do that, and then drop all
the hunks like:

 > -    return NULL;
 > +    return gdb_bfd_ref_ptr ();

?

> @@ -658,82 +654,70 @@ solib_aix_bfd_open (char *pathname)
>      }
>    filename_len = sep - pathname;
>  
> -  filename = xstrprintf ("%.*s", filename_len, pathname);
> -  cleanup = make_cleanup (xfree, filename);
> -  member_name = xstrprintf ("%.*s", path_len - filename_len - 2, sep + 1);
> -  make_cleanup (xfree, member_name);
> +  gdb::unique_xmalloc_ptr<char> filename
> +    (xstrprintf ("%.*s", filename_len, pathname));
> +  gdb::unique_xmalloc_ptr<char> member_name
> +    (xstrprintf ("%.*s", path_len - filename_len - 2, sep + 1));

I think these could be:

    std::string filename
      = string_printf ("%.*s", filename_len, pathname);
    std::string member_name
      = string_printf ("%.*s", path_len - filename_len - 2, sep + 1));

>  
>    /* Calling solib_find makes certain that sysroot path is set properly
>       if program has a dependency on .a archive and sysroot is set via
>       set sysroot command.  */
> -  found_pathname = solib_find (filename, &found_file);
> +  found_pathname = solib_find (filename.get (), &found_file);
>    if (found_pathname == NULL)
>        perror_with_name (pathname);
> -  archive_bfd = solib_bfd_fopen (found_pathname, found_file);
> +  gdb_bfd_ref_ptr archive_bfd (solib_bfd_fopen (found_pathname, found_file));
>    if (archive_bfd == NULL)
>      {
>        warning (_("Could not open `%s' as an executable file: %s"),
> -	       filename, bfd_errmsg (bfd_get_error ()));
> -      do_cleanups (cleanup);
> -      return NULL;
> +	       filename.get (), bfd_errmsg (bfd_get_error ()));
> +      return gdb_bfd_ref_ptr ();
>      }
>  
> -  if (bfd_check_format (archive_bfd, bfd_object))
> -    {
> -      do_cleanups (cleanup);
> -      return archive_bfd;
> -    }
> +  if (bfd_check_format (archive_bfd.get (), bfd_object))
> +    return archive_bfd;
>  
> -  if (! bfd_check_format (archive_bfd, bfd_archive))
> +  if (! bfd_check_format (archive_bfd.get (), bfd_archive))
>      {
>        warning (_("\"%s\": not in executable format: %s."),
> -	       filename, bfd_errmsg (bfd_get_error ()));
> -      gdb_bfd_unref (archive_bfd);
> -      do_cleanups (cleanup);
> -      return NULL;
> +	       filename.get (), bfd_errmsg (bfd_get_error ()));
> +      return gdb_bfd_ref_ptr ();
>      }
>  
> -  object_bfd = gdb_bfd_openr_next_archived_file (archive_bfd, NULL);
> +  gdb_bfd_ref_ptr object_bfd
> +    (gdb_bfd_openr_next_archived_file (archive_bfd.get (), NULL));
>    while (object_bfd != NULL)
>      {
> -      bfd *next;
> -
> -      if (strcmp (member_name, object_bfd->filename) == 0)
> +      if (strcmp (member_name.get (), object_bfd->filename) == 0)
>  	break;

Then here:

    member_name == object_bfd->filename

>  
> -      next = gdb_bfd_openr_next_archived_file (archive_bfd, object_bfd);
> -      gdb_bfd_unref (object_bfd);
> -      object_bfd = next;
> +      object_bfd = gdb_bfd_openr_next_archived_file (archive_bfd.get (),
> +						     object_bfd.get ());
>      }
>  

Otherwise LGTM.

Thanks,
Pedro Alves