Glynn Clements wrote:
Markus Metz wrote:
[...] An integer literal without a
trailing suffix is an "int". Using a larger type to hold the value
where necessary is a gcc extension. Other compilers may simply
truncate the value to an int, even if off_t is 64 bits.
The latest change:
#define OFF_T_TEST 0x0102030405060708LL
will fail to compile with a compiler which doesn't have long long int.
Both the definition and use of OFF_T_TEST need to be conditionalised
with "#ifdef HAVE_LONG_LONG_INT".
OK. But on Linux 32 without LFS I get
port_init.c:217: warning: overflow in implicit constant conversion
at compile time because long long int is available but sizeof(off_t) =
4. Portability and its test is ok because OFF_T_TEST is not used in this
case.
Making the conversion explicit would eliminate the warning:
u_o = (off_t) OFF_T_TEST;
OK.
[...]
A 64-bit system might have a 64-bit off_t
but no "long long" type. We really need to have autoconf use define
SIZEOF_LONG etc in config.h (AC_CHECK_SIZEOF macro), so that we can
use e.g.:
#if SIZEOF_LONG == 8
#define LONG_LONG_TEST 0x0102030405060708L
#elif defined(HAVE_LONG_LONG_INT)
#define LONG_LONG_TEST 0x0102030405060708LL
#endif
You can take it from gdal.
I still see:
719: s[top].sn.branch[j].child.id =
720: (int)s[top].childpos[j];
754: s[top].sn.branch[j].child.id =
755: (int)s[top].childpos[j];
Also:
1011: if (!shcb((int)s[top].sn.branch[i].child, cbarg)) {
A narrowing cast always brings up the question of "what if the value
won't fit in the destination type"?
Am I missing something, or is childpos[j] *always* an "int"?
On level 0, it is always an int. It's the ID of the rectangle passed to
int RTreeInsertRect(struct Rect *r, int tid, struct RTree *t)
as tid. That applies to all three cases above.
I am writing out the rectangle ID as off_t although it is int and have
to read it as off_t then cast it to int again when loading the sidx file
to memory for updating. Of course I could also write the ID as int and
read it as int, then there would be no type casting, at the cost of one
more if ... else ... .
If so,
why is it stored in the file as an off_t?
On higher levels (RTree internal nodes), it is the position in file of
the child nodes of a node. For large sidx files, off_t is needed. The
corresponding line in spindex_rw.c is
668 s[top].pos[s[top].branch_id - 1] = nextfreepos;
where I set the node positions in file as off_t.
This is related to the use of union Child: on level 0, int id is used,
on higher levels, struct Node *ptr is used.
So it's a case of simplifying the sidx file I/O?
Yes, partly my laziness. Took me ages to get that particular post-order
traversal right.
That's reasonable enough. But, personally, I would:
1. Make the pos and childpos fields unions (int/off_t).
That's now easy, similar to the union in rtree.
2. Add an explicit range check before casting the off_t read from the
file to an int.
That should only be necessary if there is reason to suspect that the
sidx file is not read properly. Hmm, actually that would be a good check
for exactly that.
An assert() would suffice; the main purpose is to
answer the "what if it doesn't fit in an int?" question for the
benefit of anyone trying to read that code.
Only works if off_t_size = 8 is both available and used, otherwise it
will obviously always fit into an int.
The answer would be that the programmer wrote bad code with bugs because
it should never happen that it doesn't fit into an int.
Markus M
_______________________________________________
grass-dev mailing list
[email protected]
http://lists.osgeo.org/mailman/listinfo/grass-dev
