Re: [Evolution-hackers] e-d-s/libedataserver portability patch 1

[Not Zed <notzed ximian com>]

First look ... why change the code quoted below?  It works doens't it?
It seems like a patch just for the sake of changing the code style.

The same goes for changing blank lines - as much as i hate lines of
whitespace.  Do you have some sort of auto-formatting of existing code
in your editor?  Please turn it off. (as much as it sounds anal,
changing white-space is listed in the patch guidelines as a no no).
(don't worry about the white-space in this patch, just try to avoid it
in the future).

On Fri, 2005-05-20 at 12:45 +0300, Tor Lillqvist wrote:
> -#ifdef G_THREADS_ENABLED
> -static GStaticMutex lock = G_STATIC_MUTEX_INIT;
> -#define LOCK() g_static_mutex_lock(&lock)
> -#define UNLOCK() g_static_mutex_unlock(&lock)
> -#else
> -#define LOCK()
> -#define UNLOCK()
> -#endif
> +G_LOCK_DEFINE_STATIC(lock); 


-#include <glib/gfileutils.h>
-#include <glib/gmem.h>
-#include <glib/gmessages.h>
-#include <glib/gstrfuncs.h>
-#include <glib/gunicode.h>
-#include <glib/gutils.h>
-#include <glib/galloca.h>
-#include <glib/gconvert.h>
+#include <glib.h>
+#include <glib/gstdio.h>
+

Dont do this either, it was done like that on purpose ...


Note that g_file_test() is not the same as access().  It will for
example fail if a path component along the way isn't readable or
writable by the calling process, although it is examinable.  Not a
completely unlikely scenario.

For the e-msgport stuff, can't you just write a new version for windows
and compile that instead?  You're going to have to change almost every
line of the code.

Also note that the cleanup stuff isn't needed - we never call
pthread_cancel because there were too many bugs in it when it was
originally written, and its almost impossible to write code to handle it
properly anyway.  So i'd say you can safely ignore/remove it.

e_mutex could (should probably) be removed, if you change its uses to
the appropriate mutex types (recursive mutexes didn't exist in glib when
it was done).


@@ -1,2 +1,7 @@
+if OS_WIN32
+NO_UNDEFINED = -no-undefined
+SOCKET_LIBS = -lws2_32
+endif
+
 INCLUDES
=                                                             \
        -I
$(top_srcdir)                                                 \
@@ -45,8 +50,9 @@ libedataserver_1_2_la_LIBADD
=                                \
        $(E_DATA_SERVER_LIBS)                           \
        $(ICONV_LIBS)                                   \
-       $(top_builddir)/libdb/dist/libdb-4.1.la
+       $(top_builddir)/libdb/dist/libdb-4.1.la         \
+       $(SOCKET_LIBS)


Can this be done at a higher-level?  e.g. in configure.in?  Otherwise
you're going to have to copy the if os_win32 stuff in every makefile.


I'm not sure this is right either, do you really want to treat / as the
dir separator if there isn't any \'s?  Shouldn't you just use
G_DIR_SEPRATOR always?

> +
> +       p = (char *) g_path_skip_root (p);
> +       do {
> +               char *p1 = strchr (p, '/');
> +#ifdef G_OS_WIN32
> +               {
> +                       char *p2 = strchr (p, '\\');
> +                       if (p1 == NULL ||
> +                           (p2 != NULL && p2 < p1))
> +                               p1 = p2;
> +               }
> +#endif
> +               p = p1;
> +               if (p)
> +                       *p = '\0';
> +               if (!g_file_test (copy, G_FILE_TEST_IS_DIR)) {
> +                       if (g_mkdir (copy, mode) == -1) {
> +                               g_free (copy);
> +                               return -1;
> +                       }
> +               }
> +               if (p) {
> +                       *p++ = G_DIR_SEPARATOR;