[GRASS-dev] Re: About the vector changes
markus.metz.giswork at googlemail.com
Sun Aug 9 12:00:14 EDT 2009
> 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".
There was something about long long int, it's part of the C99 standard
but not supported by all compilers if I remember correctly. That's the
reason for HAVE_LONG_LONG_INT?
> 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
> 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
Its usage is conditionalised: it's only used if sizeof(off_t) == 8,
otherwise OFF_T_TEST is ignored and LONG_TEST is used instead. Applies
to port_test.c as well. The current portability test is passed on 32bit
> (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
> 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.
In line 746, I was stuffing an off_t value that is smaller than INT_MAX
into a pointer. There was an integer stored initially.
> But I would strongly suggest
> using a union rather than casting;
You suggested that earlier already. First time I work with unions, done
in trunk r38654. Could please have a look at lib/vector/rtree/index.h?
The compiler warnings above about pointer <-> integer casts are gone,
but I'm not sure if I used that union thing correctly.
Picking your brain, unrelated: can I assume that DBL_MAX is always
available on all platforms? I would like to use it in lib/vector/rtree.
> 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.
Agreed. Any volunteers out there who want to analyse the rtree code?
That would actually be very welcome...
More information about the grass-dev