[GRASS-dev] native WinGRASS and attribute data deadlock, next try

[Glynn Clements]

Moritz Lennert wrote:

> >> So, where should we go from here ? Is it still wiser to implement 
> >> readn/writen as you suggest above ?
> > 
> > This is where it gets awkward.
> 
> Reading on below it actually seems quite straightforward. ;-)
> 
> > GRASS uses XDR/RPC for two purposes: libgis uses it for reading and 
> > writing FP raster maps, and DBMI uses it for communication between
> > the client and driver.
> 
> So we should probably thoroughly test libgis' uses of it in windows. 

I don't think that this is necessary. AFAICT, stdio works okay on
files; it's only on pipes where it misbehaves.

Actually, libgis uses xdrmem_create() to create a memory stream; it
doesn't use xdrstdio at all. So ignore what I said about removing the
buffering being a performance hit for raster I/O.

> What would be the best way to do this (i.e. in which 
> circumstances/modules is the xdr part of libgis called) ?

XDR is used for reading and writing FP maps.

> > My preferred approach would be to change lib/db/dbmi_base to simply 
> > not use XDR (that isn't anywhere near as much work as it might
> > sound).
> > 
> > As the driver and client always run on the same system, it doesn't 
> > matter if the protocol is platform-dependent (I have no idea what 
> > Radim was thinking when he decided to use XDR for the DBMI 
> > communication).
> 
> I always had the same question, but (in my ignorance) I thought that it
> might make a difference if the actual database was on a different
> system.

No; the driver does the actual database I/O. XDR is only used for the
communication between the client and the driver.

[The vector library has its own "portable" I/O routines for reading
and writing vector files, which are entirely unrelated to XDR or
DBMI.]

> If it doesn't, then let's drop xdr for DBMI (we do really need
> it for libgis, I suppose ?).

The libgis raster I/O functions use it to ensure that the format of FP
rasters isn't platform-dependent (it uses its own encoding/decoding
routines for integers).

Having said that, if you only care about platforms which use IEEE-754
format for floating-point (which is every common architecture except
for VAX), the only thing that XDR actually does is to convert 32- and
64-bit quantities to big-endian.

The only thing that using XDR really gets us is the ability to have FP
maps which are portable between VAX and non-VAX systems.

> > The dbmi_base library uses the following functions from XDR:
> > 
> > xdr_char xdr_double xdr_float xdr_int xdr_short
> > 
> > xdr_string
> > 
> > xdrstdio_create
> > 
> > The first five all simply read/write the specified value in a fixed 
> > (i.e. non-platform-dependent) format; i.e. convert to big-endian 
> > order, and convert FP values to IEEE format. For DBMI, we can just
> > use the host's native format, so the first five all amount to calling
> >  read/write on the value.
> > 
> > xdr_string is slightly more complex: read/write the length (including
> >  the terminating NUL) as an unsigned int, followed by the bytes.
> > 
> > xdrstdio_create just sets everything up so that the read/write 
> > operations go through fread/fwrite (as opposed to e.g. xdrmem_create 
> > which sets up for reading/writing from memory).
> > 
> > IOW, the actual implementation of an XDR replacement is trivial. So 
> > trivial that you would just inline most of it into the 
> > db__{send,recv}_* functions.
> > 
> > It's changing the dbmi_base library to use it which will be most of 
> > the work. You would probably want separate put/get functions rather 
> > than the XDR mechanism of setting the "direction" when creating the 
> > XDR object and having a single function for both read and write.
> > 
> > E.g. db__send_int() would change from:
> > 
> > int db__send_int(int n) { XDR xdrs; int stat;  stat = DB_OK;  
> > xdr_begin_send (&xdrs); if(!xdr_int (&xdrs, &n)) stat =
> > DB_PROTOCOL_ERR; xdr_end_send (&xdrs);  if (stat == DB_PROTOCOL_ERR) 
> > db_protocol_error(); return stat; }
> > 
> > to something like:
> > 
> > int db__send_int(int n) { int stat = DB_OK;  if (!db__send(&n,
> > sizeof(n))) stat = DB_PROTOCOL_ERR;  if (stat == DB_PROTOCOL_ERR) 
> > db_protocol_error(); return stat; }
> > 
> > with db__send() defined as:
> > 
> > int db__send(void *buf, size_t size) { return writen(_send_fd, buf,
> > size) == size; }
> > 
> > [Actually, you would probably inline writen() here, as this is the 
> > only place it would be used.]
> 
> I will need a while understanding this, especially understanding which 
> files need replacement.
> 
> Currently we have the following files in dbmi_base (ls -1 xdr*):
> 
> xdr.h
> xdr.c
> xdrchar.c
> xdrcolumn.c
> xdrdatetime.c
> xdrdouble.c
> xdrfloat.c
> xdrhandle.c
> xdrindex.c
> xdrint.c
> xdrprocedure.c
> xdrshort.c
> xdrstring.c
> xdrtable.c
> xdrtoken.c
> xdrvalue.c
> 
> 
> of these only the following reference xdr.h ( grep -l xdr.h *.c)
> 
> xdr.c
> xdrchar.c
> xdrdouble.c
> xdrfloat.c
> xdrint.c
> xdrprocedure.c
> xdrshort.c
> xdrstring.c
> 
> 
> xdrchar.c, xdrdouble.c, xdrfloat.c, xdrint.c, xdrshort.c, xdrstring.c 
> contain the functions you explain how to change above. The only place 
> where they are called is in the macros.h in the same directory. I 
> imagine that xdr.c and xdrprocedure.c can just go to the bin.

xdrprocedure.c can stay. db__start_procedure_call() uses the DB_*
macros rather than XDR. db__recv_procnum() is almost the same as
db__recv_int(), except that it passes the success/failure code back to
the caller rather than processing it itself.

[When the client terminates the communication, the driver will
typically be waiting for db__recv_procnum() to return the next request
from the client, so in this specific case EOF needs to be treated as a
termination request rather than as an error. Getting EOF anywhere else
is an error.]

xdr.c would be where db__send() and db__recv() go in place of the
xdr_{begin,end}_{send,recv} functions..

> So "all" we need to do is create new files containing the functions 
> called in macros.h according to the model you describe above.

I would be inclined to modify the existing files to use the new
db__{send,recv} functions in place of XDR functions. There's no harm
in leaving the "xdr" prefix in their names for now (once we move to
SVN, they can be renamed without losing the change history).

> What about the other xdr* files which do not actually directly use any 
> xdr calls (xdrhandle.c, xdrindex.c, xdrtable.c, xdrtoken.c, xdrvalue.c). 
> Should we just leave them as they are ? Should we rename them to avoid 
> future confusion ?

Leave them alone.

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