Re: [RFA] Use FILENAME_CMP to compare filenames in compare_search_syms.

[Joel Brobecker <brobecker at adacore dot com>]

> I'd say that use of FILENAME_CMP instead of strcmp is an obvious change.
> 
> However, this is not complete.  This function is called by:
[...]
> But really, so that this sort of out-of-sync bugs doesn't
> happen, it'd be better if search_symbols_equal were reimplemented
> in terms of compare_search_syms or even be eliminated.

Indeed! I'm turning this fix into a 2-patch series:

  1. Delete search_symbols_equal

     This patch does not fix anything, other than removing the duplication.

     I pondered a bit over that change, thinking maybe we'd want
     to keep it, just because the arguments have the right type
     instead of "void *". In the end, I felt it was sufficiently
     localized that deleting was OK. Putting it back is easy,
     though, so whichever we want it shall be.

  2. The patch I initially proposed.

     This patch implements the strcmp -> FILENAME_CMP fix.

gdb/ChangeLog:

        * symtab.c (search_symbols_equal): Delete.
        (sort_search_symbols_remove_dups): Replace call to
        search_symbols_equal by call to compare_search_syms,
        adjusting as necessary.

gdb/ChangeLog:

        * symtab.c (compare_search_syms): Use FILENAME_CMP instead of
        strcmp to compare two symtab filenames.

Tested on x86_64-linux, no regression. OK to apply?

Thanks,
-- 
Joel

>From 56b13bd393e3e1a205888af374a236aae668140a Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Tue, 1 Oct 2013 14:15:46 +0200
Subject: [PATCH 1/2] Delete search_symbols_equal (use compare_search_syms
 instead).

This avoids duplicating the logic comparing two symbol_search
objects (in search_symbols_equal and compare_search_syms).

gdb/ChangeLog:

        * symtab.c (search_symbols_equal): Delete.
        (sort_search_symbols_remove_dups): Replace call to
        search_symbols_equal by call to compare_search_syms,
        adjusting as necessary.
---
 gdb/symtab.c | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/gdb/symtab.c b/gdb/symtab.c
index dbff042..cf03b43 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -3347,19 +3347,6 @@ compare_search_syms (const void *sa, const void *sb)
 		 SYMBOL_PRINT_NAME (sym_b->symbol));
 }
 
-/* Helper function for sort_search_symbols_remove_dups.
-   Return TRUE if symbols A, B are equal.  */
-
-static int
-search_symbols_equal (const struct symbol_search *a,
-		      const struct symbol_search *b)
-{
-  return (strcmp (a->symtab->filename, b->symtab->filename) == 0
-	  && a->block == b->block
-	  && strcmp (SYMBOL_PRINT_NAME (a->symbol),
-		     SYMBOL_PRINT_NAME (b->symbol)) == 0);
-}
-
 /* Sort the NFOUND symbols in list FOUND and remove duplicates.
    The duplicates are freed, and the new list is returned in
    *NEW_HEAD, *NEW_TAIL.  */
@@ -3393,7 +3380,7 @@ sort_search_symbols_remove_dups (struct symbol_search *found, int nfound,
   /* Collapse out the dups.  */
   for (i = 1, j = 1; i < nfound; ++i)
     {
-      if (! search_symbols_equal (symbols[j - 1], symbols[i]))
+      if (compare_search_syms (&symbols[j - 1], &symbols[i]) != 0)
 	symbols[j++] = symbols[i];
       else
 	xfree (symbols[i]);
-- 
1.8.1.2

>From a191522c5b61288616ef91780f4e89edbf4d33fb Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Thu, 12 Sep 2013 14:03:47 -0400
Subject: [PATCH 2/2] Use FILENAME_CMP to compare filenames in
 compare_search_syms.

gdb/ChangeLog:

        * symtab.c (compare_search_syms): Use FILENAME_CMP instead of
        strcmp to compare two symtab filenames.
---
 gdb/symtab.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/symtab.c b/gdb/symtab.c
index cf03b43..d3622e5 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -3336,7 +3336,7 @@ compare_search_syms (const void *sa, const void *sb)
   struct symbol_search *sym_b = *(struct symbol_search **) sb;
   int c;
 
-  c = strcmp (sym_a->symtab->filename, sym_b->symtab->filename);
+  c = FILENAME_CMP (sym_a->symtab->filename, sym_b->symtab->filename);
   if (c != 0)
     return c;
 
-- 
1.8.1.2