Re: [PATCH 02/16] perf pmu: Let pmu's with no events show up on perf list

From: Adrian Hunter
Date: Fri Oct 24 2014 - 10:38:58 EST


On 24/10/2014 5:36 p.m., Adrian Hunter wrote:
On 24/10/2014 4:21 p.m., Arnaldo Carvalho de Melo wrote:
Em Fri, Oct 24, 2014 at 10:03:20AM -0300, Arnaldo Carvalho de Melo escreveu:
Em Fri, Oct 24, 2014 at 09:57:02AM -0300, Arnaldo Carvalho de Melo escreveu:
Em Fri, Oct 24, 2014 at 02:15:52PM +0900, Namhyung Kim escreveu:
On Thu, 23 Oct 2014 13:45:10 +0300, Adrian Hunter wrote:
+ if (pmu->selectable) {
+ scnprintf(buf, sizeof(buf), "%s//", pmu->name);
+ aliases[j] = strdup(buf);

You need to check the return value here (and above too).

Well spotted, fixing this up.

Oh well, this print_pmu_events() function needs some care, it starts by
trying to alloc the array, if it fails, it silently returns, does that
mean that there are no pmu events? Or that memory allocation failed?

Ok, will do the fixes in a separate patch...

- Arnaldo

The patch below should check everything and warn the user, even
maintaining that void return...

Acked-by tags welcome as always :-)

- Arnaldo


diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 91dca60..881b754 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -753,9 +753,9 @@ void print_pmu_events(const char *event_glob, bool name_only)
if (pmu->selectable)
len++;
}
- aliases = malloc(sizeof(char *) * len);
+ aliases = zalloc(sizeof(char *) * len);
if (!aliases)
- return;
+ goto out_enomem;

That path tries to free aliases[j] but aliases is null. You could set len to 0 in that case.

Than again, no it doesn't sorry for the noise.


pmu = NULL;
j = 0;
while ((pmu = perf_pmu__scan(pmu)) != NULL) {
@@ -768,16 +768,20 @@ void print_pmu_events(const char *event_glob, bool name_only)
(!is_cpu && strglobmatch(alias->name,
event_glob))))
continue;
- aliases[j] = name;
+
if (is_cpu && !name_only)
- aliases[j] = format_alias_or(buf, sizeof(buf),
- pmu, alias);
- aliases[j] = strdup(aliases[j]);
+ name = format_alias_or(buf, sizeof(buf), pmu, alias);
+
+ aliases[j] = strdup(name);
+ if (aliases[j] == NULL)
+ goto out_enomem;
j++;
}
if (pmu->selectable) {
- scnprintf(buf, sizeof(buf), "%s//", pmu->name);
- aliases[j] = strdup(buf);
+ char *s;
+ if (asprintf(&s, "%s//", pmu->name) < 0)
+ goto out_enomem;
+ aliases[j] = s;
j++;
}
}
@@ -789,12 +793,20 @@ void print_pmu_events(const char *event_glob, bool name_only)
continue;
}
printf(" %-50s [Kernel PMU event]\n", aliases[j]);
- zfree(&aliases[j]);
printed++;
}
if (printed)
printf("\n");
- free(aliases);
+out_free:
+ for (j = 0; j < len; j++)
+ zfree(&aliases[j]);
+ zfree(&aliases);
+ return;
+
+out_enomem:
+ printf("FATAL: not enough memory to print PMU events\n");
+ if (aliases)
+ goto out_free;
}

bool pmu_have_event(const char *pname, const char *name)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/