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

[Pedro Alves <palves at redhat dot com>]

On 10/01/2013 10:07 AM, Joel Brobecker wrote:
> Hello,
> 
> While working on an unrelated issue, I noticed that two symbols were
> sorted differently on Windows, compared to the other Unix platforms.
> I tracked it down to compare_search_syms which compares filenames
> using a plain strcmp instead of FILENAME_CMP.
> 
> Not sure how to create a testcase...
> 
> gdb/ChangeLog:
> 
>         * symtab.c (compare_search_syms): Use FILENAME_CMP instead of
>         strcmp to compare two symtab filenames.
> 
> Tested on x86-windows and x86_64-linux, no regression.
> OK to apply?

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:

/* 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.  */

static void
sort_search_symbols_remove_dups (struct symbol_search *found, int nfound,
				 struct symbol_search **new_head,
				 struct symbol_search **new_tail)
{
...
  qsort (symbols, nfound, sizeof (struct symbol_search *),
	 compare_search_syms);

So this is sorting symbols, to then walk over the sorted list
linearly and remove dups.  That happens right afterwards:

  /* Collapse out the dups.  */
  for (i = 1, j = 1; i < nfound; ++i)
    {
      if (! search_symbols_equal (symbols[j - 1], symbols[i]))
	symbols[j++] = symbols[i];
      else
	xfree (symbols[i]);
    }

So the compare_search_syms and search_symbols_equal predicates
_must_ agree.  And, lo, search_symbols_equal also has that strcmp
that would need adjustment as well:

 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);
 }


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.

-- 
Pedro Alves