From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1757113AbcCCXwY (ORCPT <rfc822;w@1wt.eu>);
	Thu, 3 Mar 2016 18:52:24 -0500
Received: from LGEAMRELO13.lge.com ([156.147.23.53]:38532 "EHLO
	lgeamrelo13.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1756853AbcCCXwT (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Thu, 3 Mar 2016 18:52:19 -0500
X-Original-SENDERIP: 156.147.1.125
X-Original-MAILFROM: namhyung@kernel.org
X-Original-SENDERIP: 165.244.98.203
X-Original-MAILFROM: namhyung@kernel.org
X-Original-SENDERIP: 10.177.227.17
X-Original-MAILFROM: namhyung@kernel.org
Date: Fri, 4 Mar 2016 08:52:13 +0900
From: Namhyung Kim <namhyung@kernel.org>
To: Jiri Olsa <jolsa@redhat.com>
CC: Arnaldo Carvalho de Melo <acme@kernel.org>, Ingo Molnar <mingo@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>, Jiri Olsa <jolsa@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>, David Ahern <dsahern@gmail.com>,
        Andi Kleen <andi@firstfloor.org>,
        Stephane Eranian <eranian@google.com>, Wang Nan <wangnan0@huawei.com>
Subject: Re: [PATCH 2/8] perf tools: Introduce perf_hpp__setup_hists_formats()
Message-ID: <20160303235213.GB16631@sejong>
References: <1456935128-31299-1-git-send-email-namhyung@kernel.org>
 <1456935128-31299-3-git-send-email-namhyung@kernel.org>
 <20160303134022.GA5487@krava.redhat.com>
MIME-Version: 1.0
In-Reply-To: <20160303134022.GA5487@krava.redhat.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
X-MIMETrack: Itemize by SMTP Server on LGEKRMHUB01/LGE/LG Group(Release 8.5.3FP6|November
 21, 2013) at 2016/03/04 08:52:15,
	Serialize by Router on LGEKRMHUB01/LGE/LG Group(Release 8.5.3FP6|November
 21, 2013) at 2016/03/04 08:52:15,
	Serialize complete at 2016/03/04 08:52:15
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Jiri,

On Thu, Mar 03, 2016 at 02:40:22PM +0100, Jiri Olsa wrote:
> On Thu, Mar 03, 2016 at 01:12:02AM +0900, Namhyung Kim wrote:
> 
> SNIP
> 
> > +struct perf_hpp_fmt *perf_hpp_fmt__copy(struct perf_hpp_fmt *fmt);
> > +
> >  int hist_entry__filter(struct hist_entry *he, int type, const void *arg);
> >  
> >  static inline bool perf_hpp__should_skip(struct perf_hpp_fmt *format,
> > diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> > index 590ebf70e6da..29c75f0374c4 100644
> > --- a/tools/perf/util/sort.c
> > +++ b/tools/perf/util/sort.c
> > @@ -1908,6 +1908,31 @@ __alloc_dynamic_entry(struct perf_evsel *evsel, struct format_field *field,
> >  	return hde;
> >  }
> >  
> > +struct perf_hpp_fmt *perf_hpp_fmt__copy(struct perf_hpp_fmt *fmt)
> 
> perf_hpp_fmt__dup might be better

OK

> 
> > +{
> > +	struct perf_hpp_fmt *new_fmt = NULL;
> > +
> > +	if (perf_hpp__is_sort_entry(fmt)) {
> > +		struct hpp_sort_entry *hse, *new_hse;
> > +
> > +		hse = container_of(fmt, struct hpp_sort_entry, hpp);
> > +		new_hse = memdup(hse, sizeof(*hse));
> > +		if (new_hse)
> > +			new_fmt = &new_hse->hpp;
> > +	} else if (perf_hpp__is_dynamic_entry(fmt)) {
> > +		struct hpp_dynamic_entry *hde, *new_hde;
> > +
> > +		hde = container_of(fmt, struct hpp_dynamic_entry, hpp);
> > +		new_hde = memdup(hde, sizeof(*hde));
> > +		if (new_hde)
> > +			new_fmt = &new_hde->hpp;
> > +	} else {
> > +		new_fmt = memdup(fmt, sizeof(*fmt));
> > +	}
> 
> how about also initialize new_fmt's list and sort_list in here
> 
> could save some time in future if perf_hpp_fmt__copy is
> used in another place, which won't directly initialize them
> like you do in add_hierarchy_fmt

Fair enough, will change.

Thanks,
Namhyung