#469: raster data needs binary mode on windows
-------------------------+--------------------------------------------------
Reporter: jef | Owner: grass-dev@lists.osgeo.org
Type: defect | Status: new
Priority: major | Milestone: 6.4.0
Component: default | Version: 6.4.0 RCs
Keywords: | Platform: MSWindows XP
Cpu: Unspecified |
-------------------------+--------------------------------------------------
I compiled the GRASS plugin for GDAL for OSGeo4W and tried to use it from
QGIS (MinGW build GRASS used from MSVC built QGIS, GDAL plugin and GDAL).
GRASS fails to recognize for instance slope from spearfish60 as valid
raster. This seems to be because the fcell file is not opened in binary
mode. The attached patch changes that.
#469: raster data needs binary mode on windows
---------------------------+------------------------------------------------
Reporter: jef | Owner: grass-dev@lists.osgeo.org
Type: defect | Status: new
Priority: major | Milestone: 6.4.0
Component: default | Version: 6.4.0 RCs
Resolution: | Keywords:
Platform: MSWindows XP | Cpu: Unspecified
---------------------------+------------------------------------------------
Comment (by pkelly):
This seems like some sort of compile problem. There is an object file
fmode.o that is supposed to be linked into everything compiled on Windows
to force files to be opened in binary mode. Perhaps that is not getting
included with the OSGeo4W compile?
#469: raster data needs binary mode on windows
---------------------------+------------------------------------------------
Reporter: jef | Owner: grass-dev@lists.osgeo.org
Type: defect | Status: new
Priority: major | Milestone: 6.4.0
Component: default | Version: 6.4.0 RCs
Resolution: | Keywords:
Platform: MSWindows XP | Cpu: Unspecified
---------------------------+------------------------------------------------
Comment (by jef):
Replying to [comment:1 pkelly]:
> This seems like some sort of compile problem. There is an object file
fmode.o that is supposed to be linked into everything compiled on Windows
to force files to be opened in binary mode. Perhaps that is not getting
included with the OSGeo4W compile?
Ah, ok. I just linked against the necessary DLLs, but not fmode.o. I'll
give that a try. Thanks
#469: raster data needs binary mode on windows
---------------------------+------------------------------------------------
Reporter: jef | Owner: grass-dev@lists.osgeo.org
Type: defect | Status: new
Priority: major | Milestone: 6.4.0
Component: default | Version: 6.4.0 RCs
Resolution: | Keywords:
Platform: MSWindows XP | Cpu: Unspecified
---------------------------+------------------------------------------------
Comment (by warmerdam):
Dear grass team. I would humbly submit that making anyone who wants to
use the grass libraries also link with fmode.o is risky and that it would
be safer to actually enforce binary behavior at the open() call(s). In
other packages my practice has been to always pass O_BINARY to open(), and
to predefine it to zero if it isn't already defined.
#469: raster data needs binary mode on windows
---------------------------+------------------------------------------------
Reporter: jef | Owner: grass-dev@lists.osgeo.org
Type: defect | Status: new
Priority: major | Milestone: 6.4.0
Component: default | Version: 6.4.0 RCs
Resolution: | Keywords:
Platform: MSWindows XP | Cpu: Unspecified
---------------------------+------------------------------------------------
Comment (by jef):
Replying to [comment:2 jef]:
> Ah, ok. I just linked against the necessary DLLs, but not fmode.o.
I'll give that a try. Thanks
Doesn't do the trick for me. I'm probably not using the same runtime
library the GRASS DLLs use and therefore cannot reach the right _fmode.
But I think the better approch would be to have the DLLs open in the right
mode anyway instead of requiring to mess with the defaults, which might
break other things.
#469: raster data needs binary mode on windows
---------------------------+------------------------------------------------
Reporter: jef | Owner: grass-dev@lists.osgeo.org
Type: defect | Status: new
Priority: major | Milestone: 6.4.0
Component: default | Version: 6.4.0 RCs
Resolution: | Keywords:
Platform: MSWindows XP | Cpu: Unspecified
---------------------------+------------------------------------------------
Comment (by glynn):
Replying to [comment:3 warmerdam]:
> Dear grass team. I would humbly submit that making anyone who wants to
use the grass libraries also link with fmode.o is risky and that it would
be safer to actually enforce binary behavior at the open() call(s). In
other packages my practice has been to always pass O_BINARY to open(), and
to predefine it to zero if it isn't already defined.
Note: setting _fmode is the only way to get stdin/stdout into binary mode
(we don't care about stderr). At least, this used to be the case. It
appears that later versions of MSVCRT require that you explicitly change
the mode of these descriptors. However, MinGW always links against
msvcrt.dll, not a later version.
BTW, I count 52 occurrences of open() or open64() in GRASS, including 6 in
libgis and 6 in other libraries.
I don't know how this affects other functions which create descriptors,
e.g. pipe(), dup2(), etc. Putting "_fmode = O_BINARY" into gisinit() may
suffice for everything except stdin/stdout; that would be preferable to
requiring each case to be handled explicitly.
#469: raster data needs binary mode on windows
---------------------------+------------------------------------------------
Reporter: jef | Owner: grass-dev@lists.osgeo.org
Type: defect | Status: new
Priority: major | Milestone: 6.4.0
Component: default | Version: 6.4.0 RCs
Resolution: | Keywords:
Platform: MSWindows XP | Cpu: Unspecified
---------------------------+------------------------------------------------
Comment (by jef):
Replying to [comment:5 glynn]:
> Note: setting _fmode is the only way to get stdin/stdout into binary
mode (we don't care about stderr). At least, this used to be the case. It
appears that later versions of MSVCRT require that you explicitly change
the mode of these descriptors. However, MinGW always links against
msvcrt.dll, not a later version.
Ok, I adapted fmode.c to dllmain.c and include it in every dll. _fmode is
set when the DLL is loaded. That also helps - and might fix more than the
raster problem addressed by the original patch - which I replaced.
#469: raster data needs binary mode on windows
---------------------------+------------------------------------------------
Reporter: jef | Owner: grass-dev@lists.osgeo.org
Type: defect | Status: new
Priority: major | Milestone: 6.4.0
Component: default | Version: 6.4.0 RCs
Resolution: | Keywords:
Platform: MSWindows XP | Cpu: Unspecified
---------------------------+------------------------------------------------
Comment (by glynn):
Replying to [comment:7 neteler]:
> @grass-dev: is the new patch "patch_for_469.2.diff" ok?
Rather than using "dllmain.dat" to prevent compilation on MinGW, you can
call it dllmain.c and exclude it from compilation with:
{{{
MOD_OBJS := $(filter-out dllmain.o,$(AUTO_OBJS))
}}}
Hmm; we should do this for fmode.o as well.
#469: raster data needs binary mode on windows
---------------------------+------------------------------------------------
Reporter: jef | Owner: grass-dev@lists.osgeo.org
Type: defect | Status: new
Priority: major | Milestone: 6.4.0
Component: default | Version: 6.4.0 RCs
Resolution: | Keywords:
Platform: MSWindows XP | Cpu: Unspecified
---------------------------+------------------------------------------------
Comment (by glynn):
Replying to [comment:12 neteler]:
> The lib/gis/ patch still fails to compile on my Linux box:
>
{{{
dllmain.c:1:21: error: windows.h: No such file or directory
}}}
>
> as it doesn't exclude dllmain.c from compilation.
I think that you need to modify ARCH_LIB_OBJS (which has the $(OBJDIR)/
prefix added) on 6.x. That uses eager assignment (:=), so its value is set
based upon the value of $(LIB_OBJS) at the point that Lib.make is
included; subsequent changes to LIB_OBJS won't have any effect.
7.0 added an extra lazy assignment step to make it easier to override the
list of object files.
#469: raster data needs binary mode on windows
---------------------------+------------------------------------------------
Reporter: jef | Owner: grass-dev@lists.osgeo.org
Type: defect | Status: new
Priority: major | Milestone: 6.4.0
Component: default | Version: 6.4.0 RCs
Resolution: | Keywords:
Platform: MSWindows XP | Cpu: Unspecified
---------------------------+------------------------------------------------
Comment (by glynn):
Replying to [comment:13 glynn]:
> I think that you need to modify ARCH_LIB_OBJS (which has the $(OBJDIR)/
prefix added) on 6.x. That uses eager assignment (:=), so its value is set
based upon the value of $(LIB_OBJS) at the point that Lib.make is
included; subsequent changes to LIB_OBJS won't have any effect.
Scratch that; it won't work in 6.x. The only way to override the list of
object files is to fully define LIB_OBJS '''before''' Lib.make is
included, e.g.:
{{{
LIB_OBJS := $(subst .c,.o,$(wildcard *.c))
LIB_OBJS := $(filter-out fmode.o dllmain.o,$(LIB_OBJS))
}}}
> 7.0 added an extra lazy assignment step to make it easier to override
the list of object files.
It also separated out the variable definitions from the rules, so that you
can override variables after including Vars.make but before including e.g.
Lib.make.
6.x doesn't have this; you can't get at the variables before the rules
which use them are defined. So, you can't modify any variables which are
used in dependency lines, as those are expanded as they are read. You can
override variables which are used in commands, as those aren't expanded
until the commands are executed.