From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54015) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bkSdU-0002RN-Gv for qemu-devel@nongnu.org; Thu, 15 Sep 2016 05:07:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bkSdP-0003Ui-Oh for qemu-devel@nongnu.org; Thu, 15 Sep 2016 05:07:11 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46640) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bkSdP-0003UY-FZ for qemu-devel@nongnu.org; Thu, 15 Sep 2016 05:07:07 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C3666E7B74 for ; Thu, 15 Sep 2016 09:07:06 +0000 (UTC) Date: Thu, 15 Sep 2016 10:07:03 +0100 From: "Daniel P. Berrange" Message-ID: <20160915090703.GC26068@redhat.com> Reply-To: "Daniel P. Berrange" References: <1473872922-23449-1-git-send-email-berrange@redhat.com> <1473872922-23449-2-git-send-email-berrange@redhat.com> <878tuuywqh.fsf@fimbulvetr.bsc.es> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <878tuuywqh.fsf@fimbulvetr.bsc.es> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 1/6] trace: add trace event iterator APIs List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org, Stefan Hajnoczi , Paolo Bonzini On Wed, Sep 14, 2016 at 11:53:10PM +0200, Llu=C3=ADs Vilanova wrote: > Daniel P Berrange writes: >=20 > > Currently methods which want to iterate over trace events, > > do so using the trace_event_count() and trace_event_id() > > methods. This leaks the concept of a single ID enum to > > the callers. There is an alternative trace_event_pattern() > > method which can be used in an iteration context, but its > > design is stateless, so is not easy to expand it in the > > future. >=20 > > This defines a formal iterator API will provide an future > > proof way of iterating over events. >=20 > > The iterator is also able to apply a pattern match filter > > to events, further removing the need for the pattern >=20 > > Signed-off-by: Daniel P. Berrange > > --- > > trace/control.c | 20 ++++++++++++++++++++ > > trace/control.h | 27 +++++++++++++++++++++++++++ > > 2 files changed, 47 insertions(+) >=20 > > diff --git a/trace/control.c b/trace/control.c > > index 05d85ac..b871727 100644 > > --- a/trace/control.c > > +++ b/trace/control.c > > @@ -125,6 +125,26 @@ TraceEvent *trace_event_pattern(const char *pat,= TraceEvent *ev) > > return NULL; > > } > =20 > > +void trace_event_iter_init(TraceEventIter *iter, const char *pattern= ) > > +{ > > + iter->event =3D 0; > > + iter->pattern =3D pattern; > > +} > > + > > +TraceEvent *trace_event_iter_next(TraceEventIter *iter) > > +{ > > + while (iter->event < TRACE_EVENT_COUNT) { > > + if (!iter->pattern || > > + pattern_glob(iter->pattern, > > + trace_event_get_name(&(trace_events[iter->e= vent])))) { > > + return &(trace_events[iter->event]); >=20 > That's a picky one (feel free to ignore), but can you refactor > "&(trace_events[iter->event])" out into a variable? The long pattern_gl= ob() call > is a bit hard to parse. >=20 >=20 > > + } > > + iter->event++; > > + } > > + > > + return NULL; > > +} > > + > > void trace_list_events(void) > > { > > int i; > > diff --git a/trace/control.h b/trace/control.h > > index 27a16fc..c71b405 100644 > > --- a/trace/control.h > > +++ b/trace/control.h > > @@ -13,6 +13,10 @@ > > #include "qemu-common.h" > > #include "trace/generated-events.h" > =20 > > +typedef struct TraceEventIter { > > + size_t event; >=20 > Shouldn't this be TraceEventID for consistence with "trace/control.h"? = But if > you're going to drop TraceEventID in a later series feel free to ignore= me. For array indexes size_t is always the optimal type, since that matches the maximum logical size that an array can be. It just happens that array indexes match TraceEventID but since that'll be killed in the API later, I figured it best to stick to the normal size_t . Regards, Daniel --=20 |: http://berrange.com -o- http://www.flickr.com/photos/dberrange= / :| |: http://libvirt.org -o- http://virt-manager.or= g :| |: http://autobuild.org -o- http://search.cpan.org/~danberr= / :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vn= c :|