[GRASS-dev] diglib test.c fails on 64bit win

Hi,

test in diglib fails (trunk) on MS Windows 64bit

landa@GEO1 /osgeo4w/usr/src/grass_trunk/lib/vector/diglib
$ make
if [ "" != "" -a -f "".html ] ; then make html ; fi
==============TEST=============
make test
make[1]: Entering directory `/osgeo4w/usr/src/grass_trunk/lib/vector/diglib'
diff OBJ.i686-pc-mingw32/test.tmp test64.ok
Files OBJ.i686-pc-mingw32/test.tmp and test64.ok differ
make[1]: *** [test] Error 2
make[1]: Leaving directory `/osgeo4w/usr/src/grass_trunk/lib/vector/diglib'
make: *** [default] Error 2

Not sure how to fix it. Any pointer welcomed. Martin

--
Martin Landa <landa.martin gmail.com> * http://gama.fsv.cvut.cz/~landa

On Mon, Apr 12, 2010 at 8:40 PM, Martin Landa <landa.martin@gmail.com> wrote:

Hi,

test in diglib fails (trunk) on MS Windows 64bit

landa@GEO1 /osgeo4w/usr/src/grass_trunk/lib/vector/diglib
$ make
if [ "" != "" -a -f "".html ] ; then make html ; fi
==============TEST=============
make test
make[1]: Entering directory `/osgeo4w/usr/src/grass_trunk/lib/vector/diglib'
diff OBJ.i686-pc-mingw32/test.tmp test64.ok
Files OBJ.i686-pc-mingw32/test.tmp and test64.ok differ
make[1]: *** [test] Error 2
make[1]: Leaving directory `/osgeo4w/usr/src/grass_trunk/lib/vector/diglib'
make: *** [default] Error 2

Not sure how to fix it. Any pointer welcomed. Martin

Do you have LFS active?
http://trac.osgeo.org/grass/ticket/930#comment:1

Markus

Hi,

2010/4/12 Markus Neteler <neteler@osgeo.org>:

Do you have LFS active?
http://trac.osgeo.org/grass/ticket/930#comment:1

yes, using

https://svn.osgeo.org/grass/grass/trunk/mswindows/osgeo4w/package.sh

Martin

--
Martin Landa <landa.martin gmail.com> * http://gama.fsv.cvut.cz/~landa

Martin Landa wrote:

> Do you have LFS active?
> http://trac.osgeo.org/grass/ticket/930#comment:1

yes, using

https://svn.osgeo.org/grass/grass/trunk/mswindows/osgeo4w/package.sh

That should be fixed, then. On Windows, --enable-largefile has no
effect other than breaking the diglib test.

--
Glynn Clements <glynn@gclements.plus.com>

Hi,

2010/4/13 Martin Landa <landa.martin@gmail.com>:

2010/4/12 Glynn Clements <glynn@gclements.plus.com>:

That should be fixed, then. On Windows, --enable-largefile has no
effect other than breaking the diglib test.

then probably diglib test should be disabled on 64bit Windows machines
or better to be fixed.

any chance to fix this issue?

Thanks, Martin

--
Martin Landa <landa.martin gmail.com> * http://gama.fsv.cvut.cz/~landa

On Wed, May 12, 2010 at 7:27 AM, Martin Landa <landa.martin@gmail.com> wrote:

Hi,

2010/4/13 Martin Landa <landa.martin@gmail.com>:

2010/4/12 Glynn Clements <glynn@gclements.plus.com>:

That should be fixed, then. On Windows, --enable-largefile has no
effect other than breaking the diglib test.

then probably diglib test should be disabled on 64bit Windows machines
or better to be fixed.

any chance to fix this issue?

The root of the problem is that LFS is enabled although LFS is not
available. This is done by configure, which defines USE_LARGEFILES
globally which in turn sets LFS_CFLAGS = -D_FILE_OFFSET_BITS=64 in
Grass.make. If USE_LARGEFILES would not be defined if LFS is not
available, the diglib test would be passed. Maybe it is a good thing
to have this test not only for portability, but also for testing if
LFS is actually working.

Does

#ifdef __MINGW32__
#define off_t off64_t
#define fseeko fseeko64
#define ftello ftello64
#endif

in config.h have any effect?

Markus M

Markus Metz wrote:

#ifdef __MINGW32__
#define off_t off64_t
#define fseeko fseeko64
#define ftello ftello64
#endif

in config.h have any effect?

If you do that, you will also need to fix lseek() and stat().

Also, are there any cases where we pass an off_t to a third-party
library? Because that would require the library to use a 64-bit off_t
(this is one of the reasons why _FILE_OFFSET_BITS=64 has to be
explicitly enabled).

--
Glynn Clements <glynn@gclements.plus.com>

On Wed, May 12, 2010 at 1:11 PM, Glynn Clements
<glynn@gclements.plus.com> wrote:

Markus Metz wrote:

#ifdef __MINGW32__
#define off_t off64_t
#define fseeko fseeko64
#define ftello ftello64
#endif

in config.h have any effect?

If you do that, you will also need to fix lseek() and stat().

I found that in config.h of trunk, generated from config.h.in, so I
wondered about its purpose and effect.

Also, are there any cases where we pass an off_t to a third-party
library? Because that would require the library to use a 64-bit off_t
(this is one of the reasons why _FILE_OFFSET_BITS=64 has to be
explicitly enabled).

But even with _FILE_OFFSET_BITS=64, off_t in trunk is 32 bit when
compiled with MING32, right?

Markus Metz wrote:

> Also, are there any cases where we pass an off_t to a third-party
> library? Because that would require the library to use a 64-bit off_t
> (this is one of the reasons why _FILE_OFFSET_BITS=64 has to be
> explicitly enabled).

But even with _FILE_OFFSET_BITS=64, off_t in trunk is 32 bit when
compiled with MING32, right?

Right. MinGW doesn't use _FILE_OFFSET_BITS and doesn't have any
equivalent, i.e. there is no built-in mechanism to "redirect" off_t,
lseek, stat etc to their 64-bit equivalents.

--
Glynn Clements <glynn@gclements.plus.com>

Glynn Clements wrote:

Markus Metz wrote:

> Also, are there any cases where we pass an off_t to a third-party
> library? Because that would require the library to use a 64-bit off_t
> (this is one of the reasons why _FILE_OFFSET_BITS=64 has to be
> explicitly enabled).

But even with _FILE_OFFSET_BITS=64, off_t in trunk is 32 bit when
compiled with MING32, right?

Right. MinGW doesn't use _FILE_OFFSET_BITS and doesn't have any
equivalent, i.e. there is no built-in mechanism to "redirect" off_t,
lseek, stat etc to their 64-bit equivalents.

In summary, it seems that grass libraries have no access to a 64-bit
off_t when compiled in MINGW32, also with ./configure
--enable-largefile.

To avoid confusion, on MINGW32 it would be better to not use
_FILE_OFFSET_BITS=64 and don't set USE_LARGEFILES and HAVE_LARGEFILES
even if LFS is requested? Unset/undefine somewhere in configure.in or
a bit later in config.h.in USE_LARGEFILES and HAVE_LARGEFILES for
MINGW32?

Markus Metz wrote:

To avoid confusion, on MINGW32 it would be better to not use
_FILE_OFFSET_BITS=64 and don't set USE_LARGEFILES and HAVE_LARGEFILES
even if LFS is requested? Unset/undefine somewhere in configure.in or
a bit later in config.h.in USE_LARGEFILES and HAVE_LARGEFILES for
MINGW32?

How about:

if test $ac_cv_largefiles = yes; then
  USE_LARGEFILES=1
+ if test "$MINGW32" = yes ; then
+ AC_MSG_ERROR([*** --enable-largefile doesn't work on Windows])
+ fi
fi

?

--
Glynn Clements <glynn@gclements.plus.com>

On Fri, May 14, 2010 at 8:31 PM, Glynn Clements
<glynn@gclements.plus.com> wrote:

Markus Metz wrote:

To avoid confusion, on MINGW32 it would be better to not use
_FILE_OFFSET_BITS=64 and don't set USE_LARGEFILES and HAVE_LARGEFILES
even if LFS is requested? Unset/undefine somewhere in configure.in or
a bit later in config.h.in USE_LARGEFILES and HAVE_LARGEFILES for
MINGW32?

How about:

if test $ac_cv_largefiles = yes; then
USE_LARGEFILES=1
+ if test "$MINGW32" = yes ; then
+ AC_MSG_ERROR([*** --enable-largefile doesn't work on Windows])
+ fi
fi

I dunnknonothin about AC_MSG_ERROR, so I would do

if test $ac_cv_largefiles = yes; then
       USE_LARGEFILES=1
+ if test "$MINGW32" = yes ; then
+ USE_LARGEFILES=0
+ fi
fi

but that is your turf, you can judge what is portable. I guess, in the
near future, MINGW64 will become more popular which is IMHO good and
grass should consider this build environment.

Markus Metz wrote:

>> To avoid confusion, on MINGW32 it would be better to not use
>> _FILE_OFFSET_BITS=64 and don't set USE_LARGEFILES and HAVE_LARGEFILES
>> even if LFS is requested? Unset/undefine somewhere in configure.in or
>> a bit later in config.h.in USE_LARGEFILES and HAVE_LARGEFILES for
>> MINGW32?
>
> How about:
>
> if test $ac_cv_largefiles = yes; then
> USE_LARGEFILES=1
> + if test "$MINGW32" = yes ; then
> + AC_MSG_ERROR([*** --enable-largefile doesn't work on Windows])
> + fi
> fi

I dunnknonothin about AC_MSG_ERROR,

It prints the specified message then aborts the configure script.

so I would do

> if test $ac_cv_largefiles = yes; then
> USE_LARGEFILES=1
> + if test "$MINGW32" = yes ; then
> + USE_LARGEFILES=0
> + fi
> fi

This *silently* disables LFS. I would prefer to tell the user that LFS
isn't available.

but that is your turf, you can judge what is portable. I guess, in the
near future, MINGW64 will become more popular which is IMHO good and
grass should consider this build environment.

I wouldn't assume that Win64 will be any different with regard to LFS.
Apart from anything else, "long" is still only 32 bits on Win64.

--
Glynn Clements <glynn@gclements.plus.com>

Glynn Clements wrote:

Markus Metz wrote:

I would do

> if test $ac_cv_largefiles = yes; then
> USE_LARGEFILES=1
> + if test "$MINGW32" = yes ; then
> + USE_LARGEFILES=0
> + fi
> fi

This *silently* disables LFS. I would prefer to tell the user that LFS
isn't available.

OK

but that is your turf, you can judge what is portable. I guess, in the
near future, MINGW64 will become more popular which is IMHO good and
grass should consider this build environment.

I wouldn't assume that Win64 will be any different with regard to LFS.
Apart from anything else, "long" is still only 32 bits on Win64.

I think there is one of int64, int64_t, _int64, _int64_t available if
a 64-bit integer type is really needed.

I did some testing on w32 with mingw32, and

#ifdef __MINGW32__
#define off_t off64_t
#define fseeko fseeko64
#define ftello ftello64
#endif

provided me with a 64-bit off_t. In LibGw32C, there are a number of 64
variants of standard functions: stat, fstat, seek, fseek, tell, ftell
are available as 64 variants as e.g. stat64. Somewhere there is also a
lseek64. What about fopen/fopen64 etc?

In GRASS, config.h does not know about __MINGW32__ even if __MINGW32__
is (somewhere else) defined, thus off_t is not redefined for mingw32,
and I placed the redefinitions for testing in another header. Looks
like a lot of hacking to get LFS for wingrass.

Markus Metz wrote:

>> but that is your turf, you can judge what is portable. I guess, in the
>> near future, MINGW64 will become more popular which is IMHO good and
>> grass should consider this build environment.
>
> I wouldn't assume that Win64 will be any different with regard to LFS.
> Apart from anything else, "long" is still only 32 bits on Win64.

I think there is one of int64, int64_t, _int64, _int64_t available if
a 64-bit integer type is really needed.

Yes; those are also available in Win32.

My point is that Win64 isn't especially different from Win32. Binary
compatibility is much more of an issue on Windows than on Unix, hence
the decision to use a 32-bit "long" on Win64.

I did some testing on w32 with mingw32, and

#ifdef __MINGW32__
#define off_t off64_t
#define fseeko fseeko64
#define ftello ftello64
#endif

provided me with a 64-bit off_t. In LibGw32C, there are a number of 64
variants of standard functions: stat, fstat, seek, fseek, tell, ftell
are available as 64 variants as e.g. stat64. Somewhere there is also a
lseek64.

All of the necessary 64-bit versions exist in MSVCRT. The issue is
that the headers don't have a "use 64-bit offsets everywhere" macro
like _FILE_OFFSET_BITS. If you want to redefine off_t to off64_t, you
have to manually redirect *everything* which uses off_t to the
appropriate 64-bit versions.

What about fopen/fopen64 etc?

There isn't an open64() or fopen64(). Unlike Unix, MSVCRT's open() and
fopen() functions don't care about the size of the file.

Unix requires the program to assert that it is "large file aware" by
using open64() or the O_LARGEFILE flag, otherwise it will refuse to
open large files. This prevents the situation where a legacy program
starts working on a large file then suddenly fails when it reaches the
2GiB mark (either due to EOVERFLOW or due to calculated offsets
wrapping).

In GRASS, config.h does not know about __MINGW32__ even if __MINGW32__
is (somewhere else) defined, thus off_t is not redefined for mingw32,
and I placed the redefinitions for testing in another header. Looks
like a lot of hacking to get LFS for wingrass.

__MINGW32__ is defined by the compiler.

But note that the "#ifdef __MINGW32__" block is within a
"#ifdef USE_LARGEFILES" block, and USE_LARGEFILES isn't defined as a
preprocessor macro (AC_DEFINE), only as a substitution (AC_SUBST), so
all of that LFS stuff at the bottom of config.h is ignored.

--
Glynn Clements <glynn@gclements.plus.com>

Glynn Clements wrote:

Markus Metz wrote:

In GRASS, config.h does not know about __MINGW32__ even if __MINGW32__
is (somewhere else) defined, thus off_t is not redefined for mingw32,
and I placed the redefinitions for testing in another header. Looks
like a lot of hacking to get LFS for wingrass.

__MINGW32__ is defined by the compiler.

But note that the "#ifdef __MINGW32__" block is within a
"#ifdef USE_LARGEFILES" block, and USE_LARGEFILES isn't defined as a
preprocessor macro (AC_DEFINE), only as a substitution (AC_SUBST), so
all of that LFS stuff at the bottom of config.h is ignored.

Interesting. IOW, "#ifdef USE_LARGEFILES" in config.h.in is
(unintentionally?) used to disable all that LFS stuff. That also means
that the IOSTREAM library always uses fseek and never fseeko because
HAVE_LARGEFILES is not defined. IIUC, the largefile test in configure
is passed for MINGW32 because fseeko64 and ftello64 exist,
USE_LARGEFILES is defined, LFS flags are set in Grass.make, but LFS is
not available because these explicit 64bit versions are never
activated. Wouldn't it make more sense to sync configure.in and
config.h.in? I think that LFS in grass also for mingw32 would be
highly appreciated by Windows users.

Markus Metz wrote:

>> In GRASS, config.h does not know about __MINGW32__ even if __MINGW32__
>> is (somewhere else) defined, thus off_t is not redefined for mingw32,
>> and I placed the redefinitions for testing in another header. Looks
>> like a lot of hacking to get LFS for wingrass.
>
> __MINGW32__ is defined by the compiler.
>
> But note that the "#ifdef __MINGW32__" block is within a
> "#ifdef USE_LARGEFILES" block, and USE_LARGEFILES isn't defined as a
> preprocessor macro (AC_DEFINE), only as a substitution (AC_SUBST), so
> all of that LFS stuff at the bottom of config.h is ignored.
>
Interesting. IOW, "#ifdef USE_LARGEFILES" in config.h.in is
(unintentionally?) used to disable all that LFS stuff.

Yes. I suspect that it was copied verbatim from cdrtools (that's where
the LFS stuff came from) without entirely understanding it.

That also means
that the IOSTREAM library always uses fseek and never fseeko because
HAVE_LARGEFILES is not defined. IIUC, the largefile test in configure
is passed for MINGW32 because fseeko64 and ftello64 exist,
USE_LARGEFILES is defined, LFS flags are set in Grass.make, but LFS is
not available because these explicit 64bit versions are never
activated. Wouldn't it make more sense to sync configure.in and
config.h.in? I think that LFS in grass also for mingw32 would be
highly appreciated by Windows users.

In 6.x, LFS is enabled for specific libraries or modules using e.g.:

  ifneq ($(USE_LARGEFILES),)
    EXTRA_CFLAGS = -D_FILE_OFFSET_BITS=64
  endif

in the corresponding Makefile. This needs to be done on a case-by-case
basis because not all modules are LFS-aware, e.g. because they
calculate offsets using "int" or "long" arithmetic, or use
fseek/ftell.

In 7.x, LFS is enabled globally by the following in Grass.make:

  ifdef USE_LARGEFILES
  LFS_CFLAGS = -D_FILE_OFFSET_BITS=64
  endif

POSIX defines 3 feature-test macros:

   _LARGEFILE_SOURCE
   _LARGEFILE64_SOURCE
   _FILE_OFFSET_BITS

Defining _LARGEFILE_SOURCE makes fseeko() and ftello() available.
Defining _LARGEFILE64_SOURCE makes the 64-bit functions (open64(),
lseek64(), etc) available. Defining _FILE_OFFSET_BITS=64 does both of
the above and also "redirects" the standard names to their 64-bit
equivalents.

MinGW doesn't have any of these. Instead, the 64-bit names are visible
unless __NO_MINGW_LFS is defined. There are no fseeko/ftello functions
(only fseeko64/ftello64), and no equivalent of _FILE_OFFSET_BITS.

--
Glynn Clements <glynn@gclements.plus.com>

Glynn Clements wrote:

Markus Metz wrote:

In 6.x, LFS is enabled for specific libraries or modules using e.g.:

   ifneq \($\(USE\_LARGEFILES\),\)
           EXTRA\_CFLAGS = \-D\_FILE\_OFFSET\_BITS=64
   endif

in the corresponding Makefile. This needs to be done on a case-by-case
basis because not all modules are LFS-aware, e.g. because they
calculate offsets using "int" or "long" arithmetic, or use
fseek/ftell.

IOW, for 6.x and mingw32, each module would need to decide if it wants
to use e.g. fseeko64 or fseek.

In 7.x, LFS is enabled globally by the following in Grass.make:

   ifdef USE\_LARGEFILES
   LFS\_CFLAGS = \-D\_FILE\_OFFSET\_BITS=64
   endif

I know, I've put that there initially for vector LFS in r37943, you
recycled that in r40620 for global LFS.

POSIX defines 3 feature-test macros:

_LARGEFILE_SOURCE
_LARGEFILE64_SOURCE
_FILE_OFFSET_BITS

[snip]

MinGW doesn't have any of these. Instead, the 64-bit names are visible
unless __NO_MINGW_LFS is defined. There are no fseeko/ftello functions
(only fseeko64/ftello64), and no equivalent of _FILE_OFFSET_BITS.

So, how about

--- config.h.in (revision 42291)
+++ config.h.in (working copy)
@@ -259,8 +259,6 @@
/*
  * Defines needed to get large file support - from cdrtools-2.01
  */
-#ifdef USE_LARGEFILES
-
#undef HAVE_LARGEFILES

#ifdef HAVE_LARGEFILES /* If we have working largefiles at all */
@@ -279,13 +277,18 @@
         /* defined, we have fseeko() */

#ifdef __MINGW32__
+/* add/remove as needed */
#define off_t off64_t
#define fseeko fseeko64
#define ftello ftello64
+#define lseek lseek64
+#define tell tell64
+#define seek seek64
+#define stat stat64
+#define fstat fstat64
#endif

#endif /* HAVE_LARGEFILES */
-#endif /* USE_LARGEFILES */

/* define if langinfo.h exists */
#undef HAVE_LANGINFO_H

?

Markus Metz wrote:

> In 6.x, LFS is enabled for specific libraries or modules using e.g.:
>
> ifneq ($(USE_LARGEFILES),)
> EXTRA_CFLAGS = -D_FILE_OFFSET_BITS=64
> endif
>
> in the corresponding Makefile. This needs to be done on a case-by-case
> basis because not all modules are LFS-aware, e.g. because they
> calculate offsets using "int" or "long" arithmetic, or use
> fseek/ftell.

IOW, for 6.x and mingw32, each module would need to decide if it wants
to use e.g. fseeko64 or fseek.

We don't use fseeko() directly. In 6.x, modules which use fseek() are
built without LFS; in 7.0, G_fseek() is used.

So, how about

#ifdef __MINGW32__

#if defined(__MINGW32__) && defined(_FILE_OFFSET_BITS) && _FILE_OFFSET_BITS == 64

+/* add/remove as needed */
#define off_t off64_t
#define fseeko fseeko64
#define ftello ftello64
+#define lseek lseek64
+#define tell tell64
+#define seek seek64
+#define stat stat64
+#define fstat fstat64

If you redirect stat() to stat64(), you also need to redirect
"struct stat" to "struct __stat64". But I'm not sure how to go about
that.

Also, there are two 64-bit stat interfaces, stati64() and
"struct _stati64", and stat64() and "struct __stat64". AFAICT, the
latter requires MSVCRT >= 6.1 (and uses __time64_t instead of time_t
for the timestamps).

--
Glynn Clements <glynn@gclements.plus.com>

Glynn wrote:

We don't use fseeko() directly. In 6.x, modules which use
fseek() are built without LFS; in 7.0, G_fseek() is used.

fwiw r.terraflow does, see include/iostream/ami_stream.h
(wrapped in a #ifdef HAVE_LARGEFILES)

Hamish