From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757718AbcIUPap (ORCPT ); Wed, 21 Sep 2016 11:30:45 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49824 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757688AbcIUPan (ORCPT ); Wed, 21 Sep 2016 11:30:43 -0400 Date: Wed, 21 Sep 2016 17:30:38 +0200 From: Jiri Olsa To: Arnaldo Carvalho de Melo Cc: Jiri Olsa , lkml , Don Zickus , Joe Mario , Ingo Molnar , Peter Zijlstra , Namhyung Kim , David Ahern , Andi Kleen Subject: Re: [PATCH 03/61] perf tools: Make hist_entry__snprintf work over struct perf_hpp_list Message-ID: <20160921153038.GA28272@krava> References: <1474290610-23241-1-git-send-email-jolsa@kernel.org> <1474290610-23241-4-git-send-email-jolsa@kernel.org> <20160921151432.GG4973@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160921151432.GG4973@kernel.org> User-Agent: Mutt/1.7.0 (2016-08-17) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Wed, 21 Sep 2016 15:30:42 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Sep 21, 2016 at 12:14:32PM -0300, Arnaldo Carvalho de Melo wrote: > Em Mon, Sep 19, 2016 at 03:09:12PM +0200, Jiri Olsa escreveu: > > Make hist_entry__snprintf to take perf_hpp_list as an argument > > instead of using he->hists->hpp_list. This way we can display > > arbitrary list of entries regardles of the hists setup, which > > will be useful in following patches. > > > > Link: http://lkml.kernel.org/n/tip-j2sizkyglam3narmndlj99xq@git.kernel.org > > Signed-off-by: Jiri Olsa > > --- > > tools/perf/ui/stdio/hist.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c > > index a57131e61fe3..cb0371106c21 100644 > > --- a/tools/perf/ui/stdio/hist.c > > +++ b/tools/perf/ui/stdio/hist.c > > @@ -373,7 +373,8 @@ static size_t hist_entry_callchain__fprintf(struct hist_entry *he, > > return 0; > > } > > > > -static int hist_entry__snprintf(struct hist_entry *he, struct perf_hpp *hpp) > > +static int hist_entry__snprintf(struct hist_entry *he, struct perf_hpp *hpp, > > + struct perf_hpp_list *hpp_list) > > What I usually do in these cases is to keep the existing interface and > add a new one that is more low level, that exhibits more flexibility, > i.e.: > > static int __hist_entry__snprintf(struct hist_entry *he, struct perf_hpp *hpp, > struct perf_hpp_list *hpp_list) > { > ... > } > > And: > > static int hist_entry__snprintf(struct hist_entry *he, struct perf_hpp *hpp) > { > return __hist_entry__snprintf(he, hpp, he->hists->hpp_list); > } > > This way no users of the existing function need to be changed, and new > ones can use the more flexible, lower level interface. > > In this case there is just one such user, but the refactoring technique > could be consistently used, other people will not be left scratching > their heads asking why we pass something that can be obtained from > another parameter already in that function, while __ functions already > indicate they are more flexible and thus can flout that assumption. ook, will change thanks, jirka