From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23858 invoked by alias); 23 Aug 2011 23:24:45 -0000 Received: (qmail 23660 invoked by uid 22791); 23 Aug 2011 23:24:43 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from elasmtp-mealy.atl.sa.earthlink.net (HELO elasmtp-mealy.atl.sa.earthlink.net) (209.86.89.69) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 23 Aug 2011 23:24:30 +0000 Received: from [70.170.59.51] (helo=macbook2.local) by elasmtp-mealy.atl.sa.earthlink.net with esmtpa (Exim 4.67) (envelope-from ) id 1Qw0Kb-0001C9-Kn; Tue, 23 Aug 2011 19:24:29 -0400 Message-ID: <4E54369C.9000706@earthlink.net> Date: Tue, 23 Aug 2011 23:24:00 -0000 From: Stan Shebs User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:6.0) Gecko/20110812 Thunderbird/6.0 MIME-Version: 1.0 To: Hui Zhu CC: gdb-patches@sourceware.org Subject: Re: [PATCH]tracepoint.c: Add conditionals num to find_matching_tracepoint References: <4E406AE9.100@earthlink.net> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-ELNK-Trace: ae6f8838ff913eba0cc1426638a40ef67e972de0d01da94050bded970d7450d54a0d9f7674c6b65d350badd9bab72f9c350badd9bab72f9c350badd9bab72f9c X-IsSubscribed: yes 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 X-SW-Source: 2011-08/txt/msg00430.txt.bz2 On 8/14/11 6:40 AM, Hui Zhu wrote: > [...] > Hi Stan, > > Thanks for your review. > > I make a new patch that check the condition according to your mail. > > Best, > Hui > > 2011-08-14 Hui Zhu > > * tracepoint.c (cond_string_is_same): New function. > (find_matching_tracepoint): Add condition check > by cond_string_is_same. > --- > tracepoint.c | 19 ++++++++++++++++++- > 1 file changed, 18 insertions(+), 1 deletion(-) > > --- a/tracepoint.c > +++ b/tracepoint.c > @@ -3091,6 +3091,22 @@ free_uploaded_tsvs (struct uploaded_tsv > } > } > > +static int > +cond_string_is_same(char *str1, char *str2) Don't forget the all-important space! :-) > +{ > + if (str1 == NULL || str2 == NULL) > + { > + if (str1 == str2) > + return 1; > + else > + return 0; This bit is a little simpler as just "return (str1 == str2);" > + } > + if (strcmp (str1, str2)) > + return 0; > + > + return 1; Typically, I would write this as "return (strcmp (str1, str2) == 0);" It would also be good to add a function header comment that this function is heuristic and will miss the cases where the conditional is semantically identical but differs in whitespace, such as "x == 0" vs "x==0" - but that's generally OK, because it just results in an extra tracepoint that is easily deleted. OK to install with these changes, and thanks! Stan stan@codesourcery.com > +} > + > /* Look for an existing tracepoint that seems similar enough to the > uploaded one. Enablement isn't compared, because the user can > toggle that freely, and may have done so in anticipation of the > @@ -3111,7 +3127,8 @@ find_matching_tracepoint (struct uploade > if (b->type == utp->type > && t->step_count == utp->step > && t->pass_count == utp->pass > - /* FIXME also test conditionals and actions. */ > + && cond_string_is_same (t->base.cond_string, utp->cond_string) > + /* FIXME also test actions. */ > ) > { > /* Scan the locations for an address match. */ >