Bug 43986 - Wocky fails to cross compile with mingw32
Summary: Wocky fails to cross compile with mingw32
Status: RESOLVED FIXED
Alias: None
Product: Wocky
Classification: Unclassified
Component: General (show other bugs)
Version: git master
Hardware: x86 (IA32) Windows (All)
: medium normal
Assignee: Siraj Razick
QA Contact: Telepathy bugs list
URL: http://cgit.collabora.com/git/user/si...
Whiteboard:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2011-12-20 09:53 UTC by Siraj Razick
Modified: 2012-01-02 05:07 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments

Description Siraj Razick 2011-12-20 09:53:32 UTC
Wocky has some build errors when cross-compiling with mingw32. This bug is to fix that.
Comment 1 Siraj Razick 2011-12-20 09:54:48 UTC
First patch with build fixes
Comment 2 Jonny Lamb 2011-12-21 07:06:25 UTC
It seems the nicest way to check the OS in a GLib app/library is to use something like:

#ifdef G_OS_WIN32
  foo;
#else
  bar;
#endif

wocky-openssl.c already does this. For consistency could you do this too please? I'm not sure you need the AC_CHECK_HEADERS then.

+#if defined (__WIN32__)

^ same thing.

The rest looks reasonable.
Comment 3 Siraj Razick 2011-12-21 09:42:57 UTC
Hi:) Thanks for the review

Patch is updated as a single commit in a new branch mingw32 now.
Comment 4 Jonny Lamb 2011-12-22 05:44:38 UTC
(In reply to comment #3)
> Patch is updated as a single commit in a new branch mingw32 now.

Actually I did mean that you should use #ifdef G_OS_WIN32 ... #else ... #endif with the linux stuff in the #else as these are mutually exclusive, so instead of:

> +#include <config.h>
> +
> +#ifdef HAVE_SYS_SOCKET_H
>  #include <sys/socket.h>
>  #include <netinet/in.h>
>  #include <arpa/inet.h>
> +#endif
> +
> +#ifdef G_OS_WIN32
> +#include <windows.h>
> +#include <ws2tcpip.h>
> +#endif

this:

#include <config.h>

#ifdef G_OS_WIN32
#include <windows.h>
#include <ws2tcpip.h>
#else
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#endif

We've already depended on these header files being present so I think it's fine to assume they're there without the #ifdef HAVE_SYS_SOCKET_H.
Comment 5 Siraj Razick 2011-12-22 09:41:39 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > Patch is updated as a single commit in a new branch mingw32 now.
> 
> Actually I did mean that you should use #ifdef G_OS_WIN32 ... #else ... #endif
> with the linux stuff in the #else as these are mutually exclusive, so instead
> of:
> 
> > +#include <config.h>
> > +
> > +#ifdef HAVE_SYS_SOCKET_H
> >  #include <sys/socket.h>
> >  #include <netinet/in.h>
> >  #include <arpa/inet.h>
> > +#endif
> > +
> > +#ifdef G_OS_WIN32
> > +#include <windows.h>
> > +#include <ws2tcpip.h>
> > +#endif
> 
> this:
> 
> #include <config.h>
> 
> #ifdef G_OS_WIN32
> #include <windows.h>
> #include <ws2tcpip.h>
> #else
> #include <sys/socket.h>
> #include <netinet/in.h>
> #include <arpa/inet.h>
> #endif
> 
> We've already depended on these header files being present so I think it's fine
> to assume they're there without the #ifdef HAVE_SYS_SOCKET_H.
HI ..

patch updated, how does it look now :) ?

BR
siraj
Comment 6 Jonny Lamb 2011-12-22 09:46:51 UTC
(In reply to comment #5)
> patch updated, how does it look now :) ?

Oh, sorry I completely missed this in the first review:

+ #ifdef G_OS_WIN32
+   u_long mode = 0;
+   WSAEventSelect( csock, 0, 0);
+   ioctlsocket (csock, FIONBIO, &mode);
+ #else
    flags = fcntl (csock, F_GETFL );
    flags = flags & ~O_NONBLOCK;
    fcntl (csock, F_SETFL, flags);
+ #endif

Two things:

 1. the opening bracket is in the wrong place in: WSAEventSelect( csock, 0, 0). Actually it looks like it's in the wrong place for fnctl too; could you fix that too while you're there? :-)
 2. you should declare "u_long mode" earlier in the function so you don't have declarations after method calls. It might not be strictly necessary for the mingw32 compiler but it's the C90 style we're using throughout.

Thanks, otherwise it looks fine.
Comment 7 Siraj Razick 2011-12-30 10:15:43 UTC
(In reply to comment #6)
> (In reply to comment #5)
> > patch updated, how does it look now :) ?
> 
> Oh, sorry I completely missed this in the first review:
> 
> + #ifdef G_OS_WIN32
> +   u_long mode = 0;
> +   WSAEventSelect( csock, 0, 0);
> +   ioctlsocket (csock, FIONBIO, &mode);
> + #else
>     flags = fcntl (csock, F_GETFL );
>     flags = flags & ~O_NONBLOCK;
>     fcntl (csock, F_SETFL, flags);
> + #endif
> 
> Two things:
> 
>  1. the opening bracket is in the wrong place in: WSAEventSelect( csock, 0, 0).
> Actually it looks like it's in the wrong place for fnctl too; could you fix
> that too while you're there? :-)
>  2. you should declare "u_long mode" earlier in the function so you don't have
> declarations after method calls. It might not be strictly necessary for the
> mingw32 compiler but it's the C90 style we're using throughout.
> 
> Thanks, otherwise it looks fine.

Patch updated :)
Comment 8 Jonny Lamb 2012-01-02 05:07:42 UTC
(In reply to comment #7)
> Patch updated :)

It would be cool if, next time, you could check whether patches add extra whitespace at the end of lines or remove lines unnecessarily.

Thanks, pushed upstream.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.