On Mon, Jun 11, 2012 at 6:20 PM, Lluís Vilanova <vilan...@ac.upc.edu> wrote: > Lluís Vilanova writes: > >> Stefan Hajnoczi writes: >>> On Tue, May 8, 2012 at 3:38 PM, Lluís Vilanova <vilan...@ac.upc.edu> wrote: >>>> Signed-off-by: Lluís Vilanova <vilan...@ac.upc.edu> >>>> --- >>>> monitor.c | 15 ++++++++++++--- >>>> 1 file changed, 12 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/monitor.c b/monitor.c >>>> index 8946a10..86c2538 100644 >>>> --- a/monitor.c >>>> +++ b/monitor.c >>>> @@ -625,10 +625,19 @@ static void do_trace_event_set_state(Monitor *mon, >>>> const QDict *qdict) >>>> { >>>> const char *tp_name = qdict_get_str(qdict, "name"); >>>> bool new_state = qdict_get_bool(qdict, "option"); >>>> - int ret = trace_event_set_state(tp_name, new_state); >>>> >>>> - if (!ret) { >>>> - monitor_printf(mon, "unknown event name \"%s\"\n", tp_name); >>>> + if (trace_event_is_pattern(tp_name)) { >>>> + TraceEvent *ev = NULL; >>>> + while ((ev = trace_event_pattern(tp_name, ev)) != NULL) { >>>> + trace_event_set_state_dynamic(ev, new_state); >>>> + } >>>> + } else { >>>> + TraceEvent *ev = trace_event_name(tp_name); >>>> + if (ev == NULL) { >>>> + monitor_printf(mon, "unknown event name \"%s\"\n", tp_name); >>>> + } else { >>>> + trace_event_set_state_dynamic(ev, new_state); >>>> + } > >>> Why check for a pattern and split the code in two? How about just: > >>> while ((ev = trace_event_pattern(tp_name, ev)) != NULL) { >>> ... >>> } > >>> That should cover both the single trace event name case and the wildcard >>> case. > >> That's true... it's just that somehow I thought it was abusive to use pattern >> matching on a string without patterns :) > > When going through the code to add your comments I realized why it made sense > to > use 'trace_event_name' instead of 'trace_event_pattern'. > > In the case the string contains no pattern, not finding a result is an error > in > 'trace_backend_init_events' (when reading the list of events given in the > commandline file), as well as in 'do_trace_event_set_state' (when setting the > state from the monitor, although here the error is not fatal). > > In comparison, setting by pattern never fails.
I see. Then the single code-path implementation looks like this: bool found = false; while ((ev = trace_event_pattern(tp_name, ev)) != NULL) { found = true; ... } if (!found && !trace_event_is_pattern(tp_name)) { fprintf(stderr, "ERROR!"); } It's up to you which you prefer. Stefan