Andres Freund <andres@anarazel.de> writes:
> Use new overflow aware integer operations.
I just tried to compile manually on prairiedog for the first time since
this went in, and noticed that it now produces a boatload of warnings
like
int.c: In function 'int2pl':
int.c:735: warning: 'result' may be used uninitialized in this function
I think it's entirely within its rights to complain, because
the non-builtin code paths in int.h look like
int32 res = (int32) a + (int32) b;
if (res > PG_INT16_MAX || res < PG_INT16_MIN)
return true;
*result = (int16) res;
return false;
and so if an overflow occurs then *result won't get set.
I do not think it is reasonable for these functions to not set the
output variable at all in the overflow case; it is not their job
to opine on whether the caller may use the result. Furthermore,
this is an incorrect emulation of the built-in functions;
the gcc manual clearly says
These built-in functions promote the first two operands into
infinite precision signed type and perform addition on those
promoted operands. The result is then cast to the type the third
pointer argument points to and stored there. If the stored result
is equal to the infinite precision result, the built-in functions
return false, otherwise they return true.
So as coded, there is a difference of behavior depending on whether
you have the built-in, and that cannot be anything but bad.
Therefore I think all of these need a patch like
int32 res = (int32) a + (int32) b;
+ *result = (int16) res;
if (res > PG_INT16_MAX || res < PG_INT16_MIN)
return true;
- *result = (int16) res;
return false;
regards, tom lane
