This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA 2/8] Use class to manage BFD reference counts
- From: Pedro Alves <palves at redhat dot com>
- To: Tom Tromey <tom at tromey dot com>, gdb-patches at sourceware dot org
- Date: Fri, 2 Dec 2016 13:05:20 +0000
- Subject: Re: [RFA 2/8] Use class to manage BFD reference counts
- Authentication-results: sourceware.org; auth=none
- References: <1480395946-10924-1-git-send-email-tom@tromey.com> <1480395946-10924-3-git-send-email-tom@tromey.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