public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Andrew Burgess <aburgess@redhat.com>
To: gdb-patches@sourceware.org
Cc: Andrew Burgess <aburgess@redhat.com>
Subject: [PATCH] gdb: small cleanup in symbol_file_add_with_addrs
Date: Thu, 14 Sep 2023 16:49:29 +0100	[thread overview]
Message-ID: <6219a1ae148c0b9796f7153b9a6c3b7173fb8f5a.1694706545.git.aburgess@redhat.com> (raw)

While looking at how gdb::observers::new_objfile was used, I found
some code in symbol_file_add_with_addrs that I thought could be
improved.

Instead of:

  if (condition)
   {
     ...
     return;
   }

  ...
  return;

Where some parts of '...' identical between the two branches.  I think
it would be nicer if the duplication is removed, and we just use:

  if (!condition)
    ...

to guard the one statement that should only happen when the condition
is not true.

There is one change in this commit though that is (possibly)
significant, there is a call to bfd_cache_close_all() that was only
present in the second block.  After this commit we now call that
function for both paths.

The call to bfd_cache_close_all was added in commit:

  commit ce7d45220e4ed342d4a77fcd2f312e85e1100971
  Date:   Fri Jul 30 12:05:45 2004 +0000

with the purpose of ensuring that GDB doesn't hold the BFDs open
unnecessarily, thus preventing the files from being updated on some
hosts (e.g. Win32).

In the early exit case we previously didn't call bfd_cache_close_all,
with the result that GDB would continue to hold open some BFD objects
longer than needed.

After this commit, but calling bfd_cache_close_all for both paths this
problem is solved.

I'm not sure how this change could be tested, I don't believe there's
any GDB (maintenance) command that displays the BFD cache contents, so
we can't check the cache contents easily.  Ideas are welcome though.
---
 gdb/symfile.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/gdb/symfile.c b/gdb/symfile.c
index 1b46ec45f2e..c20c0c50b9a 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1119,18 +1119,13 @@ symbol_file_add_with_addrs (const gdb_bfd_ref_ptr &abfd, const char *name,
      time.  */
   gdb_flush (gdb_stdout);
 
-  if (objfile->sf == NULL)
-    {
-      gdb::observers::new_objfile.notify (objfile);
-      return objfile;	/* No symbols.  */
-    }
-
-  finish_new_objfile (objfile, add_flags);
+  if (objfile->sf != nullptr)
+    finish_new_objfile (objfile, add_flags);
 
   gdb::observers::new_objfile.notify (objfile);
 
   bfd_cache_close_all ();
-  return (objfile);
+  return objfile;
 }
 
 /* Add BFD as a separate debug file for OBJFILE.  For NAME description

base-commit: d7680f13df105aa8fa0edbdf8efae31a5411f579
-- 
2.25.4


             reply	other threads:[~2023-09-14 15:49 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-14 15:49 Andrew Burgess [this message]
2023-09-14 16:17 ` Tom Tromey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6219a1ae148c0b9796f7153b9a6c3b7173fb8f5a.1694706545.git.aburgess@redhat.com \
    --to=aburgess@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).