Re: [RFA 10/11] Use a std::vector for ada_exceptions_list

[Pedro Alves <palves@redhat.com>]

On 09/12/2017 07:57 PM, Tom Tromey wrote:
> Change ada_exceptions_list to return a std::vector and fix up the
> users.  This allows removing a cleanup in MI.

Looks good to me with the nits below addressed.

> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index 64f1a33..cafba2d 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -62,6 +62,7 @@
>  #include "cli/cli-utils.h"
>  #include "common/function-view.h"
>  #include "common/byte-vector.h"
> +#include <algorithm>
>  
>  /* Define whether or not the C operator '/' truncates towards zero for
>     differently signed operands (truncation direction is undefined in C).
> @@ -13121,23 +13122,23 @@ ada_is_non_standard_exception_sym (struct symbol *sym)
>     The comparison is determined first by exception name, and then
>     by exception address.  */

This comment talks about qsort.  It should be updated to mention
std::sort instead, since the logic is different.

>  
> -static int
> -compare_ada_exception_info (const void *a, const void *b)
> +bool
> +ada_exc_info::operator< (const ada_exc_info &other)
>  {
> -  const struct ada_exc_info *exc_a = (struct ada_exc_info *) a;
> -  const struct ada_exc_info *exc_b = (struct ada_exc_info *) b;
>    int result;
>  
> -  result = strcmp (exc_a->name, exc_b->name);
> -  if (result != 0)
> -    return result;
> -
> -  if (exc_a->addr < exc_b->addr)
> -    return -1;
> -  if (exc_a->addr > exc_b->addr)
> -    return 1;
> +  result = strcmp (name, other.name);
> +  if (result < 0)
> +    return true;
> +  if (result == 0 && addr < other.addr)
> +    return true;
> +  return false;
> +}
>  
> -  return 0;
> +bool
> +ada_exc_info::operator== (const ada_exc_info &other)
> +{
> +  return strcmp (name, other.name) == 0 && addr == other.addr;

I'd swap the comparisons to put the cheap addr comparison first.

>    if (regexp != NULL)
>      printf_filtered
> @@ -13404,10 +13386,8 @@ info_exceptions_command (char *regexp, int from_tty)
>    else
>      printf_filtered (_("All defined Ada exceptions:\n"));
>  
> -  for (ix = 0; VEC_iterate(ada_exc_info, exceptions, ix, info); ix++)
> -    printf_filtered ("%s: %s\n", info->name, paddress (gdbarch, info->addr));
> -
> -  do_cleanups (cleanup);
> +  for (ada_exc_info &info : exceptions)
> +    printf_filtered ("%s: %s\n", info.name, paddress (gdbarch, info.addr));

I'd write 'const ada_exc_info &', just for general use-const-if-possible
reasons.

> -  for (ix = 0; VEC_iterate(ada_exc_info, exceptions, ix, info); ix++)
> +  for (ada_exc_info &info : exceptions)

Ditto.

Thanks,
Pedro Alves