Re: [PATCH v3 10/10] perf list: Add json output option

[Ian Rogers]

On Wed, Nov 16, 2022 at 7:21 AM Arnaldo Carvalho de Melo
<acme@xxxxxxxxxx> wrote:
>
> Em Wed, Nov 16, 2022 at 10:10:53AM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Wed, Nov 16, 2022 at 09:41:43AM -0300, Arnaldo Carvalho de Melo escreveu:
> > > But then:
> >
> > > [root@five ~]# perf list syscalls:sys_enter_open* |& grep syscalls:
> > >   syscalls:sys_enter_open                            [Tracepoint event]
> > >   syscalls:sys_enter_open_by_handle_at               [Tracepoint event]
> > >   syscalls:sys_enter_open_tree                       [Tracepoint event]
> > >   syscalls:sys_enter_openat                          [Tracepoint event]
> > >   syscalls:sys_enter_openat2                         [Tracepoint event]
> > > [root@five ~]#
> > >
> > > This stops working, looking into it.
> >
> > Sidetracked with other stuff, please find what I have patched at
> > perf/perf-list-json-output in my tree.
> >
> > I removed the last two patches and I'm testing so that I can push
> > perf/core with your series modulo the last two + Namhyung's 'perf list'
> > kit.
>
> I just saw you sent a patch on top of the previous one, will try and
> combine stuff to remove failures from the bisect history.
>
> - Arnaldo

So the failing test was skipping for me due to a lack of kernel
symbols, sorry for not spotting it. I find that the issue is resolved
with your fixes and:

```
diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 30937e1dd82c..ad6cb5d2e1cc 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -107,7 +107,7 @@ static void default_print_event(void *ps, const
char *pmu_name, const char *topi
       if (deprecated && !print_state->deprecated)
               return;

-       if (print_state->pmu_glob && !strglobmatch(pmu_name,
print_state->pmu_glob))
+       if (print_state->pmu_glob && pmu_name &&
!strglobmatch(pmu_name, print_state->pmu_glob))
               return;

       if (print_state->event_glob &&
@@ -534,24 +534,18 @@ int cmd_list(int argc, const char **argv)
                       default_ps.metrics = false;
                       metricgroup__print(&print_cb, ps);
               } else if ((sep = strchr(argv[i], ':')) != NULL) {
-                       int sep_idx;
-
-                       sep_idx = sep - argv[i];
-                       s = strdup(argv[i]);
-                       if (s == NULL) {
+                       default_ps.event_glob = strdup(argv[i]);
+                       if (!default_ps.event_glob) {
                               ret = -1;
                               goto out;
                       }
-
-                       s[sep_idx] = '\0';
-                       default_ps.pmu_glob = s;
-                       default_ps.event_glob = s + sep_idx + 1;
                       print_tracepoint_events(&print_cb, ps);
                       print_sdt_events(&print_cb, ps);
                       default_ps.metrics = true;
                       default_ps.metricgroups = true;
                       metricgroup__print(&print_cb, ps);
-                       free(s);
+                       free(default_ps.event_glob);
+                       default_ps.event_glob = NULL;
```
I think this should be squashed into "perf list: Reorganize to use
callbacks". Some explanation, in porting the : glob case I'd assumed
the before the colon would be the PMU and the after the event. Doing
things caused tracepoint output to differ too much and so for
tracepoints the : is kept in the event name. So we can simplify the
matching to not be pmu and event, just use the event glob.

Thanks,
Ian

               } else {
                       if (asprintf(&s, "*%s*", argv[i]) < 0) {
                               printf("Critical: Not enough memory!
Trying to continue...\n");