RE: [PATCH 3/4] perf annotate: add powerpc support

From: David Laight
Date: Tue Jun 28 2016 - 12:09:43 EST


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@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@xxxxxxxxxxxxxxxxxx>
> ---
> 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