# HG changeset patch # User mas01cr # Date 1229017863 0 # Node ID 223eda8408e1ad7be57541759e67401d548010de # Parent ad2206c249865edfa9aec3f1e261fa1999d8cb66 Fix the last compiler warning. Bah. A 26-line comment to explain a one-line change will not do any good for the code deletion metric of productivity. However, I have learnt something today: specifically, the arithmetic type conversion rules in C. Wow. I had been beginning to think that C programming wasn't so bad after all: simple, clean semantics, and very little going on under the hood; nothing there to help the programmer, but nothing much to actively get in the way. And then I spend an hour on this. diff -r ad2206c24986 -r 223eda8408e1 insert.cpp --- a/insert.cpp Thu Dec 11 08:54:06 2008 +0000 +++ b/insert.cpp Thu Dec 11 17:51:03 2008 +0000 @@ -276,7 +276,33 @@ if(fstat(fd, &st)) { goto error; } - if((st.st_size - sizeof(uint32_t)) != (size / datum->dim)) { + /* This cast is so non-trivial that it deserves a comment. + * + * The data types in this expression, left to right, are: off_t, + * size_t, off_t, uint32_t. The rules for conversions in + * arithmetic expressions with mixtures of integral types are + * essentially that the widest type wins, with unsigned types + * winning on a tie-break. + * + * Because we are enforcing (through the use of sufficient + * compiler flags, if necessary) that off_t be a (signed) 64-bit + * type, the only variability in this set of types is in fact the + * size_t. On 32-bit machines, size_t is uint32_t and so the + * coercions on both sides of the equality end up promoting + * everything to int64_t, which is fine. On 64-bit machines, + * however, the left hand side is promoted to a uint64_t, while + * the right hand side remains int64_t. + * + * The mixture of signed and unsigned types in comparisons is Evil + * Bad and Wrong, and gcc complains about it. (It's right to do + * so, actually). Of course in this case it will never matter + * because of the particular relationships between all of these + * numbers, so we just cast the left hand side to off_t, which + * will do the right thing for us on all platforms. + * + * I hate C. + */ + if(((off_t) (st.st_size - sizeof(uint32_t))) != (size / datum->dim)) { goto error; } read_or_goto_error(fd, &dim, sizeof(uint32_t));