This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA 10/11] Use a std::vector for ada_exceptions_list
- From: Pedro Alves <palves at redhat dot com>
- To: Tom Tromey <tom at tromey dot com>, gdb-patches at sourceware dot org
- Date: Thu, 28 Sep 2017 11:20:41 +0100
- Subject: Re: [RFA 10/11] Use a std::vector for ada_exceptions_list
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=palves at redhat dot com
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com C89402CE1EC
- References: <20170912185736.20436-1-tom@tromey.com> <20170912185736.20436-11-tom@tromey.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