| 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
| ||
|
Message-ID: <20160704190854.GV5324@kernel.org>
Date: Mon, 4 Jul 2016 16:08:54 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Jiri Olsa <jolsa@...nel.org>
Cc: lkml <linux-kernel@...r.kernel.org>,
David Ahern <dsahern@...il.com>,
Ingo Molnar <mingo@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>
Subject: Re: [PATCH 1/4] perf tools: Introduce hist_entry__init function
Em Mon, Jul 04, 2016 at 04:01:36PM +0200, Jiri Olsa escreveu:
> Move hist_entry initialization code into separate function.
> It'll be useful and more clear for following patches that
> introduce allocation callbacks.
>
> Link: http://lkml.kernel.org/n/tip-j8dz6ytir91x8qd1zk4vf7ki@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@...nel.org>
> ---
> tools/perf/util/hist.c | 142 ++++++++++++++++++++++++++-----------------------
> 1 file changed, 75 insertions(+), 67 deletions(-)
>
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index e1fcc8d7c01a..ed7bea9aa68d 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -352,89 +352,97 @@ void hists__delete_entries(struct hists *hists)
> * histogram, sorted on item, collects periods
> */
>
> -static struct hist_entry *hist_entry__new(struct hist_entry *template,
> - bool sample_self)
> +static int hist_entry__init(struct hist_entry *he,
> + struct hist_entry *template,
> + bool sample_self)
> {
> - size_t callchain_size = 0;
> - struct hist_entry *he;
> + *he = *template;
>
> - if (symbol_conf.use_callchain)
> - callchain_size = sizeof(struct callchain_root);
> -
> - he = zalloc(sizeof(*he) + callchain_size);
> + if (symbol_conf.cumulate_callchain) {
> + he->stat_acc = malloc(sizeof(he->stat));
> + if (he->stat_acc == NULL) {
> + free(he);
> + return -ENOMEM;
> + }
> + memcpy(he->stat_acc, &he->stat, sizeof(he->stat));
> + if (!sample_self)
> + memset(&he->stat, 0, sizeof(he->stat));
> + }
>
> - if (he != NULL) {
> - *he = *template;
> + map__get(he->ms.map);
>
> - if (symbol_conf.cumulate_callchain) {
> - he->stat_acc = malloc(sizeof(he->stat));
> - if (he->stat_acc == NULL) {
> - free(he);
> - return NULL;
> - }
> - memcpy(he->stat_acc, &he->stat, sizeof(he->stat));
> - if (!sample_self)
> - memset(&he->stat, 0, sizeof(he->stat));
> + if (he->branch_info) {
> + /*
> + * This branch info is (a part of) allocated from
> + * sample__resolve_bstack() and will be freed after
> + * adding new entries. So we need to save a copy.
> + */
> + he->branch_info = malloc(sizeof(*he->branch_info));
> + if (he->branch_info == NULL) {
> + map__zput(he->ms.map);
> + free(he->stat_acc);
> + free(he);
> + return -ENOMEM;
> }
>
> - map__get(he->ms.map);
> + memcpy(he->branch_info, template->branch_info,
> + sizeof(*he->branch_info));
>
> - if (he->branch_info) {
> - /*
> - * This branch info is (a part of) allocated from
> - * sample__resolve_bstack() and will be freed after
> - * adding new entries. So we need to save a copy.
> - */
> - he->branch_info = malloc(sizeof(*he->branch_info));
> - if (he->branch_info == NULL) {
> - map__zput(he->ms.map);
> - free(he->stat_acc);
> - free(he);
> - return NULL;
> - }
> + map__get(he->branch_info->from.map);
> + map__get(he->branch_info->to.map);
> + }
>
> - memcpy(he->branch_info, template->branch_info,
> - sizeof(*he->branch_info));
> + if (he->mem_info) {
> + map__get(he->mem_info->iaddr.map);
> + map__get(he->mem_info->daddr.map);
> + }
>
> - map__get(he->branch_info->from.map);
> - map__get(he->branch_info->to.map);
> - }
> + if (symbol_conf.use_callchain)
> + callchain_init(he->callchain);
>
> - if (he->mem_info) {
> - map__get(he->mem_info->iaddr.map);
> - map__get(he->mem_info->daddr.map);
> + if (he->raw_data) {
> + he->raw_data = memdup(he->raw_data, he->raw_size);
> +
> + if (he->raw_data == NULL) {
> + map__put(he->ms.map);
> + if (he->branch_info) {
> + map__put(he->branch_info->from.map);
> + map__put(he->branch_info->to.map);
> + free(he->branch_info);
> + }
> + if (he->mem_info) {
> + map__put(he->mem_info->iaddr.map);
> + map__put(he->mem_info->daddr.map);
> + }
> + free(he->stat_acc);
> + free(he);
> + return -ENOMEM;
> }
> + }
> + INIT_LIST_HEAD(&he->pairs.node);
> + thread__get(he->thread);
>
> - if (symbol_conf.use_callchain)
> - callchain_init(he->callchain);
> + if (!symbol_conf.report_hierarchy)
> + he->leaf = true;
>
> - if (he->raw_data) {
> - he->raw_data = memdup(he->raw_data, he->raw_size);
> + return 0;
> +}
>
> - if (he->raw_data == NULL) {
> - map__put(he->ms.map);
> - if (he->branch_info) {
> - map__put(he->branch_info->from.map);
> - map__put(he->branch_info->to.map);
> - free(he->branch_info);
> - }
> - if (he->mem_info) {
> - map__put(he->mem_info->iaddr.map);
> - map__put(he->mem_info->daddr.map);
> - }
> - free(he->stat_acc);
> - free(he);
> - return NULL;
> - }
> - }
> - INIT_LIST_HEAD(&he->pairs.node);
> - thread__get(he->thread);
> +static struct hist_entry *hist_entry__new(struct hist_entry *template,
> + bool sample_self)
> +{
> + size_t callchain_size = 0;
> + struct hist_entry *he;
> + int err = 0;
>
> - if (!symbol_conf.report_hierarchy)
> - he->leaf = true;
> - }
> + if (symbol_conf.use_callchain)
> + callchain_size = sizeof(struct callchain_root);
>
> - return he;
> + he = zalloc(sizeof(*he) + callchain_size);
> + if (he)
> + err = hist_entry__init(he, template, sample_self);
An init function shouldn't free the object being initialized, I haven't
checked if that is the case, the renaming of new->init made the patch
difficult to read, perhaps its just leaking it here in case init()
returns !0, please check.
I.e. if init() somehow fails, then new() should detect that, and free
what new() allocated ('he' in this case).
> +
> + return err ? NULL : he;
> }
>
> static u8 symbol__parent_filter(const struct symbol *parent)
> --
> 2.4.11
Powered by blists - more mailing lists