From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 45794 invoked by alias); 10 May 2017 02:45:55 -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 45529 invoked by uid 89); 10 May 2017 02:45:35 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.0 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_SOFTFAIL autolearn=ham version=3.3.2 spammy=wish 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; Wed, 10 May 2017 02:45:33 +0000 Received: by simark.ca (Postfix, from userid 33) id A480C1E4A4; Tue, 9 May 2017 22:45:34 -0400 (EDT) To: Tim Wiederhake Subject: Re: [PATCH v3 06/12] btrace: Remove constant arguments. 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: Wed, 10 May 2017 02:45:00 -0000 From: Simon Marchi Cc: gdb-patches@sourceware.org, markus.t.metzger@intel.com In-Reply-To: <1494312929-22749-7-git-send-email-tim.wiederhake@intel.com> References: <1494312929-22749-1-git-send-email-tim.wiederhake@intel.com> <1494312929-22749-7-git-send-email-tim.wiederhake@intel.com> Message-ID: <05da714930334ac9ae083f28dcfa2fe0@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.2.5 X-IsSubscribed: yes X-SW-Source: 2017-05/txt/msg00236.txt.bz2 On 2017-05-09 02:55, Tim Wiederhake wrote: > 2017-05-09 Tim Wiederhake > > gdb/ChangeLog: > > * btrace.c (ftrace_new_function, ftrace_new_call, ftrace_new_tailcall, > ftrace_new_return, ftrace_new_switch, ftrace_new_gap, > ftrace_update_function): Remove arguments that implicitly were always > BTINFO->END. > (btrace_compute_ftrace_bts, ftrace_add_pt, btrace_compute_ftrace_pt): > Don't pass BTINFO->END. Looks good, just a few comments below. > diff --git a/gdb/btrace.c b/gdb/btrace.c > index cb30dcf..1bd11f0 100644 > --- a/gdb/btrace.c > +++ b/gdb/btrace.c > @@ -202,19 +202,19 @@ ftrace_function_switched (const struct > btrace_function *bfun, > return 0; > } > > -/* Allocate and initialize a new branch trace function segment. > +/* Allocate and initialize a new branch trace function segment at the > end of > + the trace. > BTINFO is the branch trace information for the current thread. > - PREV is the chronologically preceding function segment. > MFUN and FUN are the symbol information we have for this function. > */ > > static struct btrace_function * > ftrace_new_function (struct btrace_thread_info *btinfo, > - struct btrace_function *prev, > struct minimal_symbol *mfun, > struct symbol *fun) > { > - struct btrace_function *bfun; > + struct btrace_function *bfun, *prev; > > + prev = btinfo->end; Note that we can now declare variables at the point we use it, and drop the struct keyword, like: btrace_function *prev = btinfo->end; It's up to you, you can continue with your current style if you wish. > bfun = XCNEW (struct btrace_function); > > bfun->msym = mfun; > @@ -238,7 +238,7 @@ ftrace_new_function (struct btrace_thread_info > *btinfo, > } > > btinfo->functions.push_back (bfun); > - return bfun; > + return btinfo->end = bfun; Err I'm really not a fan of assignment as a side effect. > @@ -515,13 +510,13 @@ ftrace_new_gap (struct btrace_thread_info > *btinfo, > Return the chronologically latest function segment, never NULL. */ > > static struct btrace_function * > -ftrace_update_function (struct btrace_thread_info *btinfo, > - struct btrace_function *bfun, CORE_ADDR pc) > +ftrace_update_function (struct btrace_thread_info *btinfo, CORE_ADDR > pc) The comment of this function would need to be updated as well. Thanks, Simon