Messages in this thread | | | From | David Laight <> | Subject | RE: [PATCH 3/4] perf annotate: add powerpc support | Date | Tue, 28 Jun 2016 16:07:10 +0000 |
| |
From: Ravi Bangoria > Sent: 28 June 2016 12:37 > > Powerpc has long list of branch instructions and hardcoding them in table > appears to be error-prone. So, add new function to find instruction > instead of creating table. > > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> > Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> > --- > tools/perf/util/annotate.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 64 insertions(+) > > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c > index 36a5825..96c6610 100644 > --- a/tools/perf/util/annotate.c > +++ b/tools/perf/util/annotate.c > @@ -476,6 +476,68 @@ static int ins__cmp(const void *a, const void *b) > return strcmp(ia->name, ib->name); > } > > +static struct ins *ins__find_powerpc(const char *name)
It would be better if the function name include 'branch'.
> +{ > + int i; > + struct ins *ins; > + > + ins = zalloc(sizeof(struct ins)); > + if (!ins) > + return NULL; > + > + ins->name = strdup(name); > + if (!ins->name) > + return NULL;
You leak 'ins' here.
> + > + if (name[0] == 'b') { > + /* branch instructions */ > + ins->ops = &jump_ops; > + > + /* > + * - Few start with 'b', but aren't branch instructions. > + * - Let's also ignore instructions involving 'ctr' and > + * 'tar' since target branch addresses for those can't > + * be determined statically. > + */ > + if (!strncmp(name, "bcd", 3) || > + !strncmp(name, "brinc", 5) || > + !strncmp(name, "bper", 4) || > + strstr(name, "ctr") || > + strstr(name, "tar")) > + return NULL;
More importantly you leak 'ins' and 'ins->name' here. And on other paths below.
...
David
| |