lkml.org 
[lkml]   [2013]   [Nov]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/2] perf trace: Fix segfault on perf trace -i perf.data
Em Tue, Nov 12, 2013 at 09:27:23PM +0000, Namhyung Kim escreveu:
> Hi Arnaldo,
>
> On Tue, Nov 12, 2013 at 08:57:00AM -0300, Arnaldo Carvalho de Melo wrote:
> > So this becomes the first part of this patch, split from yours and
> > massaged a bit so that by looking at the patch it becomes quickly clear
> > what it is doing, please let me now if I can keep this as-is (with your
> > authorship, etc).
>
> Looks good to me.

Thanks for checking!

> But I just have a nitpick, please see below.

> > +{
> > + evsel->priv = malloc(sizeof(struct syscall_tp));
> > + if (evsel->priv != NULL) {
> > + if (perf_evsel__init_sc_tp_uint_field(evsel, id))
> > + goto out_delete;
> > +
> > + evsel->handler = handler;
> > + return 0;
> > + }
> > +
> > + return -ENOMEM;
> > +
> > +out_delete:
> > + free(evsel->priv);
> > + evsel->priv = NULL;

> Is this part needed? I can see that perf_evsel__delete_priv() can do
> it for you anyway. Yes I know it's needed for my later change, but I
> think we do it a bit differently.

> And again, is perf_evsel__delete_priv() needed? Isn't the ->priv is
> not used for anything else? Why not just letting perf_evsel__delete()
> handle this transparently?

Because it may point to something not allocated via malloc, so the one
who allocates it, frees it, furthermore, the one who allocates it, if
fails to complete the greater init sequence of which the allocation is
part of, frees it and leaves it as it was before the transaction
started.

- Arnaldo


\
 
 \ /
  Last update: 2013-11-12 15:01    [W:0.110 / U:0.772 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site