Hi Ravi, (I avoided to review parser part since it may go to yacc in next version)
On Tue, 14 Mar 2017 20:36:54 +0530 Ravi Bangoria <[email protected]> wrote: [SNIP] > @@ -1516,9 +1534,10 @@ static bool dry_run; > * using pipes, etc. > */ > static struct option __record_options[] = { > - OPT_CALLBACK('e', "event", &record.evlist, "event", > - "event selector. use 'perf list' to list available events", > - parse_events_option), > + OPT_CALLBACK_ARG('e', "event", &record.evlist, > + &record.sdt_event_list, "event", > + "event selector. use 'perf list' to list available > events", > + record__parse_events_option), Does --event option NOT requires argument without this patch? If it should be changed to use OPT_CALLBACK_ARG(), would it be better merge this part to previous patch? [SNIP] > +/* > + * Delete the SDT events from uprobe_events file that > + * were created initially. > + */ > +void remove_sdt_event_list(struct list_head *sdt_events) > +{ > + struct sdt_event_list *sdt_event; > + struct strfilter *filter = NULL; > + const char *err = NULL; > + > + if (list_empty(sdt_events)) > + return; > + > + list_for_each_entry(sdt_event, sdt_events, list) { > + if (!filter) { > + filter = strfilter__new(sdt_event->name, &err); > + if (!filter) > + goto free_list; Don't we need to return error code for this case? > + } else { > + strfilter__or(filter, sdt_event->name, &err); strfilter__or() can fail here. > + } > + } > + > + del_perf_probe_events(filter); Here too, if it is ignored silently by design, please comment it here. > + > +free_list: > + free_sdt_list(sdt_events); > +} > + > +static int get_sdt_events_from_cache(struct perf_probe_event *pev) > +{ > + int ret = 0; > + > + pev->ntevs = find_cached_events_all(pev, &pev->tevs); > + > + if (pev->ntevs < 0) { > + pr_err("Error: Cache lookup failed (code: %d)\n", pev->ntevs); > + ret = pev->ntevs; > + } else if (!pev->ntevs) { > + pr_err("Error: %s:%s not found in the cache\n", > + pev->group, pev->event); > + ret = -EINVAL; > + } else if (pev->ntevs > 1) { > + pr_warning("Warning : Recording on %d occurences of %s:%s\n", > + pev->ntevs, pev->group, pev->event); > + } > + > + return ret; > +} > + > +static int add_event_to_sdt_evlist(struct probe_trace_event *tev, > + struct list_head *sdt_evlist) > +{ > + struct sdt_event_list *tmp; Well, strbuf can make this simpler as below ;-) struct strbuf buf = STRBUF_INIT; > + > + tmp = zalloc(sizeof(*tmp)); > + if (!tmp) > + return -ENOMEM; > + > + INIT_LIST_HEAD(&tmp->list); if (strbuf_addf(&buf, "%s:%s", tev->group, tev->event)) goto error; tmp->name = strbuf_detach(&buf); > + list_add(&tmp->list, sdt_evlist); > + > + return 0; error: free(tmp); return -ENOMEM; > +} > + > +static int add_events_to_sdt_evlist(struct perf_probe_event *pev, > + struct list_head *sdt_evlist) > +{ > + int i, ret; > + > + for (i = 0; i < pev->ntevs; i++) { > + ret = add_event_to_sdt_evlist(&pev->tevs[i], sdt_evlist); > + > + if (ret < 0) > + return ret; > + } > + return 0; > +} Thanks, -- Masami Hiramatsu <[email protected]>
