Bug 43986

First patch with build fixes

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.

Hi:) Thanks for the review

Patch is updated as a single commit in a new branch mingw32 now.

(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.

(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

(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.

(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 :)

(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.