From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14916 invoked by alias); 5 Jun 2017 08:43:12 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 14893 invoked by uid 89); 5 Jun 2017 08:43:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_SOFTFAIL autolearn=no version=3.3.2 spammy= X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 05 Jun 2017 08:43:09 +0000 Received: by simark.ca (Postfix, from userid 33) id 884FC1E5A3; Mon, 5 Jun 2017 04:43:11 -0400 (EDT) To: Tom Tromey Subject: Re: [RFA 1/2] C++-ify break-catch-sig X-PHP-Originating-Script: 33:rcube.php MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Mon, 05 Jun 2017 08:43:00 -0000 From: Simon Marchi Cc: gdb-patches@sourceware.org In-Reply-To: <20170604225353.18008-2-tom@tromey.com> References: <20170604225353.18008-1-tom@tromey.com> <20170604225353.18008-2-tom@tromey.com> Message-ID: <4bbbfc1a66bb3d2432cdac491a677053@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.2.5 X-IsSubscribed: yes X-SW-Source: 2017-06/txt/msg00100.txt.bz2 On 2017-06-05 00:53, Tom Tromey wrote: > This changes signal_catchpoint to be more of a C++ class, using > std::vector and updating the users. > > gdb/ChangeLog > 2017-06-04 Tom Tromey > > * break-catch-sig.c (gdb_signal_type): Remove typedef. > (struct signal_catchpoint) : Now a > std::vector. > (~signal_catchpoint, signal_catchpoint_insert_location) > (signal_catchpoint_remove_location) > (signal_catchpoint_breakpoint_hit, signal_catchpoint_print_one) > (signal_catchpoint_print_mention) > (signal_catchpoint_print_recreate) > (signal_catchpoint_explains_signal): Update. > (create_signal_catchpoint): Change type of "filter". > (catch_signal_split_args): Return a std::vector. > (catch_signal_command): Update. > --- Hi Tom, Thanks for doing this. The patch looks good in general. I'd suggest doing these in the same patch: - make the catch_all field/parameter/variable a bool, - remove the destructor, since it's now empty. > @@ -99,20 +94,17 @@ static int > signal_catchpoint_insert_location (struct bp_location *bl) > { > struct signal_catchpoint *c = (struct signal_catchpoint *) > bl->owner; > - int i; > > - if (c->signals_to_be_caught != NULL) > + if (!c->signals_to_be_caught.empty ()) > { > - gdb_signal_type iter; > + gdb_signal iter; You don't need this declaration... > - for (i = 0; > - VEC_iterate (gdb_signal_type, c->signals_to_be_caught, i, iter); > - i++) > + for (auto iter : c->signals_to_be_caught) ...because it's overriden by this one. Although I'd prefer if you used "gdb_signal" instead of "auto" here. It's not much longer, is more expressive. There are a few instances of this throughout the patch. > @@ -246,26 +231,25 @@ signal_catchpoint_print_one (struct breakpoint > *b, > uiout->field_skip ("addr"); > annotate_field (5); > > - if (c->signals_to_be_caught > - && VEC_length (gdb_signal_type, c->signals_to_be_caught) > 1) > + if (!c->signals_to_be_caught.empty ()) I think you changed the behavior of this condition. It should probably be: c->signals_to_be_caught.size () > 1 With those comments addressed, the patch is good to me. Thanks! Simon