public inbox for gdb-testers@sourceware.org
help / color / mirror / Atom feed
From: gdb-buildbot@sergiodj.net
To: gdb-testers@sourceware.org
Subject: [binutils-gdb] Use std::sort instead of qsort in minsyms.c
Date: Tue, 01 Oct 2019 11:52:00 -0000	[thread overview]
Message-ID: <6fb08628e0f894f01e3e4834d5262629ca8f1920@gdb-build> (raw)

*** TEST RESULTS FOR COMMIT 6fb08628e0f894f01e3e4834d5262629ca8f1920 ***

commit 6fb08628e0f894f01e3e4834d5262629ca8f1920
Author:     Christian Biesinger <cbiesinger@google.com>
AuthorDate: Sat Sep 28 19:45:20 2019 -0500
Commit:     Christian Biesinger <cbiesinger@google.com>
CommitDate: Mon Sep 30 13:32:32 2019 -0500

    Use std::sort instead of qsort in minsyms.c
    
    This has better typesafety and is also marginally faster (either
    due to inlining or because it avoids indirection through a
    function pointer).
    
    Note that in this change:
    -       return 1;               /* fn1 has no name, so it is "less".  */
    +       return true;            /* fn1 has no name, so it is "less".  */
           else if (name1)          /* fn2 has no name, so it is "less".  */
    -       return -1;
    +       return false;
    I am fairly sure the old code was wrong (ie. code didn't match the
    comment and the comment seemed correct), so I fixed it.
    
    gdb/ChangeLog:
    
    2019-09-28  Christian Biesinger  <cbiesinger@google.com>
    
            * minsyms.c (compare_minimal_symbols): Rename to...
            (minimal_symbol_is_less_than): ...this, and adjust to STL
            conventions (return bool, take arguments as references)
            (minimal_symbol_reader::install): Call std::sort instead
            of qsort.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index faac59d255..3a5dff17b2 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2019-09-28  Christian Biesinger  <cbiesinger@google.com>
+
+	* minsyms.c (compare_minimal_symbols): Rename to...
+	(minimal_symbol_is_less_than): ...this, and adjust to STL
+	conventions (return bool, take arguments as references)
+	(minimal_symbol_reader::install): Call std::sort instead
+	of qsort.
+
 2019-09-29  Christian Biesinger  <cbiesinger@google.com>
 
 	* minsyms.h (msymbol_hash): Document that this is a case-insensitive
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index f06de4d88e..83f5d89577 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -1124,41 +1124,36 @@ minimal_symbol_reader::record_full (const char *name, int name_len,
   return msymbol;
 }
 
-/* Compare two minimal symbols by address and return a signed result based
-   on unsigned comparisons, so that we sort into unsigned numeric order.
+/* Compare two minimal symbols by address and return true if FN1's address
+   is less than FN2's, so that we sort into unsigned numeric order.
    Within groups with the same address, sort by name.  */
 
-static int
-compare_minimal_symbols (const void *fn1p, const void *fn2p)
+static inline bool
+minimal_symbol_is_less_than (const minimal_symbol &fn1,
+			     const minimal_symbol &fn2)
 {
-  const struct minimal_symbol *fn1;
-  const struct minimal_symbol *fn2;
-
-  fn1 = (const struct minimal_symbol *) fn1p;
-  fn2 = (const struct minimal_symbol *) fn2p;
-
-  if (MSYMBOL_VALUE_RAW_ADDRESS (fn1) < MSYMBOL_VALUE_RAW_ADDRESS (fn2))
+  if (MSYMBOL_VALUE_RAW_ADDRESS (&fn1) < MSYMBOL_VALUE_RAW_ADDRESS (&fn2))
     {
-      return (-1);		/* addr 1 is less than addr 2.  */
+      return true;		/* addr 1 is less than addr 2.  */
     }
-  else if (MSYMBOL_VALUE_RAW_ADDRESS (fn1) > MSYMBOL_VALUE_RAW_ADDRESS (fn2))
+  else if (MSYMBOL_VALUE_RAW_ADDRESS (&fn1) > MSYMBOL_VALUE_RAW_ADDRESS (&fn2))
     {
-      return (1);		/* addr 1 is greater than addr 2.  */
+      return false;		/* addr 1 is greater than addr 2.  */
     }
   else
     /* addrs are equal: sort by name */
     {
-      const char *name1 = MSYMBOL_LINKAGE_NAME (fn1);
-      const char *name2 = MSYMBOL_LINKAGE_NAME (fn2);
+      const char *name1 = MSYMBOL_LINKAGE_NAME (&fn1);
+      const char *name2 = MSYMBOL_LINKAGE_NAME (&fn2);
 
       if (name1 && name2)	/* both have names */
-	return strcmp (name1, name2);
+	return strcmp (name1, name2) < 0;
       else if (name2)
-	return 1;		/* fn1 has no name, so it is "less".  */
+	return true;		/* fn1 has no name, so it is "less".  */
       else if (name1)		/* fn2 has no name, so it is "less".  */
-	return -1;
+	return false;
       else
-	return (0);		/* Neither has a name, so they're equal.  */
+	return false;		/* Neither has a name, so they're equal.  */
     }
 }
 
@@ -1315,8 +1310,7 @@ minimal_symbol_reader::install ()
 
       /* Sort the minimal symbols by address.  */
 
-      qsort (msymbols, mcount, sizeof (struct minimal_symbol),
-	     compare_minimal_symbols);
+      std::sort (msymbols, msymbols + mcount, minimal_symbol_is_less_than);
 
       /* Compact out any duplicates, and free up whatever space we are
          no longer using.  */


             reply	other threads:[~2019-10-01 11:52 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-01 11:52 gdb-buildbot [this message]
2019-10-01 11:52 ` Failures on Ubuntu-Aarch64-m64, branch master gdb-buildbot
2019-10-01 14:08 ` Failures on Ubuntu-Aarch64-native-gdbserver-m64, " gdb-buildbot
2019-10-04 12:12 ` *** COMPILATION FAILED *** Failures on Fedora-i686, branch master *** BREAKAGE *** gdb-buildbot
2019-10-04 12:16 ` *** COMPILATION FAILED *** Failures on Fedora-x86_64-cc-with-index, " gdb-buildbot
2019-10-04 12:40 ` *** COMPILATION FAILED *** Failures on Fedora-x86_64-m64, " gdb-buildbot
2019-10-04 12:40 ` *** COMPILATION FAILED *** Failures on Fedora-x86_64-m32, " gdb-buildbot
2019-10-04 12:44 ` *** COMPILATION FAILED *** Failures on Fedora-x86_64-native-extended-gdbserver-m32, " gdb-buildbot
2019-10-04 13:11 ` *** COMPILATION FAILED *** Failures on Fedora-x86_64-native-gdbserver-m32, " gdb-buildbot
2019-10-04 13:11 ` *** COMPILATION FAILED *** Failures on Fedora-x86_64-w64-mingw32, " gdb-buildbot
2019-10-04 16:21 ` Failures on Fedora-x86_64-native-gdbserver-m64, branch master gdb-buildbot

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=6fb08628e0f894f01e3e4834d5262629ca8f1920@gdb-build \
    --to=gdb-buildbot@sergiodj.net \
    --cc=gdb-testers@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).