Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1436548
| From | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH 1/4] perf tools: Introduce hist_entry__init function |
| Date | 2016-07-04 21:10 +0200 |
| Message-ID | <rRhpL-116-15@gated-at.bofh.it> (permalink) |
| References | <rRcJs-6B7-21@gated-at.bofh.it> <rRcJs-6B7-27@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
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@kernel.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
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[RFC 0/4] perf tools: Add hist_entry allocation callbacks Jiri Olsa <jolsa@kernel.org> - 2016-07-04 16:10 +0200
[PATCH 1/4] perf tools: Introduce hist_entry__init function Jiri Olsa <jolsa@kernel.org> - 2016-07-04 16:10 +0200
Re: [PATCH 1/4] perf tools: Introduce hist_entry__init function Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-07-04 21:10 +0200
[PATCH 2/4] perf tools: Do the error path hist_entry release in hist_entry__new Jiri Olsa <jolsa@kernel.org> - 2016-07-04 16:10 +0200
Re: [PATCH 2/4] perf tools: Do the error path hist_entry release in hist_entry__new Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-07-04 21:10 +0200
Re: [PATCH 2/4] perf tools: Do the error path hist_entry release in hist_entry__new Jiri Olsa <jolsa@redhat.com> - 2016-07-05 08:40 +0200
[PATCH 3/4] perf tools: Introduce hist_entry_ops Jiri Olsa <jolsa@kernel.org> - 2016-07-04 16:10 +0200
[PATCH 4/4] perf tools: Introduce hists__add_entry_ops function Jiri Olsa <jolsa@kernel.org> - 2016-07-04 16:10 +0200
csiph-web