[GRASS-dev] Re: [Qgis-developer] [patch] MSVC patch (less warnings and GRASS)

[Glynn Clements]

Jürgen E. Fischer wrote:

> > E.g. the S_ISDIR() stuff suggessts that we need a G_is_directory()
> > function. Similarly, a G_rmdir() function.
> 
> lib/db/dbmi_base/isdir.c doesn't include unistd.h, so I still need to
> define S_ISDIR there as long as there is no G_is_directory()

The Linux stat(2) manpage implies that <unistd.h> should be included
for stat(), so I'll add that.

> > I'm not sure about the rewinddir() issue:
> > > Index: lib/db/dbmi_base/dirent.c
> > > Index: lib/gis/list.c
> 
> dirent.h is a wrapper I found that maps opendir/readdir/closedir to
> FindFirstFile/FindNextFile/FindClose.  It used to have no rewinddir(), which
> I've added now.

The dirent stuff is about the only actual code which is part of MinGW. 
It provides a fair amount of Unix compatibility in its headers, but it
relies upon MSVCRT for the actual functions.

> > Regarding the DBMI RPC mechanism:
> > > Index: lib/db/dbmi_base/xdr.c
> 
> I compile lib/db/dbmi_base/xdr.c without __MINGW32__ now.  I didn't check if
> fread() works, I'm not even sure the QGIS uses that at all.  I'll keep you
> posted if I run into trouble.

It's fundamental to the DBMI subsystem. The individual DB drivers
(PostgreSQL, MySQL, DBF, SQLite, ODBC) are separate processes. When a
db.* or v.* module initialises DBMI, the driver is spawned as a child
process with its stdin/stdout connected via pipes.

The various DBMI functions call the driver via a home-grown RPC
mechanism. This used to use XDR, but we ran into problems with the
xdrstdio_* not working, apparently due to fread/fwrite not working
with pipes. When the sunrpc library was hacked to use read/write, the
problem went away.

Rather than requiring a hacked sunrpc library, we just eliminated the
use of XDR. As both the client and the driver run on the same system,
there's no need for the RPC protocol to be portable.

> > > Index: lib/gis/parser.c
> > What about G_parser() is causing problems with MSVC? Obviously, we
> > can't disable that function in GRASS itself.
> 
> No, that was to avoid additional dependencies - at least I thought so.
> But replacing popen/pclose with G_popen and G_pclose is enough to get
> the DLL build with G_parser().

Right. Although that should probably be changed, I'm not especially
confident in GRASS' G_popen() implementation, and that code is
relatively important.

[FWIW, GRASS uses G_popen() in two files, lib/gis/error.c and
lib/gis/list.c, versus 27 files for popen(). Although more than half
of those are accounted for by i.ortho.photo (6 files) and (10 files).]

It's unfortunate that all of this has arisen right now, as:

a) We've recently started preparing for a 6.3.0 release, and
b) my Windows XP system is (still) out of commission due to a dead motherboard.

> So following is what's left of the "not so keen" part of the patch . The first
> part would be replaced by G_is_directory() later and the second looks ok as it
> is, I suppose.
> 
> Index: lib/db/dbmi_base/isdir.c

Hopefully this should be fixed by including <unistd.h>.

> Index: lib/gis/parser.c

I think that I'll change this to G_popen() and see how it goes.

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