[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <063D6719AE5E284EB5DD2968C1650D6D5F4E67FA@AcuExch.aculab.com>
Date: Tue, 28 Jun 2016 16:07:10 +0000
From: David Laight <David.Laight@...LAB.COM>
To: 'Ravi Bangoria' <ravi.bangoria@...ux.vnet.ibm.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"acme@...nel.org" <acme@...nel.org>,
"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>
CC: "ananth@...ibm.com" <ananth@...ibm.com>,
"naveen.n.rao@...ux.vnet.ibm.com" <naveen.n.rao@...ux.vnet.ibm.com>,
"dja@...ens.net" <dja@...ens.net>
Subject: RE: [PATCH 3/4] perf annotate: add powerpc support
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@...ux.vnet.ibm.com>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@...ux.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
Powered by blists - more mailing lists