linux-kernel - Re: [PATCH 03/61] perf tools: Make hist_entry__snprintf work over struct perf_hpp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160921153038.GA28272@krava>
Date:   Wed, 21 Sep 2016 17:30:38 +0200
From:   Jiri Olsa <jolsa@...hat.com>
To:     Arnaldo Carvalho de Melo <acme@...nel.org>
Cc:     Jiri Olsa <jolsa@...nel.org>, lkml <linux-kernel@...r.kernel.org>,
        Don Zickus <dzickus@...hat.com>, Joe Mario <jmario@...hat.com>,
        Ingo Molnar <mingo@...nel.org>,
        Peter Zijlstra <a.p.zijlstra@...llo.nl>,
        Namhyung Kim <namhyung@...nel.org>,
        David Ahern <dsahern@...il.com>,
        Andi Kleen <andi@...stfloor.org>
Subject: Re: [PATCH 03/61] perf tools: Make hist_entry__snprintf work over
 struct perf_hpp_list

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 <jolsa@...nel.org>
> > ---
> >  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