From patchwork Mon Apr 8 19:17:00 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 32205 Received: (qmail 46315 invoked by alias); 8 Apr 2019 19:17:06 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 46303 invoked by uid 89); 8 Apr 2019 19:17:06 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy= X-HELO: mail-wr1-f65.google.com Received: from mail-wr1-f65.google.com (HELO mail-wr1-f65.google.com) (209.85.221.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 08 Apr 2019 19:17:04 +0000 Received: by mail-wr1-f65.google.com with SMTP id y7so2140820wrn.11 for ; Mon, 08 Apr 2019 12:17:04 -0700 (PDT) Return-Path: Received: from ?IPv6:2001:8a0:f913:f700:56ee:75ff:fe8d:232b? ([2001:8a0:f913:f700:56ee:75ff:fe8d:232b]) by smtp.gmail.com with ESMTPSA id o10sm45835322wru.54.2019.04.08.12.17.01 (version=TLS1_3 cipher=AEAD-AES128-GCM-SHA256 bits=128/128); Mon, 08 Apr 2019 12:17:01 -0700 (PDT) Subject: Re: [PATCH] Some gdb_exception{,error,quit} tweaks To: Tom Tromey Cc: gdb-patches@sourceware.org References: <20190408172425.31039-1-palves@redhat.com> <87bm1g75ab.fsf@tromey.com> From: Pedro Alves Message-ID: <6c65fd2b-b22e-b42e-d1c4-4d1f4201569b@redhat.com> Date: Mon, 8 Apr 2019 20:17:00 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <87bm1g75ab.fsf@tromey.com> On 4/8/19 6:53 PM, Tom Tromey wrote: >>>>>> "Pedro" == Pedro Alves writes: > > Thanks. This looks good to me. > > Pedro> + gdb_exception (enum return_reason r, enum errors e, > Pedro> + const char *fmt, va_list ap) > Pedro> + : reason (r), > Pedro> + error (e), > Pedro> + message (std::make_shared (string_vprintf (fmt, ap))) > > I think these constructors should probably have ATTRIBUTE_PRINTF with 0 > as the "first-to-check" parameter, like string_vprintf itself. Indeed. I've added those, fixed a few typos in the commit log, and pushed it, as below. From 56be6ea89cdf94078d5dff3734b8c1970dbf52fa Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Mon, 8 Apr 2019 13:03:54 +0100 Subject: [PATCH] Some gdb_exception{,error,quit} tweaks - Explicitly include for std::string. - Use std::make_shared to construct gdb_exception::message instead of operator new, avoiding one heap allocation (2 instead of 3). Add 'const char *fmt, va_list ap' parameters to gdb_exception{,error,quit}'s ctors, and do the std::make_shared in the gdb_exception ctor. - gdb_exception_error's constructor does not need to have an 'enum return_reason' parameter, since it is always RETURN_ERROR, by definition. - Similarly, gdb_exception_quit's contructor does not need to have 'enum return_reason'/'enum errors' parameters. - In the gdb_exception_{quit,_error} ctors that take a gdb_exception as argument, assert that they're being passed a gdb_exception object of the right 'reason'. gdb/ChangeLog: 2019-04-08 Pedro Alves * common/common-exceptions.c (throw_exception): Don't create named object to throw; throw directly. (throw_it): Likewise. Don't initialize gdb_exception::message here, with new; pass FMT and AP to the ctor instead. * common/common-exceptions.h: Include . (gdb_exception::gdb_exception(enum return_reason, enum errors, const char *, va_list)): New ctor. Use std::make_shared. (gdb_exception_error::gdb_exception_error(enum return_reason, enum errors)): Delete. (gdb_exception_error::gdb_exception_error(enum errors, const char *, va_list)): New. (gdb_exception_error::gdb_exception_error(const gdb_exception &)): Add assertion. (gdb_exception_quit::gdb_exception_quit(enum return_reason, enum errors)): Delete. (gdb_exception_quit::gdb_exception_quit(const char *, va_list)): New. (gdb_exception_quit::gdb_exception_quit(const gdb_exception &)): Add assertion. --- gdb/ChangeLog | 21 +++++++++++++++++++++ gdb/common/common-exceptions.c | 22 ++++------------------ gdb/common/common-exceptions.h | 22 ++++++++++++++++++---- 3 files changed, 43 insertions(+), 22 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 39c380815aa..9910952aa56 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,24 @@ +2019-04-08 Pedro Alves + + * common/common-exceptions.c (throw_exception): Don't create + named object to throw; throw directly. + (throw_it): Likewise. Don't initialize gdb_exception::message + here, with new; pass FMT and AP to the ctor instead. + * common/common-exceptions.h: Include . + (gdb_exception::gdb_exception(enum return_reason, enum errors, + const char *, va_list)): New ctor. Use std::make_shared. + (gdb_exception_error::gdb_exception_error(enum return_reason, enum + errors)): Delete. + (gdb_exception_error::gdb_exception_error(enum errors, const char + *, va_list)): New. + (gdb_exception_error::gdb_exception_error(const gdb_exception &)): + Add assertion. + (gdb_exception_quit::gdb_exception_quit(enum return_reason, enum + errors)): Delete. + (gdb_exception_quit::gdb_exception_quit(const char *, va_list)): New. + (gdb_exception_quit::gdb_exception_quit(const gdb_exception &)): + Add assertion. + 2019-04-08 Tom Tromey * valops.c (value_rtti_indirect_type): Replace throw_exception diff --git a/gdb/common/common-exceptions.c b/gdb/common/common-exceptions.c index 83f2c74bfd4..6378dc40d6d 100644 --- a/gdb/common/common-exceptions.c +++ b/gdb/common/common-exceptions.c @@ -184,15 +184,9 @@ void throw_exception (const gdb_exception &exception) { if (exception.reason == RETURN_QUIT) - { - gdb_exception_quit ex (exception); - throw ex; - } + throw gdb_exception_quit (exception); else if (exception.reason == RETURN_ERROR) - { - gdb_exception_error ex (exception); - throw ex; - } + throw gdb_exception_error (exception); else gdb_assert_not_reached ("invalid return reason"); } @@ -202,17 +196,9 @@ throw_it (enum return_reason reason, enum errors error, const char *fmt, va_list ap) { if (reason == RETURN_QUIT) - { - gdb_exception_quit ex (reason, error); - ex.message.reset (new std::string (string_vprintf (fmt, ap))); - throw ex; - } + throw gdb_exception_quit (fmt, ap); else if (reason == RETURN_ERROR) - { - gdb_exception_error ex (reason, error); - ex.message.reset (new std::string (string_vprintf (fmt, ap))); - throw ex; - } + throw gdb_exception_error (error, fmt, ap); else gdb_assert_not_reached ("invalid return reason"); } diff --git a/gdb/common/common-exceptions.h b/gdb/common/common-exceptions.h index 1aedb831a62..3f47caec775 100644 --- a/gdb/common/common-exceptions.h +++ b/gdb/common/common-exceptions.h @@ -23,6 +23,7 @@ #include #include #include +#include /* Reasons for calling throw_exceptions(). NOTE: all reason values must be different from zero. enum value 0 is reserved for internal @@ -123,6 +124,15 @@ struct gdb_exception { } + gdb_exception (enum return_reason r, enum errors e, + const char *fmt, va_list ap) + ATTRIBUTE_PRINTF (4, 0) + : reason (r), + error (e), + message (std::make_shared (string_vprintf (fmt, ap))) + { + } + /* The copy constructor exists so that we can mark it "noexcept", which is a good practice for any sort of exception object. */ gdb_exception (const gdb_exception &other) noexcept @@ -214,27 +224,31 @@ extern int exceptions_state_mc_catch (struct gdb_exception *, int); struct gdb_exception_error : public gdb_exception { - gdb_exception_error (enum return_reason r, enum errors e) - : gdb_exception (r, e) + gdb_exception_error (enum errors e, const char *fmt, va_list ap) + ATTRIBUTE_PRINTF (3, 0) + : gdb_exception (RETURN_ERROR, e, fmt, ap) { } explicit gdb_exception_error (const gdb_exception &ex) noexcept : gdb_exception (ex) { + gdb_assert (ex.reason == RETURN_ERROR); } }; struct gdb_exception_quit : public gdb_exception { - gdb_exception_quit (enum return_reason r, enum errors e) - : gdb_exception (r, e) + gdb_exception_quit (const char *fmt, va_list ap) + ATTRIBUTE_PRINTF (2, 0) + : gdb_exception (RETURN_QUIT, GDB_NO_ERROR, fmt, ap) { } explicit gdb_exception_quit (const gdb_exception &ex) noexcept : gdb_exception (ex) { + gdb_assert (ex.reason == RETURN_QUIT); } };