MSVC Portability

[Elliott Sales de Andrade]

Hi,

So, I just want to go over those changes to compile with VS. FYI, it
would be better to send a patch file instead of a list of changes like
this.

In general, I'm in favour of better portability, but I think there are
some better ways to go about it. We should be able to simplify things
by using GLib constructs and letting them take care of the hard parts.

> // I list here which parts of the code had to be changed in libpurple
> // to compile with Visual Studio Express 2012.
> // I won't provide line numbers because I haven't looked into the
> // latest source, and finding the problems is easy anyway. The
> // ifdef/ifndef _MSC_VER was added by me, together with the code in
> // the ifdef block, or the else block of ifndef.
>
> libpurple/plugin.c:
> // Just look for !RUNNING_ON_VALGRIND. It always evaluates
> // to true when not using Valgrind.
> #ifndef _MSC_VER
>         if (!g_getenv("PURPLE_LEAKCHECK_HELP") && !RUNNING_ON_VALGRIND)
> #else
>         if (!g_getenv("PURPLE_LEAKCHECK_HELP"))
> #endif

Maybe you should find where RUNNING_ON_VALGRIND is defined and set it
correctly there. Seems like the more relevant location.

> // This is added after all the includes. (I'm not familiar with the C syntax,
> // just guessed this is what the replacement should be in VS)
> // Replace the gg_debug_dcc define with:
> #ifndef _MSC_VER
> #define gg_debug_dcc(dcc, level, fmt...) \
>     gg_debug_session(((dcc) != NULL) ? (dcc)->sess : NULL, level, fmt)
> #else
> #define gg_debug_dcc(dcc, level, ...) \
>     gg_debug_session(((dcc) != NULL) ? (dcc)->sess : NULL, level, __VA_ARGS__)
> #endif

I don't think the variadic macro syntax is too portable and it appears
to be C99. Unless we are changing our C requirements, then I think GG
should try to remove that.

> libpurple/protocols/gg/lib/libgadu.h:
> // Add at the top right after #define __GG_LIBGADU_H
> #ifdef _MSC_VER
> #include "config.h"
> #include "internal.h"
> #define strcasecmp stricmp
> #define strncasecmp strnicmp
> #endif

Assuming all the requirements are met, then these calls should be
entirely replaced by the GLib equivalents: g_ascii_strcasecmp and
g_ascii_strncasecmp.

> // VS complained that empty structs and unions are not valid C.
> // I haven't found references to this struct in the whole source.
> // If it is added as some kind of packing this can potentially break
> // things, though GG_PACKED when not compiled under gcc is an empty define.
> #ifndef _MSC_VER
> struct gg_dcc7_dunno1 {
>     // XXX
> } GG_PACKED;
> #endif

Seems useless and can probably be removed entirely.

> libpurple/protocols/bonjour/mdns_common.h:
> // Add at the top:
> #ifdef _MSC_VER
> #include "config.h"
> #endif
> libpurple/protocols/irc/irc.h:
> // Add to the top:
> #ifdef _MSC_VER
> #include "config.h"
> #endif

Why?

> libpurple/plugins/ssl/ssl-nss.c:
> // Add after the includes. I copied this out from a different source
> // file in libpurple, so hopefully this is what it should be.
> #ifdef _MSC_VER
> #ifndef _SSIZE_T_DEFINED
> #ifdef  _WIN64
> typedef __int64    ssize_t;
> #else
> typedef _W64 int   ssize_t;
> #endif
> #define _SSIZE_T_DEFINED
> #endif
> #endif

I don't seem to see that code existing in libpurple. I'm not sure it's
right either, as ssize_t could be 32-bit depending on the platform
(though it may always be 64-bit on Windows, I don't know).

> libpurple/protocols/mxit/protocol.c:
> // Add after the includes.
> #ifdef _MSC_VER
> #include "cipher.h"
> #define strtoll _strtoi64;
> #endif

Again, should use GLib's g_ascii_strtoll. (Plus, you shouldn't have a
semicolon in a #define.) Also, why would you only need cipher.h on
MSVC?

> // In the function dump_bytes, replace the char msg declaration to:
> #ifndef _MSC_VER
>     char        msg[( len * 3 ) + 1];
> #else
>     char *msg = (char*)malloc(sizeof(char) * ((len * 3)  + 1));
> #endif
> // Add to the end of the same function:
> #ifdef _MSC_VER
>     free(msg);
> #endif
>
> // Same in mxit_write_http_post function:
> #ifndef _MSC_VER
>     char        request[256 + packet->datalen];
> #else
>     char *request = (char*)malloc(sizeof(char) * (256 + packet->datalen));
> #endif
> // Add to the end of the same function:
> #ifdef _MSC_VER
>     free(request);
> #endif
>
> // Look for the mxit_send_packet function. I added the next code on
> // the 16th line after the /* socket connection */  comment.
> // Replace the declaration of char data with:
> #ifndef _MSC_VER
>         char        data[packet->datalen + packet->headerlen];
> #else
>         char *data = (char*)malloc(sizeof(char) * (packet->datalen + packet->headerlen));
> #endif
> // Add to the end of the if block:
> #ifdef _MSC_VER
>         free(data);
> #endif

This looks like a C99-ism or GNU extension and should be removed in
all cases. But you should use g_malloc and g_free instead. The mxit
guys do like to use g_alloca though.

> // The following files had includes not in the SDK on Windows. Most can
> // be ifndef-ed. If something is still missing, add #include <io.h> in
> // an else block. (io.h had to be added to files marked with *)
>
> unistd.h not found in:
> libpurple/protocols/gg/lib/
>     dcc7.c
>     *sha1.c
>     resolver.c
>     pubdir.c
>     libgadu.c
>     handlers.c
>     events.c
>     dcc.c
>     common.c
>     dcc7.c (already mentioned above)
>     http.c
> inttypes.h not found in:
> libpurple/protocols/gg/
>     lib.message.h

unistd.h is included in internal.h, so the logic for whether to use
io.h should probably be in there. However, these GG files are part of
the external GG library, so I'm not sure how Tomasz wants to handle
it.

> unistd.h not found in:
> libpurple/protocols/novell/
>     *nmconn.c:

See note above.

> utsname.h not found in:
> libpurple/protocols/jabber/
>     iq.c

Seems to be for calling uname. I think the check around it should be
#ifndef instead of #ifdef. The rest of the code seems unimplemented,
so that's probably why no one noticed (so the #include could probably
be commented out entirely even.)

--
Elliott