Alvaro Herrera <alvhe...@2ndquadrant.com> writes: >> When multiplying variables, the overflow will take place anyway, and only >> then will the meaningless product be explicitly promoted to type int64. >> It is one of the operands that should have been cast instead to avoid the >> overflow. >> >> - if (state->availMem < (int64) ((newmemtupsize - memtupsize) * >> sizeof(SortTuple))) >> + if (state->availMem < ((int64) (newmemtupsize - memtupsize) * >> sizeof(SortTuple)))
> Doesn't sizeof() return a 64-bit wide value already? Not on 32-bit machines. However, on a 32-bit machine the clamp just above here would prevent overflow anyway. In general, said clamp ensures that the value computed here is less than MaxAllocHugeSize, so computing it in size_t width is enough. So in fact an overflow is impossible here, but it requires looking at more than this one line of code to see it. I would expect a static analyzer to understand it though. I think the actual point of this cast is to ensure that the comparison to availMem is done in signed not unsigned arithmetic --- which is critical because availMem might be negative. The proposed change would indeed break that, since multiplying a signed value by size_t is presumably going to produce an unsigned value. We could use two casts, but I don't see the point. regards, tom lane