Daniel P Berrange writes: > This converts the HMP/QMP monitor API implementations > and some internal trace control methods to use the new > trace event iterator APIs.
> Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com> > Signed-off-by: Daniel P. Berrange <berra...@redhat.com> > --- > monitor.c | 16 ++++++---- > trace/control.c | 94 > ++++++++++++++++++++++++++++++++++----------------------- > trace/qmp.c | 16 ++++++---- > 3 files changed, 76 insertions(+), 50 deletions(-) > diff --git a/monitor.c b/monitor.c > index 5c00373..7b979a6 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -3335,9 +3335,11 @@ void info_trace_events_completion(ReadLineState *rs, > int nb_args, const char *st > len = strlen(str); > readline_set_completion_index(rs, len); > if (nb_args == 2) { > - TraceEventID id; > - for (id = 0; id < trace_event_count(); id++) { > - const char *event_name = > trace_event_get_name(trace_event_id(id)); > + TraceEventIter iter; > + TraceEvent *ev; > + trace_event_iter_init(&iter, NULL); > + while ((ev = trace_event_iter_next(&iter)) != NULL) { > + const char *event_name = trace_event_get_name(ev); > if (!strncmp(str, event_name, len)) { > readline_add_completion(rs, event_name); > } > @@ -3352,9 +3354,11 @@ void trace_event_completion(ReadLineState *rs, int > nb_args, const char *str) > len = strlen(str); > readline_set_completion_index(rs, len); > if (nb_args == 2) { > - TraceEventID id; > - for (id = 0; id < trace_event_count(); id++) { > - const char *event_name = > trace_event_get_name(trace_event_id(id)); > + TraceEventIter iter; > + TraceEvent *ev; > + trace_event_iter_init(&iter, NULL); > + while ((ev = trace_event_iter_next(&iter)) != NULL) { > + const char *event_name = trace_event_get_name(ev); > if (!strncmp(str, event_name, len)) { > readline_add_completion(rs, event_name); > } > diff --git a/trace/control.c b/trace/control.c > index b871727..8fa7ed6 100644 > --- a/trace/control.c > +++ b/trace/control.c > @@ -60,9 +60,10 @@ TraceEvent *trace_event_name(const char *name) > { > assert(name != NULL); > - TraceEventID i; > - for (i = 0; i < trace_event_count(); i++) { > - TraceEvent *ev = trace_event_id(i); > + TraceEventIter iter; > + TraceEvent *ev; > + trace_event_iter_init(&iter, NULL); > + while ((ev = trace_event_iter_next(&iter)) != NULL) { > if (strcmp(trace_event_get_name(ev), name) == 0) { > return ev; > } You could pass "name" in the pattern argument, and then remove the strcmp(). It'll be simpler code, but pattern_glob() is less efficient than strcmp(). To solve that, maybe you could subsume exact name matching (trace_event_name()) and pattern matching into the iterator interface (strcmp() / pattern_glob()) by either checking trace_event_is_pattern() when initializing the iterator (pattern auto-detection), or explicitly passing either a name or pattern argument (if you want an extra-paranoid API; via two char* or a char*+bool). I haven't checked if that would weird other code out when using iterators for a simple exact match. > @@ -105,21 +106,20 @@ TraceEvent *trace_event_pattern(const char *pat, > TraceEvent *ev) > { > assert(pat != NULL); > - TraceEventID i; > - > - if (ev == NULL) { > - i = -1; > - } else { > - i = trace_event_get_id(ev); > - } > - i++; > - > - while (i < trace_event_count()) { > - TraceEvent *res = trace_event_id(i); > - if (pattern_glob(pat, trace_event_get_name(res))) { > - return res; > + bool matched = ev ? false : true; > + TraceEventIter iter; > + TraceEvent *thisev; > + trace_event_iter_init(&iter, NULL); > + while ((thisev = trace_event_iter_next(&iter)) != NULL) { > + if (matched) { > + if (pattern_glob(pat, trace_event_get_name(thisev))) { > + return thisev; > + } > + } else { > + if (ev == thisev) { > + matched = true; > + } > } > - i++; > } > return NULL; Shouldn't this pass "pat" to trace_event_iter_init() and then not use pattern_glob()? I just realized this is dropped on next patch, so ignore me. Cheers, Lluis