[libvirt] cygwin: #include netinet/inet.h -- needed only on cygwin

[Eric Blake]

On 08/17/2010 10:48 AM, Stefan Berger wrote:
>  On 08/17/2010 12:25 PM, Daniel P. Berrange wrote:
>> On Tue, Aug 17, 2010 at 12:14:47PM -0400, Stefan Berger wrote:
>>>   When doing './autogen.sh --with-sasl --with-libvirtd=no' on cygwin, I
>>> needed to #include netinet/in.h for a successful build. Now
>>> authentication using SASL works.
>> Hmm, this is the sort of thing that GNULIB usually fixes for
>> us. What is the compile error without this patch present ?
>> Ideally we can get GNULIB to fix it

Well, gnulib does not yet offer any replacements for the <rpc/....h>
headers, in part because they are not standardized by POSIX.  That's
certainly a larger task than just working around it for now in libvirt.

The real question here is whether cygwin's <rpc/rpc.h> is broken because
it is not self-sufficient, or whether the compile error is due to
something else that libvirt is doing.  Which is why the actual compile
message is important (so I went and reproduced this setup - it took me
more than an hour to get to the failure point)...

> It complains about an incomplete datatype due to sockaddr_in being used
> in rpc/svc.h (xp_raddr in SVCXPRT struct).

I could not reproduce the failure with the latest cygwin:
cygwin               1.7.6-1        OK
libsasl2-devel       2.1.23-1       OK
libxml2-devel        2.7.7-1        OK
sunrpc               4.0-3          OK

What versions do you have installed?  (Not sure if I have any other
relevant installed packages that I didn't list here).

More particularly, I see that cygwin's <rpc/svc.h> has a blind reference
to sockaddr_in inside struct SVCXPRT, but libvirt includes the master
file <rpc/rpc.h> which includes <rpc/types.h> prior to <rpc/svc.h>; and
<rpc/types.h> includes <netinet/in.h>.

> +++ b/src/remote/qemu_protocol.h
> @@ -6,6 +6,9 @@
>  #ifndef _RP_QEMU_H_RPCGEN
>  #define _RP_QEMU_H_RPCGEN
>
> +#ifdef __CYGWIN__
> +# include <netinet/in.h>
> +#endif
>  #include <rpc/rpc.h> 

If we decide to apply this, then lose the #ifdef __CYGWIN__.  There's
nothing wrong with making this #include unconditional, particularly
since POSIX is clear that sockaddr_in may be defined in several headers
(<arpa/inet.h>, <netdb.h>), but only must be defined in <netinit/in.h>,
and since gnulib guarantees that <netinet/in.h> will exist.  glibc's
implementation is a bit looser and provides sockaddr_in in more places,
but that's no excuse for us to depend on that laziness.

But since I think upgrading the installation on your end will clear up
the compilation problem, I'm inclined to NAK this patch.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20100817/0068650d/attachment-0001.sig>