Re: [Qemu-devel] [PATCH v3 06/18] trace: break circular dependancy in event-internal.h

[Daniel P. Berrange]

On Mon, Sep 19, 2016 at 07:08:57PM +0200, Lluís Vilanova wrote:
> Typo in subject: dependancy -> dependency
> 
> Daniel P Berrange writes:
> 
> > Currently event-internal.h includes generated-events.h,
> > while generated-events.h includes event-internal.h
> > causing a circular dependency.
> 
> > event-internal.h requires that the content of
> > generated-events.h comes first, so that it can see
> > the typedefs for TraceEventID and TraceEventVCPUID.
> 
> > Switching the TraceEvent struct to use uint32_t
> > for the two ID fields, removes the dependency on
> > the typedef, allowing events-internal.h to be a
> > self-contained header. This will then let the patch
> > following this move event-internal.h to the top of
> > generated-events.h, so we can expose TraceEvent
> > struct variables in generated-events.h
> 
> > Signed-off-by: Daniel P. Berrange <address@hidden>
> > ---
> >  trace/event-internal.h | 7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> > diff --git a/trace/event-internal.h b/trace/event-internal.h
> > index 3b9ceb5..36906e2 100644
> > --- a/trace/event-internal.h
> > +++ b/trace/event-internal.h
> > @@ -10,9 +10,6 @@
> >  #ifndef TRACE__EVENT_INTERNAL_H
> >  #define TRACE__EVENT_INTERNAL_H
>  
> > -#include "trace/generated-events.h"
> > -
> > -
> >  /**
> >   * TraceEvent:
> >   * @id: Unique event identifier.
> > @@ -28,8 +25,8 @@
> >   * Opaque generic description of a tracing event.
> >   */
> >  typedef struct TraceEvent {
> > -    TraceEventID id;
> > -    TraceEventVCPUID vcpu_id;
> > +    uint32_t id;
> > +    uint32_t vcpu_id;
> 
> Shouldn't these be 'size_t' for consistency with your iterator patch?

No, the iterator is using size_t because it is looping over array
entries and the logical max number of elements of an array in C is
the max value of size_t.

The ID values we assign events are fixed to 32-bits because that
is what we're encoding them as in simple trace.

Since we're going to have multiple groups of events we won't
have the assumption that ID values are the same as array
indexes by the end of the series.

IOW, I think it is correct as is.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|