Bug 24395

Created attachment 30158 [details] [review]
patch for making tp-gabble build work on windows

Thank you for this patch. I'll try to disentangle the #ifdefs into something more maintainable...

Comment on attachment 30158 [details] [review]
patch for making tp-gabble build work on windows

Ignoring lib/gibber for now, it contains most of the non-portable networking tricks. In the 0.9 series we'll gradually use GIO (GLib 2.22) for this; if you want to get a head start on Gabble portability, make sure you can compile GLib 2.22, and submit patches to that if you can't.

As a general matter of approach, #ifdef WIN32 is rarely the right thing to do. The first option for portability should always be to use some functionality that's portable, like a GLib function. Failing that, #ifdef HAVE_SOME_FUNCTION or #ifdef HAVE_SOMETHING_H makes it clear what is going on; checking for OSs should be a last resort, and should use the G_OS_FOO macros.

(We do need to check for OS for credentials-passing - every Unix OS implements it differently, and so far we only know how on Linux, so in git master we use #ifdef __linux__.)

As another general matter of approach, Telepathy has feature discovery in many places. If you're going to make Unix sockets not work, for instance (this is entirely reasonable, because Windows doesn't have them!) then you need to make the feature-discovery function not return them as an option.

Similarly, GibberUnixTransport should simply not be compiled, unless G_OS_UNIX is defined. It doesn't make sense to try to force it to compile on non-Unix platforms.

Tubes and file transfer should be able to work even on Windows, using IPv4 sockets; we have the code for it.

>diff --git a/src/bytestream-socks5.c b/src/bytestream-socks5.c
>index f4c452b..ff1ff06 100755
>--- a/src/bytestream-socks5.c
>+++ b/src/bytestream-socks5.c
>@@ -21,23 +21,82 @@
> #include "config.h"
> #include "bytestream-socks5.h"
> 
>+#ifndef WIN32
> #include <arpa/inet.h>
> #include <errno.h>
> #include <fcntl.h>
> #include <netdb.h>
> #include <net/if.h>
> #include <netinet/in.h>
>+#endif

If these headers are non-portable, they should be checked for in configure.ac, and guarded by #ifdef HAVE_FOO_H.

From other bits of the code, it appears that Windows *does* have errno.h, so that one doesn't need guarding?

>+#include <winsock2.h>
>+#include <ws2tcpip.h>

We could check for these too.

>+   #define IFNAMSIZ         16
>+#define SIZE_INTERFACES	100
>+
>+
>+struct ifmap
>+  {
>+    unsigned long int mem_start;
>+    unsigned long int mem_end;
>+    unsigned short int base_addr;
>+    unsigned char irq;
>+    unsigned char dma;
>+    unsigned char port;
>+    /* 3 bytes spare */
>+  };
>+
>+struct ifreq 
>+  {
>+   char ifr_name[IFNAMSIZ]; /* Interface name */
>+   union {
>+       struct sockaddr ifr_addr;
>+       struct sockaddr ifr_dstaddr;
>+       struct sockaddr ifr_broadaddr;
>+       struct sockaddr ifr_netmask;
>+       struct sockaddr ifr_hwaddr;
>+       short	       ifr_flags;
>+       int	       ifr_ifindex;
>+       int	       ifr_metric;
>+       int	       ifr_mtu;
>+       struct ifmap    ifr_map;
>+       char	       ifr_slave[IFNAMSIZ];
>+       char	       ifr_newname[IFNAMSIZ];
>+       char *	       ifr_data;
>+   };
>+  };
>+
>+struct ifconf 
>+  {
>+   int		      ifc_len; /* size of buffer */
>+   union {
>+       char *	      ifc_buf; /* buffer address */
>+       struct ifreq * ifc_req; /* array of structures */
>+   };
>+  };
>+#define  SIOCGIFCONF SIO_GET_INTERFACE_LIST
>+#define  SIOCGIFFLAGS SIO_GET_INTERFACE_LIST
>+#endif

This is terrifying. Is there really no Win32 system header that can define this structure properly?

>+   
> #ifdef HAVE_GETIFADDRS
>+#ifndef WIN32
>  #include <ifaddrs.h>
> #endif
>+#endif

Does Win32 define HAVE_GETIFADDRS?

>+	if ((i = WSAStartup(MAKEWORD(1,1), &info)) < 0 ) {

Shouldn't this be called from main() or something?

>+#ifndef WIN32
>   /* Loop throught the interface list and get the IP address of each IF */
>   for (ifr = ifc.ifc_req;
>       (gchar *) ifr < (gchar *) ifc.ifc_req + ifc.ifc_len;
>@@ -1780,6 +1875,9 @@ get_local_interfaces_ips (void)
>   free (ifc.ifc_req);
> 
>   return ips;
>+#else
>+  return NULL;
>+#endif
> }

All that work just to return NULL? :-(

>diff --git a/src/ft-manager.c b/src/ft-manager.c
>index c47f97c..eaf3bf0 100755
>--- a/src/ft-manager.c
>+++ b/src/ft-manager.c
>@@ -82,6 +87,19 @@ struct _GabbleFtManagerPrivate
>   gchar *tmp_dir;
> };
> 
>+#ifdef WIN32
>+char* 
>+mkdtemp(char *templt) {
>+    // In windows we dont have the mkstemp or mkdtemp function
>+    // _mktemp is almost similar for creating unique file name,
>+    // but does not create the folder, and we dont know method for creating unique folder name
>+    FILE *f;    
>+    _mktemp(templt);
>+    mkdir(templt);
>+    return templt;
>+}

This is not a suitable implementation of mkdtemp - you're ignoring the return from mkdir, meaning you might be returning a directory that can be written by someone else.

The wider issue here is that you don't actually need to implement gabble_ft_manager_get_tmp_dir at all - the FT code uses it to make Unix sockets, and Windows can't do Unix sockets!

>--- a/src/gabble.c
>+++ b/src/gabble.c
>+#ifndef WIN32
>+ #include <unistd.h>
>+#else
>+# ifndef STDOUT_FILENO
>+#  define STDOUT_FILENO 1
>+# endif
>+# ifndef STDERR_FILENO
>+#  define STDERR_FILENO 2
>+# endif
>+#endif

This should just use HAVE_UNISTD_H. We probably don't actually need unistd.h any more here, in fact.

>@@ -84,7 +93,11 @@ stamp_log (const gchar *log_domain,

This should either just use tp_debug_timestamped_log_handler (beware that the behaviour seems to be subtly different), or have the same fix as telepathy-glib.

>--- a/src/jingle-content.c
>+++ b/src/jingle-content.c
>@@ -395,9 +395,15 @@ produce_senders (JingleContentSenders senders)
> }
> 
> 
>+#ifndef WIN32
> #define SET_BAD_REQ(txt...) g_set_error (error, GABBLE_XMPP_ERROR, XMPP_ERROR_BAD_REQUEST, txt)
> #define SET_OUT_ORDER(txt) g_set_error (error, GABBLE_XMPP_ERROR, XMPP_ERROR_JINGLE_OUT_OF_ORDER, txt)
> #define SET_CONFLICT(txt...) g_set_error (error, GABBLE_XMPP_ERROR, XMPP_ERROR_CONFLICT, txt)
>+#else
>+#define SET_BAD_REQ(...) g_set_error (error, GABBLE_XMPP_ERROR, XMPP_ERROR_BAD_REQUEST, __VA_ARGS__)
>+#define SET_OUT_ORDER(...) g_set_error (error, GABBLE_XMPP_ERROR, XMPP_ERROR_JINGLE_OUT_OF_ORDER, __VA_ARGS__)
>+#define SET_CONFLICT(...) g_set_error (error, GABBLE_XMPP_ERROR, XMPP_ERROR_CONFLICT, __VA_ARGS__)
>+#endif

This should use ##__VA_ARGS__ like we do in debug.h, probably - that's theoretically non-portable, but in practice we haven't had complaints about debug.h, so...

>--- a/src/message-util.c
>+++ b/src/message-util.c
>@@ -37,6 +37,90 @@
> #include "namespaces.h"
> #include "util.h"
> 
>+#ifdef WIN32
>+
>+// convert String to tm struct in Windows, untested
>+static int
>+_string_to_tm (char *pszTime, 
>+               struct tm * tm1)
>+{
...
>+}
>+#endif

This may well be able to use g_time_val_from_iso8601, in which case we could use it on all platforms.

>+#ifndef WIN32
>   if (credentials->uid != getuid ())
>     {
>       DEBUG ("Wrong uid (%u). Rejecting", credentials->uid);
>       goto credentials_received_cb_out;
>     }
>+#else
>+  {
>+      unsigned int puid;
>+      DWORD processID = GetCurrentProcessId();
>+      puid = processID;
>+      if (credentials->uid != puid)
>+      {
>+          DEBUG ("Wrong uid (%u). Rejecting", credentials->uid);
>+          goto credentials_received_cb_out;      
>+      }
>+  }

Will you ever receive credentials on Windows? If so, how?

Credentials transfer through sockets is not portable - it's a Unixism, and it only works on Unix sockets anyway. So, Windows code can safely assume that it can never receive correct credentials, for two reasons: (1) we can't transfer credentials through Unix sockets on Windows, and (2) we can't even *make* Unix sockets on Windows.

(Also, credentials->uid is a user ID, not a process ID, so comparing it to a process ID makes no sense.)

>   else if (priv->access_control == TP_SOCKET_ACCESS_CONTROL_CREDENTIALS)

This should never be reached on Windows, because you can't use credentials-based access control unless you can transfer credentials through a Unix socket. So, this entire elseif could safely be considered Unix-specific.

>@@ -947,9 +985,11 @@ new_connection_to_socket (GabbleTubeStream *self,
>       array = g_value_get_boxed (priv->address);
>       DEBUG ("Will try to connect to socket: %s", (const gchar *) array->data);
> 
>+#ifndef WIN32
>       transport = GIBBER_TRANSPORT (gibber_unix_transport_new ());
>       gibber_unix_transport_connect (GIBBER_UNIX_TRANSPORT (transport),
>           array->data, NULL);
>+#endif

If you're not running on Unix, you should already have rejected the Unix address types by this point.

>+#ifdef WIN32
>+static gboolean
>+check_unix_params (TpSocketAddressType address_type,
>+                   const GValue *address,
>+                   TpSocketAccessControl access_control,
>+                   const GValue *access_control_param,
>+                   GError **error)
>+{
>+   return NULL;
>+}

NULL isn't a gboolean.

>--- a/src/tubes-channel.c
>+++ b/src/tubes-channel.c
>@@ -25,7 +25,16 @@
> #include <string.h>
> #include <sys/stat.h>
> #include <sys/types.h>
>+#ifndef WIN32
> #include <unistd.h>
>+#else
>+# ifndef STDOUT_FILENO
>+#  define STDOUT_FILENO 1
>+# endif
>+# ifndef STDERR_FILENO
>+#  define STDERR_FILENO 2
>+# endif
>+#endif

This doesn't seem to be necessary? Nothing in Tubes uses these macros.

> #include <glib/gstdio.h>
> #include <dbus/dbus-glib.h>
>@@ -1426,7 +1435,9 @@ tube_msg_close (GabbleTubesChannel *self,
> 
>   close_node = lm_message_node_get_child_with_namespace (msg->node, "close",
>       NS_TUBES);
>+#ifndef WIN32
>   g_assert (close != NULL);
>+#endif

This is a fairly obvious coding error. We're asserting that the libc function close(3) has a non-NULL address, where we intended to assert that close_node was non-NULL...

>diff --git a/tools/glib-ginterface-gen.py b/tools/glib-ginterface-gen.py

This has already been fixed in telepathy-glib, and the comment at the top says "The master copy of this program is in the telepathy-glib repository - please make any changes there". I've updated it from telepathy-glib on the branch.

My 08-portability branch applies some of these changes, in what I believe to be a better way. Gabble won't build on Windows with only that branch, but it's a start. Could you try it out?

http://git.collabora.co.uk/?p=user/smcv/telepathy-gabble-smcv.git;a=shortlog;h=refs/heads/08-portability

git://git.collabora.co.uk/git/user/smcv/telepathy-gabble-smcv.git

In particular:

(In reply to comment #3)
> (We do need to check for OS for credentials-passing - every Unix OS implements
> it differently, and so far we only know how on Linux, so in git master we use
> #ifdef __linux__.)

I've backported this.

> As another general matter of approach, Telepathy has feature discovery in many
> places. If you're going to make Unix sockets not work, for instance (this is
> entirely reasonable, because Windows doesn't have them!) then you need to make
> the feature-discovery function not return them as an option.

I've fixed this correctly, including the regression tests.

> Similarly, GibberUnixTransport should simply not be compiled, unless G_OS_UNIX
> is defined. It doesn't make sense to try to force it to compile on non-Unix
> platforms.

I've done this.

> Tubes and file transfer should be able to work even on Windows, using IPv4
> sockets; we have the code for it.

This might not work, in fact, since Gibber assumes that g_io_channel_unix_new() is sufficient. Gibber needs to use g_io_channel_win32_new_socket() on Windows - this is a situation where an OS-dependent wrapper function would be useful:

static inline GIOChannel *
gibber_g_io_channel_new_socket (gint fd, gboolean is_socket G_GNUC_UNUSED)
{
#ifdef G_OS_WIN32
  if (is_socket)
    {
      return g_io_channel_win32_new_socket (fd);
    }
  else
    {
      return g_io_channel_unix_new (fd);
    }
#else
  return g_io_channel_unix_new (fd);
#endif
}

with the boolean for "is it a socket?" coming from the logic of the surrounding code.

However, it might be the case that this is considerably easier to fix by relying on gnio (GLib 2.22), which is unacceptable for the 0.8 branch but is reasonable to do on the 0.9 branch.

> If these headers are non-portable, they should be checked for in configure.ac,
> and guarded by #ifdef HAVE_FOO_H.

As a starting point, I've done this for unistd.h.

> This should either just use tp_debug_timestamped_log_handler (beware that the
> behaviour seems to be subtly different), or have the same fix as
> telepathy-glib.

In my branch, it wraps tp_debug_timestamped_log_handler in order to get the differing behaviour.

> This should use ##__VA_ARGS__ like we do in debug.h, probably - that's
> theoretically non-portable, but in practice we haven't had complaints about
> debug.h, so...

Done in the branch.

> This may well be able to use g_time_val_from_iso8601

Done in the branch.

> Will you ever receive credentials on Windows? If so, how?

Never called on Windows in the branch, which is the right approach.

> >@@ -1426,7 +1435,9 @@ tube_msg_close (GabbleTubesChannel *self,
> > 
> >   close_node = lm_message_node_get_child_with_namespace (msg->node, "close",
> >       NS_TUBES);
> >+#ifndef WIN32
> >   g_assert (close != NULL);
> >+#endif

Fixed in the branch.

> >diff --git a/tools/glib-ginterface-gen.py b/tools/glib-ginterface-gen.py
> 
> This has already been fixed in telepathy-glib

Updated in the branch.

(In reply to comment #4)

Started to study gabble 08-portability branch, and run into Loudmouth-Wocky compatibility layer, is there way to bypass wocky and use loudmouth or do we allready need to port wocky also to windows to get this branch work on windows?

(Removing patch keyword so this falls off the review queue - it's not ready for review yet, since my branch is incomplete.)

In answer to your question, Wocky is part of Gabble 0.9 for now, but will eventually be a separate library. If it has portability problems, please tell us about them sooner rather than later, so we can fix them! Run "git submodule init" and "git submodule update" to get an appropriate version of Wocky checked out.

The 0.8.x stable branch will always use Loudmouth, with no way to switch to Wocky; the 0.9.x development branch always uses Wocky, with no way to revert to Loudmouth. Wocky has enough advantages over LM that we can't support LM in future development.

Snapshot of the 08-portability branch, rebased onto the latest stable release (0.8.7): http://people.collabora.co.uk/~smcv/telepathy-gabble-0.8.7+08-portability.tar.gz

(In reply to comment #7)
Hi, sorry about delay, build the portability 08 branch, ended up guarding these headers
sys/socket.h netdb.h winsock2.h ws2tcpip.h netinet/in.h sys/un.h arpa/nameser.h resolv.h Windns.h
There was also the interface issue i mentioned in comment in tp-glib bugzilla,..
also still couldnt find these couple of structs in windows, i ended putting them in separate header file:
ifmap, ifreq, ifconf, HEADER
the arpa/nameser.h definitions also cause problems
maybe i still try to look for those structs and type defs, but it kinda looks bad
When looked at jabberd server codes I saw that they had defined some types not found in windows in separate header file, could that be done with telepathy libraries?, they had a separate win32 folder
and include folder having headers ac-stdint.h win32_port.h containing some structs and type defs for windows
i'll put new patch here when i have tested it enough.., but that may still not be pretty patch..

(In reply to comment #8)
There's probably some way to make checks about what method to use in configure.ac,
so that config.h would contain defines like:
#define fcntl ioctlsocket
#define F_SETFL FIONBIO
#define O_NONBLOCK 1
so that corresponding gibber code would not change much, howerver I currently dont know how to do that, i was thinking of making patch and config file for proposal and attach them here, and someone with better knowledge would write the configure.ac,.. for the moment i'll still try to improve the new patch..

Created attachment 30763 [details] [review]
changes to portability branch to make it work in windows

Created attachment 30764 [details] [review]
adding extra files to portability branch containing needed structs and types

(In reply to comment #9)

Hi, I'm attaching here patches against portability branch, i did not patch the configure.ac because i dont know the syntax good enough, but it probably should have some check for ioctl and fcntl and ioctlsocket functions and then when needed: AC_DEFINE(fcntl,ioctlsocket)
checks for all those headers:
AC_CHECK_HEADERS([sys/socket.h],[sys/un.h],[netdb.h],[winsock2.h],[ws2tcpip.h],[Windows.h],[Winbase.h], [netinet/in.h], [arpa/nameser.h], [resolv.h], [Windns.h])
also probably should have checks for these methods:
res_query, DnsQuery_A
When including winsock2.h the interface and ERROR keywords needs to be undef:ed.
Anyway the config.h generated by configure.ac for window should contain these definitons:
/* WIN32 STUFF */
#define HAVE_WINSOCK2_H
#define HAVE_WS2TCPIP_H
#define HAVE_DNSQUERY_A
#define HAVE_WINDNS_H

/* ioctl */
#define ioctl ioctlsocket
#define fcntl ioctlsocket
#define F_SETFL FIONBIO
#define O_NONBLOCK 1
#define EAFNOSUPPORT    WSAEAFNOSUPPORT
#define EADDRINUSE		WSAEADDRINUSE

The first patch contains modifications to existing changes to portability branch and second adds 2 files containing structs and types i failed to find in windows.

(In reply to comment #12)

That feature discovery still needs to be fixed to not give unix sockets, thats probably in the connection_disco_cb, or somewhere there..

Comment on attachment 30763 [details] [review]
changes to portability branch to make it work in windows

>+#ifdef HAVE_SYS_SOCKET_H
> #include <sys/socket.h>
>+#endif

I think a better approach to this would be to have a "meta-header" which includes all the misc socket stuff on Unix, or winsock on Windows. I've implemented this in an updated 08-portability branch which I'll push soon.

>+#ifdef HAVE_RES_QUERY

I haven't looked at the DNS stuff yet.

>diff --git a/lib/gibber/gibber-tcp-transport.c b/lib/gibber/gibber-tcp-transport.c
>index 12f98a0..c46a991 100644
>--- a/lib/gibber/gibber-tcp-transport.c
>+++ b/lib/gibber/gibber-tcp-transport.c
>@@ -38,6 +38,10 @@
> 
> #include "errno.h"
> 
>+#ifdef WIN32
>+#include "errno-win32-extend.h"
>+#endif

errno is used in one place in this file.

A cursory web search indicates that winsock doesn't return its errors in errno, so you're wrong to look in errno to check for its errors.

In any case, I think a better approach is to have a wrapper function gibber_connect_errno_requires_retry() which does the right thing on either platform (either errno or WSALastError()).

>+#ifdef GIBBER_TYPE_UNIX_TRANSPORT

This won't work as desired unless the corresponding header has already been included. I switched this check to G_OS_UNIX, with a comment explaining why.

Comment on attachment 30764 [details] [review]
adding extra files to portability branch containing needed structs and types

>diff --git a/win32/structs-for-win32.h b/win32/structs-for-win32.h
>new file mode 100755
>index 0000000..97cd917
>--- /dev/null
>+++ b/win32/structs-for-win32.h
>@@ -0,0 +1,88 @@
>+
>+//structs-for-win32.h
>+
>+#define IFNAMSIZ         16
>+//#define SIZE_INTERFACES	100
>+
>+
>+struct ifmap
>+  {

Sorry, but this is entirely wrong. Either Windows has the ioctl to list interfaces, in which case it must have some header file somewhere which describes the struct layout correctly; or it doesn't, in which case you don't need this struct at all.

When making portability changes, please consider the purpose of the code, what the author is trying to achieve, whether it even makes sense to attempt it on your platform, and (if so) what the best way would be to achieve that purpose.

In this case, the place where this struct is used is a function that lists the IP addresses of all local interfaces. The fact that, on Unix, we use particular functions for this is not actually relevant at all - what's relevant is that we want a list of IP addresses! According to a cursory web search, Winsock has a different ioctl which can be used to achieve this.

For the moment, my branch just "stubs out" this function by returning an empty list of IP addresses on Windows (Bug #24775). If you want SOCKS5 bytestreams (one of several possible transports for file transfer) to work on Windows, please provide an implementation on that bug; if you don't care about SOCKS5 bytestreams, you should just be able to leave it as it is.

This *might* make a little bit of sense to have, unfortunately, since it's the layout of a DNS header in struct form; some OSs provide it in a header, others don't. In 0.9 we can fix this by using GLib's resolver code and not trying to implement it ourselves, which makes much more sense.

>+// Bad hack: can't find these on windows
>+#define EAI_OVERFLOW -1
>+#define EAI_SYSTEM -2

That would be because they're part of the Unix API for getaddrinfo(). The fact that you can't find these error constants probably means that they're meaningless on Windows, or that there is a different way to discover the equivalent error. Again, in 0.9 we'll no longer care, because we can start using GLib's resolver, and we'll no longer need to deal with getaddrinfo().

Have you made any progress on checking that GLib 2.22 compiles and works on Windows? As a reminder, Gabble 0.9's netcode will be heavily dependent on GLib >= 2.22, so if GLib 2.22 doesn't work for you, you'll be stuck with a "dead end".

(In reply to comment #9)
> (In reply to comment #8)
> There's probably some way to make checks about what method to use in
> configure.ac,
> so that config.h would contain defines like:
> #define fcntl ioctlsocket
> #define F_SETFL FIONBIO
> #define O_NONBLOCK 1
> so that corresponding gibber code would not change much

That's not the right goal. We don't mind our code changing if it's changed in ways that (a) make us more portable, and (b) still work correctly on Unix.

The right way to do this would be to replace our calls to fcntl() with some sort of gibber_socket_make_nonblocking() function (think about the purpose, not the implementation!), and reimplement that do do a different fcntl call on Windows.

(In reply to comment #16)
> (In reply to comment #9)
> > (In reply to comment #8)
> > so that config.h would contain defines like:
> > #define fcntl ioctlsocket
> > #define F_SETFL FIONBIO
> > #define O_NONBLOCK 1

According to <http://msdn.microsoft.com/en-us/library/ms738573%28VS.85%29.aspx>, that's wrong anyway: the third argument should be a pointer to a u_long (presumably a typedef for unsigned long?) containing a nonzero value.

Try this new snapshot (I've also updated the git branch):

http://people.collabora.co.uk/~smcv/telepathy-gabble-0.8.7+portability2.tar.gz

I believe this should include or obsolete all your portability changes, except for the DNS-resolution stuff; please see how much of your patch you can delete :-)

I believe you should be able to delete basically everything, except for:

* HEADER in the extra structs file
* possibly the EAI_* constants
* the res_query vs DnsQuery_A stuff

Regarding those, I strongly suspect that HEADER and the EAI_* constants will no longer be required either, if you follow the principle of "think about the purpose, not the implementation" - if you plug in the DnsQuery call at a slightly higher level in the API, you shouldn't need these hacks.

PDNS_RECORD appears to be Windows' version of HEADER, and the DNS_STATUS returned by DnsQuery_A is probably Windows' version of EAI_*, so if you're careful about the scope of the #ifdef, you shouldn't need to define HEADER or EAI_* at all.

Also, judging by the list of functions in MSDN, you probably want to be using DnsQuery_UTF8 - GLib applications like Gabble generally expect all strings to be UTF-8.

Please use #ifdef G_OS_WIN32 as I've been doing, not #ifdef WIN32.

(In reply to comment #12)
> Anyway the config.h generated by configure.ac for window should contain these
> definitons:
> /* WIN32 STUFF */
> #define HAVE_WINSOCK2_H
> #define HAVE_WS2TCPIP_H
> #define HAVE_DNSQUERY_A
> #define HAVE_WINDNS_H
> 
> /* ioctl */
> #define ioctl ioctlsocket
> #define fcntl ioctlsocket
> #define F_SETFL FIONBIO
> #define O_NONBLOCK 1
> #define EAFNOSUPPORT    WSAEAFNOSUPPORT
> #define EADDRINUSE              WSAEADDRINUSE

You should no longer need to do any of these (or the CMake equivalents).

(In reply to comment #18)
Well, pulled your updated branch and it looks like gibber-tcp-transport is not an issue any more,
i did that "errno-win32-extend.h" because I saw similar approach being 
used somewhere else, probably its bad way but anyway,
it looks now like gibber-resolver.c and gibber-util.c are the remaining files that dont compile on windows

I think posted that patch to indicate what i had to do in order to make it compile in windows,
not as a proposal to how it should work, meant to post something to get forward with..

I actually gave a thought to using G_OS_WIN32 intead of WIN32 at wery start, and thought WIN32 works
also places where glib.h is not included, at that time did not know about any coding conventions..

In gibber-util.c we still need to get u_int32_t and u_int16_t from somewhere, it builds after adding those

The GLib 2.22 i think has windows build, that i downloaded while a go, I have a glib-dev_2.22.2-1_win32
folder, and i think I was using that when i was accidently trying to build gabble 0.9, I fetched
that packet when I did not find some method, anyway to my knowledge there's no need to compile 
GLib 2.22 on windows. However i'm not sure if it was included in the gtk win32 main bundle.. had to made
separate download.. 

Good thing you mentioned about DnsQuery_UTF8, i would not have noticed that,..

About that gibber-resolver is it needed by what features?, because we can login to jabber.se, it
obviously does not need that there,.. I'm just thinking if it could be excluded now and just
not make it not work on windows by now.. and you could propose your portability branch to review,
and fix that resolve later?

(In reply to comment #3)
Have to comment on that 
#define DEBUG(f, ...) g_debug (f , ## __VA_ARGS__)
notation, it works most of the time on windows, 
but I noticed when building
telepathy-farsight in windows that 
if you call that macro with no extra arguments:

DEBUG(format) 

the end result is syntax error : ')' 

I think its because it translates to: g_debug (format,);
and compiler in windows does not like it.
So it works as long as you call it with those extra
parameters, but not if its called without them,
but that obviously works on linux..

Right, that's a GNUism (gcc interprets the " , ## __VA_ARGS__" sequence as "remove the comma if there are no varargs").

In practice we don't very often call the macro with just a string, but if the compiler's going to be that pedantic we may have to use the elaborate scheme seen in GLib and libdbus, or add a DEBUG0() macro :-(

(In reply to comment #18)
> Try this new snapshot (I've also updated the git branch):
> 
Ok I'm attaching patch against current portability banch, let's see how many errors I can fit in a patch only 131 lines long..

Created attachment 30943 [details] [review]
Patch against current portability branch

My branch might also give us portability to FreeBSD and other assorted OSs, so it might well be worth merging even if it doesn't entirely solve the problem for Windows. Laurent Bigonville has offered to try this out.

<http://git.collabora.co.uk/?p=user/smcv/telepathy-gabble-smcv.git;a=shortlog;h=refs/heads/08-portability>

(In reply to comment #24)
> My branch might also give us portability to FreeBSD and other assorted OSs, so
> it might well be worth merging even if it doesn't entirely solve the problem
> for Windows. Laurent Bigonville has offered to try this out.
> 
> <http://git.collabora.co.uk/?p=user/smcv/telepathy-gabble-smcv.git;a=shortlog;h=refs/heads/08-portability>
> 

Build fine on kfreebsd debian devel machine

Considering this reviewable, then. Someone (not me right now I'm afraid) could forward-port it to 0.9.

Will: the branch smcv/08-portability-minimal is already applied to 0.9.x, and is sufficient to build Gabble 0.8.10 on non-Linux Unix systems (at least Debian GNU/kFreeBSD - confirmed on asdfasdf.debian.net). May I merge it to telepathy-gabble-0.8?

http://git.collabora.co.uk/?p=user/smcv/telepathy-gabble-smcv.git;a=shortlog;h=refs/heads/08-portability-minimal

The branch smcv/08-portability is a rebase of the one mentioned here previously, which fixes most of the Windows portability issues. Now that Gabble 0.8 is fairly deep-frozen, perhaps we don't want to be adding patches this intrusive... up to Will, really.

http://git.collabora.co.uk/?p=user/smcv/telepathy-gabble-smcv.git;a=shortlog;h=refs/heads/08-portability

The branch smcv/09-portability is a rebase onto master, discarding patches which are no longer applicable (we deleted quite a bit of non-portable stuff in favour of using GNIO).

http://git.collabora.co.uk/?p=user/smcv/telepathy-gabble-smcv.git;a=shortlog;h=refs/heads/09-portability

Matti, did you make any progress on compiling GLib >= 2.22, Wocky, or telepathy-gabble 0.9 on Windows? I suspect that smcv/09-portability will need a few extra patches, but it might just work as-is if you're lucky (GLib 2.22 is a prerequisite for Gabble 0.9). We stopped using GibberResolver in 0.9 in favour of using GLib's GResolver, so you'll no longer need to patch that.

(In reply to comment #27)
> the branch smcv/08-portability-minimal is already applied to 0.9.x, and
> is sufficient to build Gabble 0.8.10 on non-Linux Unix systems (at least Debian
> GNU/kFreeBSD - confirmed on asdfasdf.debian.net).

I merged that. Remaining branches are:

smcv/08-portability:

> perhaps we don't want to be adding patches this
> intrusive... up to Will, really.

smcv/09-portability: review requested,

http://git.collabora.co.uk/?p=user/smcv/telepathy-gabble-smcv.git;a=shortlog;h=refs/heads/09-portability

(I don't think testing this on Windows is a prerequisite for merging: having half the necessary changes is better than having none.)

--- a/tests/twisted/file-transfer/test-receive-file-and-sender-disconnect-while-pending.py
+++ b/tests/twisted/file-transfer/test-receive-file-and-sender-disconnect-while-pending.py
@@ -21,7 +21,8 @@ class ReceiveFileAndSenderDisconnectWhilePendingTest(ReceiveFileTest):
 
         # We can't accept the transfer now
         try:
-            self.ft_channel.AcceptFile(cs.SOCKET_ADDRESS_TYPE_UNIX,
+            # IPv4 is otherwise guaranteed to be available
+            self.ft_channel.AcceptFile(cs.SOCKET_ADDRESS_TYPE_IPV4,

s,otherwise,always, ?

+    raise SystemExit(77)    # skipped on non-unix for now

Say why?

Otherwise looks reasonable!

Merged with trivial changes