changeset 412:223eda8408e1 api-inversion

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.

--- 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));