This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 01/10] change gdb to refcount bfd everywhere
>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
Jan> The comments are present in both and neither is a reference, they
Jan> are already out of sync.
I don't really follow, but I also don't mind changing the comments --
they ought to be clear for everybody, not just me. How about this?
/* Increment the reference count of ABFD. It is fine for ABFD to be
NULL; in this case the function does nothing. */
void gdb_bfd_ref (struct bfd *abfd);
/* Decrement the reference count of ABFD. If this is the last
reference, ABFD will be freed. If ABFD is NULL, this function does
nothing. */
void gdb_bfd_unref (struct bfd *abfd);
Jan> I personally disagree about the returned value from gdb_bfd_ref
Jan> being useful, it makes the code more magic IMO (plus it is not much
Jan> compatible with the narrow GNU Coding Standard code formatting).
I made this change. I'll test the patch and send it tomorrow.
Then I went a bit further and wrote a follow-up patch that wraps the
various BFD-opening functions with gdb equivalents. These initialize
the refcount and stash the filename.
E.g.:
/* A wrapper for bfd_fopen that initializes the gdb-specific reference
count and calls gdb_bfd_stash_filename. */
bfd *gdb_bfd_fopen (const char *, const char *, const char *, int);
I'll send this patch separately tomorrow. I'm curious to know what you
think. I find it cleans up the code quite a bit, I wish I'd done this
from the beginning.
>> @@ -193,9 +194,9 @@ allocate_objfile (bfd *abfd, int flags)
[...]
>> - objfile->obfd = gdb_bfd_ref (abfd);
>> + objfile->obfd = abfd;
Jan> Caller could gdb_bfd_unref its reference but YMMV.
I will look into this tomorrow.
Jan> This all pain would not exist with C++.
Yeah, well...
>> @@ -2519,14 +2512,10 @@ reread_symbols (void)
>> to close the descriptor but BFD lacks a way of closing the
>> BFD without closing the descriptor. */
>> obfd_filename = bfd_get_filename (objfile->obfd);
>> - if (!bfd_close (objfile->obfd))
>> - error (_("Can't close BFD for %s: %s"), objfile->name,
>> - bfd_errmsg (bfd_get_error ()));
>> + gdb_bfd_unref (objfile->obfd);
>> objfile-> obfd = bfd_open_maybe_remote (obfd_filename);
>> if (objfile->obfd == NULL)
>> error (_("Can't open %s to read symbols."), objfile->name);
>> - else
>> - objfile->obfd = gdb_bfd_ref (objfile->obfd);
Jan> Why isn't gdb_bfd_ref missing here?
bfd_open_maybe_remote returns a new reference.
I wonder if I should rename it to gdb_bfd_open_maybe_remote, for
consistency. What do you think? I'm inclined to do it.
Tom