Re: [gnome-db] gda-parameter-list.c:300: uint + mmap

[Brecht Sanders <brecht sanders org>]

> On 1/19/07, Brecht Sanders <brecht sanders org> wrote:
> > > > I am trying to compile libgda-2.99.2 for win32 using the mingw/msys
> > > > compiler environment.
> > > > I got pretty far into the compilation, but one of the pitfalls was this:
> > > >
> > > > The file libgda/gda-parameter-list.c has the following on lines 300 and
> > > > 304:
> > > >    gda_value_set_ushort (value, va_arg (ap, uint));
> > > >    g_value_set_uchar (value, va_arg (ap, uint));
> > > > I was able to get by this problem with a simple "-Duint=guint"
> > > > Apparently the uint type does not exist on my platform, but guint doe
> > > > and glib defines it simply as unsigned int.
> > >
> > > It should normally be unsigned short (I'll correct this), does it
> > > exist on Windows?
> >
> > Yes.
>
> Ok.
>
> >
> > > > The show stopper I am facing though is in libgda/gda-data-model-import.c
> > > > which uses sys/mman.h for memory mapped file access.
> > > > The win32 platform does not have this include file.
> > > > Is there any chance this code could be changed to be win32 compatible?
> > >
> > > Windows does have mapped files, but the code is different, AFAIK it
> > > uses OpenFileMapping function call. Modifications to the code should
> > > not be very difficult (modifications should be included in #ifdef
> > > LIBGDA_WIN32 sections).
> > >
> > > However because I know nothing of Win32 programming and I don't have a
> > > win32 box at hand, I won't code it myself (you'll find some examples
> > > fo such code for instance in http://www.google.com/codesearch).
> >
> > In that case I'll be glad to make my contribution. Below is the patch I have
> > made.
> > Note that I have moved munmap after close. Doesn't that seem more logical?
>
> Yes, right.

Ok, well the attached patch already puts munmap after close.

> >
> > I haven't tested it yet though.
> > Can you tell me what the easiest way is to test it?
>
> As there is not (yet) regression tests in libgda, you'll have to write
> your own code, or if you have libgnomedb compiled, then you can run
> "test-dyn-widgets -t 14" in the testing/ directory and try to load a
> CVS file for example.

My plan was to stay away from anything graphical in my attempt to port libgda to
win32.
Do you think it's feasible to get libgnomedb to compile on win32?
Do you know if anyone has actually done some porting before to win32 using mingw?

> > However I think I need to be able to build the entire library before I can
> > test anything...
>
> That'd be better...
>
> >
> > My next problem is strtok_r which doesn't exist on win32.
> > I believe strtok is thread safe.
>
> the strtok_r() is nt about thread safeness, but it avoids to use a
> static buffer (which can lead to problems if several strings are to be
> splitted at the same time).

Does that mean it would be best to supply code for strtok_r for platforms that
don't have it?

> > If it is I guess adding some code like this woudl fix it:
> >     #ifdef _WIN32
> >       #define strtok_r(s,d,p) strtok(s,d)
> >     #endif
> > What would be the best place to add this? I was thinking libgda/gda-decl.h?
>
> This should work, but code analysis is required here to make sure that
> there are never 2 strings being splitted at the same time (putting
> this define in libgda/gda-decl.h seems ok for me).
>
> Could you send me tha patch you made as an attachment instead, to keep
> the formatting correct (avoid long lines truncation, etc)?

Ok, see attachment.

Thanks
    Brecht

--- libgda/gda-data-model-import.c.orig  Fri Jan 19 11:17:28 2007
+++ libgda/gda-data-model-import.c.w32  Fri Jan 19 13:42:03 2007
@@ -22,7 +22,11 @@
 
 #include "gda-data-model-import.h"
 #include <unistd.h>
-#include <sys/mman.h>
+#ifndef _WIN32
+  #include <sys/mman.h>
+#else
+  #include <windows.h>
+#endif
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
@@ -314,15 +318,19 @@
 
     /* data access mem free */
     if (model->priv->is_mapped) {
+      if (model->priv->src.mapped.start) {
+#ifndef _WIN32
+        munmap (model->priv->src.mapped.start, model->priv->src.mapped.length);
+#else
+        UnmapViewOfFile (model->priv->src.mapped.start);
+#endif
+        model->priv->src.mapped.start = NULL;
+      }
       g_free (model->priv->src.mapped.filename);
       if (model->priv->src.mapped.fd >= 0) {
         close (model->priv->src.mapped.fd);
         model->priv->src.mapped.fd = -1;
       }
-      if (model->priv->src.mapped.start) {
-        munmap (model->priv->src.mapped.start, model->priv->src.mapped.length);
-        model->priv->src.mapped.start = NULL;
-      }
     }
     else {
       g_free (model->priv->src.string);
@@ -476,7 +484,7 @@
       model->priv->src.mapped.fd = open (model->priv->src.mapped.filename, O_RDONLY);
       if (model->priv->src.mapped.fd < 0) {
         /* error */
-        add_error (model, sys_errlist [errno]);
+        add_error (model, sys_errlist[errno]);
         return;
       }
       else {
@@ -489,6 +497,7 @@
           return;
         }
         model->priv->src.mapped.length = _stat.st_size;
+#ifndef _WIN32
         model->priv->src.mapped.start = mmap (NULL, model->priv->src.mapped.length,
                       PROT_READ, MAP_PRIVATE, 
                       model->priv->src.mapped.fd, 0);
@@ -498,6 +507,20 @@
           model->priv->src.mapped.start = NULL;
           return;
         }
+#else
+        HANDLE view = CreateFileMapping((HANDLE)model->priv->src.mapped.fd, NULL, PAGE_READONLY|SEC_COMMIT, 0,0 , NULL);
+        if (!view) {
+          /* error */
+          add_error (model, sys_errlist [errno]);
+          return;
+        }
+        model->priv->src.mapped.start = MapViewOfFile(view, FILE_MAP_READ, 0, 0, model->priv->src.mapped.length);
+        if (!model->priv->src.mapped.start) {
+          /* error */
+          add_error (model, sys_errlist [errno]);
+          return;
+        }
+#endif
         model->priv->data_start = model->priv->src.mapped.start;
         model->priv->data_length = model->priv->src.mapped.length;
       }