[GRASS-dev] Re: About the vector changes

[Glynn Clements]

Markus Metz wrote:

> >>> Ah, ok, you use the grass57 dataset. I have now changed the version 
> >>> number and compatibility info, you should now get reasonable 
> >>> warnings/errors with the spearfish57 dataset.
> >>>       
> >> Okay, I'll check it.
> >>     
> >
> > Nope. As of r38644, the error message is unchanged.
> 
> I got the order of the checks wrong. First check must be version check, 
> only after that comes the check for file size. That LFS check is there 
> to check if sizeof(off_t) = 8 is needed to read the sidx file. Fixed in 
> r36845.

Yep; it now says:

	ERROR: Spatial index format version 5.0 is not supported by this release.
	       Please rebuild topology.

However, as a result of the LFS changes, the code has some type
mismatches on 32-bit systems with LFS enabled.

Most of these are due to passing off_t's to G_debug() with %ld as the
format (it needs to be %lld for a 64-bit off_t on a 32-bit platform;
we really need a macro for this). There's also one G_fatal_error()
call which has "%d".

These shouldn't actually cause any problems beyond reporting bogus
offsets in debug/error messages. On little-endian platforms, the
offset will be truncated to 32 bits (so it has no effect for offsets
<2GiB). It's more of an issue on big-endian platforms, as you get the
high word, so offsets will be divided by 4GiB (i.e.they'll normally be
zero).

In lib/vector/diglib/port_init.c,

	#define OFF_T_TEST 0x0102030405060708

should have an LL suffix (it's too large for an "int"), and this
should ideally be conditionalised (although I don't know if we have a
macro for "off_t is 64-bits"; _FILE_OFFSET_BITS may not be defined on
64-bit platforms, and you can't use sizeof in #if tests). I believe
that the compiler is free to truncate the above to an int, regardless
of what it's assigned to. If that happens, the vector code will fail
entirely.

Finally:

spindex_rw.c:657: warning: cast from pointer to integer of different size

spindex_rw.c:715: warning: cast to pointer from integer of different size
spindex_rw.c:746: warning: cast to pointer from integer of different size

The first one is less of an issue; a platform with 64-bit pointers
will probably have a 64-bit off_t. The converse isn't true; on a
32-bit system with LFS, storing a 64-bit off_t in a 32-bit pointer
isn't going to work.

The lines in question are:

656:		if (s[top].sn->level == 0)	/* leaf node */
657:		    s[top].pos[j] = (off_t) s[top].sn->branch[j].child;

714:	    s[top].sn.branch[j].child =
715:		(struct Node *)s[top].childpos[j];

745:			    s[top].sn.branch[j].child =
746:				(struct Node *)s[top].childpos[j];

ISTR that it should only cast childpos[j] to a pointer if that is what
was stored there in the first place. But I would strongly suggest
using a union rather than casting; we shouldn't expect people to
simply ignore compiler warnings, and it's unreasonable to expect
anyone to analyse the code to the extent that they can determine that
the warnings are harmless.

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