On 06/14/2018 03:53 PM, Paul Clarke wrote:
> On 06/14/2018 06:48 AM, Thomas Richter wrote:
>> PMU alias definitions in sysfs files may have spaces, newlines
>> and number with leading zeroes. Same alias definitions may
>> also appear in JSON files without spaces, etc.
>>
>> Scan alias definitions and remove leading zeroes, spaces,
>> newlines, etc and rebuild string to make alias->str member
>> comparable.
>>
>> s390 for example has terms specified as
>> event=0x0091 (read from files ../<PMU>/events/<FILE>
>> and terms specified as event=0x91 (read from JSON files).
>>
>> Signed-off-by: Thomas Richter <tmri...@linux.ibm.com>
>> ---
>> tools/perf/util/pmu.c | 25 ++++++++++++++++++++++++-
>> 1 file changed, 24 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
>> index 26c79a9c4142..da8f243743d3 100644
>> --- a/tools/perf/util/pmu.c
>> +++ b/tools/perf/util/pmu.c
>> @@ -241,9 +241,11 @@ static int __perf_pmu__new_alias(struct list_head
>> *list, char *dir, char *name,
>> char *metric_expr,
>> char *metric_name)
>> {
>> + struct parse_events_term *term;
>> struct perf_pmu_alias *alias;
>> int ret;
>> int num;
>> + char newval[256];
>
> How was 256 chosen?
I copied this from function perf_pmu__new_alias() which also uses:
char buf[256];
Looking a the sysfs file contents this seems to be sufficient.
This number comes from commit long time ago.
>
>>
>> alias = malloc(sizeof(*alias));
>> if (!alias)
>> @@ -262,6 +264,27 @@ static int __perf_pmu__new_alias(struct list_head
>> *list, char *dir, char *name,
>> return ret;
>> }
>>
>> + /* Scan event and remove leading zeroes, spaces, newlines, some
>> + * platforms have terms specified as
>> + * event=0x0091 (read from files ../<PMU>/events/<FILE>
>> + * and terms specified as event=0x91 (read from JSON files).
>> + *
>> + * Rebuild string to make alias->str member comparable.
>> + */
>> + memset(newval, 0, sizeof(newval));
>> + ret = 0;
>> + list_for_each_entry(term, &alias->terms, list) {
>> + if (ret)
>> + ret += scnprintf(newval + ret, sizeof(newval) - ret,
>> + ",");
>> + if (term->type_val == PARSE_EVENTS__TERM_TYPE_NUM)
>> + ret += scnprintf(newval + ret, sizeof(newval) - ret,
>> + "%s=%#x", term->config, term->val.num);
>> + else if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR)
>> + ret += scnprintf(newval + ret, sizeof(newval) - ret,
>> + "%s=%s", term->config, term->val.str);
>
> If we exceed 256, we just suddenly terminate the rebuilding without reporting
> any issues.
Correct the string would be truncated, but see above, it would not have been
read correctly
anyway.
>
>> + }
>> +
>> alias->name = strdup(name);
>> if (dir) {
>> /*
>> @@ -285,7 +308,7 @@ static int __perf_pmu__new_alias(struct list_head *list,
>> char *dir, char *name,
>> snprintf(alias->unit, sizeof(alias->unit), "%s", unit);
>> }
>> alias->per_pkg = perpkg && sscanf(perpkg, "%d", &num) == 1 && num == 1;
>> - alias->str = strdup(val);
>> + alias->str = strdup(newval);
>>
>> list_add_tail(&alias->list, list);
>>
>
> PC
>
--
Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
--
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB
243294
