Re: [PATCH] perf jevents: Fix event code for events referencing std arch events

[John Garry]

On 12/10/2020 12:24, Jiri Olsa wrote:
On Mon, Oct 12, 2020 at 12:15:04PM +0100, John Garry wrote:
On 12/10/2020 11:54, Jiri Olsa wrote:
ff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index 99df41a9543d..e47644cab3fa 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -505,20 +505,15 @@ static char *real_event(const char *name, char *event)
   }
   static int
-try_fixup(const char *fn, char *arch_std, unsigned long long eventcode,
-         struct json_event *je)
+try_fixup(const char *fn, char *arch_std, struct json_event *je, char **event)
   {
        /* try to find matching event from arch standard values */
        struct event_struct *es;
        list_for_each_entry(es, &arch_std_events, list) {
                if (!strcmp(arch_std, es->name)) {
-                       if (!eventcode && es->event) {
-                               /* allow EventCode to be overridden */
-                               free(je->event);
-                               je->event = NULL;
-                       }
                        FOR_ALL_EVENT_STRUCT_FIELDS(TRY_FIXUP_FIELD);
+                       *event = je->event;
I'm bit rusty on this code, but isn't je->event NULL at this point?

je->event should be now assigned from es->event because of
FOR_ALL_EVENT_STRUCT_FIELDS(TRY_FIXUP_FIELD):

#define TRY_FIXUP_FIELD(field) do { if (es->field && !*field) {\
        *field = strdup(es->field);                          \
        if (!*field)                                            \
                return -ENOMEM;                                 \
} } while (0)

And es->event should be set.

right, thanks

Acked-by: Jiri Olsa <jo...@redhat.com>

jirka

.


"PMU events" testcase was still passing as it does not cover this 
scenario, so I'll look to expand the test to catch such problems.

Thanks,
John