[Libguestfs] [PATCH 5/7] New APIs: add-domain and add-libvirt-dom.

[Daniel P. Berrange]

On Wed, Nov 10, 2010 at 01:48:10PM +0000, Daniel P. Berrange wrote:
> On Wed, Nov 10, 2010 at 12:47:55PM +0000, Richard W.M. Jones wrote:
> > On Wed, Nov 10, 2010 at 12:26:03PM +0000, Daniel P. Berrange wrote:
> > > > > > diff --git a/src/guestfs.h b/src/guestfs.h
> > > > > > index 0f4f9fd..be7398a 100644
> > > > > > --- a/src/guestfs.h
> > > > > > +++ b/src/guestfs.h
> > > > > > @@ -79,6 +79,11 @@ extern void guestfs_set_private (guestfs_h *g, const char *key, void *data);
> > > > > >  extern void *guestfs_get_private (guestfs_h *g, const char *key);
> > > > > >  
> > > > > >  /*--- Structures and actions ---*/
> > > > > > +
> > > > > > +/* Hack to avoid needing to include <libvirt.h> */
> > > > > > +typedef struct _virDomain virDomain;
> > > > > > +typedef virDomain *virDomainPtr;
> > > > > > +
> > > > > >  #include <rpc/types.h>
> > > > > >  #include <rpc/xdr.h>
> > > > > >  #include <guestfs-structs.h>
> > > > > 
> > > > > Surely this will break any application whose code does
> > > > > 
> > > > >   #include <libvirt/libvirt.h>
> > > > >   #include <guestfs.h>
> > > > > 
> > > > > because they'll get a duplicated definition of virDomain from
> > > > > both headers. 
> > > > 
> > > > I checked and it works fine both ways round.  It would break if
> > > > libvirt changed the definition, but you've boxed yourself into a
> > > > corner there by documenting the current definition:
> > > 
> > > Surely the compiler should complain about duplicated typedefs, even
> > > if they are identical, because C doesn't allow duplicate typedefs
> > > within the same scope.
> > > 
> > >   http://david.tribble.com/text/cdiffs.htm#C99-typedefs
> > > 
> > > eg
> > > 
> > >   $ cat st.c 
> > >   typedef struct foo foo;
> > >   typedef struct foo foo;
> > >   $ gcc -Wall -o st st.c
> > >   st.c:3:20: error: redefinition of typedef ‘foo’
> > >   st.c:2:20: note: previous declaration of ‘foo’ was here
> > 
> > Apparently "scope" there means something a bit different.  Try this
> > program to see it definitely works (and also swap the order of the
> > include and the typedef):
> > 
> > ----------------------------------------------------------------------
> > #include <stdio.h>
> > #include <stdlib.h>
> > 
> > #include <libvirt/libvirt.h>
> > typedef struct _virDomain virDomain;
> > typedef virDomain *virDomainPtr;
> > 
> > int main () { printf ("hello\n"); exit (0); }
> > ----------------------------------------------------------------------
> > 
> > gcc -Wall -o test test.c
> > 
> > It doesn't need the #ifndef as suggested by that link.
> 
> This rather surprises me. I wonder if this is just an accident
> of the current GCC warning checker impl, or actually the correct
> standards compliant behaviour. I've always been under the impression
> that all header files had to have all their typedefs/functions/etc
> protected by #ifndef FOO/#define FOO/...content.../#endif to
> prevent multiple declaration.

Repeating from IRC for the benefit of archival...

The reason why it appeared to work when using #include <libvirt/libvirt.h>
is because libvirt.h was being resolved to /usr/include. This is one of
GCC's nominated "system include directories". A huge set of compiler
warnings are disabled for defintions from headers in a system include
directory.

So if both libguestfs & libvirt are installed in /usr or /usr/local,
the warnings will be avoided. If they're installed anywhere else,
then the duplicated typedefs will raise the compiler warnings.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|