At Fri, 11 Jun 2021 23:05:29 -0300, Ranier Vilela <ranier.vf@gmail.com> wrote in > Hi, > > Removing legitimate warnings can it be worth it?
From what the warning comes from? And what is the exact message?
msvc 64 bits compiler (Level4)
warning C4245: '=': conversion from 'int' to 'Bucket', signed/unsigned mismatch
> -1 CAST can be wrong, when there is an invalid value defined > (InvalidBucket, InvalidBlockNumber). > I think depending on the compiler -1 CAST may be different from > InvalidBucket or InvalidBlockNumber.
The definitions are not ((type) -1) but ((type) 0xFFFFFFFF) so actually they might be different if we forget to widen the constant when widening the types. Regarding to the compiler behavior, I think we are assuming C99[1] and C99 defines that -1 is converted to Uxxx_MAX. (6.3.1.3 Singed and unsigned integers)
I'm +0.2 on it. It might be worthwhile as a matter of style.
I think about more than style.
This is one of the tricks that should not be used.
> pg_rewind is one special case. > All cases of XLogSegNo (uint64) initialization are zero, but in pg_rewind > was used -1? > I did not find it InvalidXLogSegNo!
I'm not sure whether that is a thinko that the variable is signed or that it is intentional to assign the maximum value.
It is a thinko.
Anyway, actually there's no need for initializing the variable at all. So I don't think it's worth changing the initial value.
It is the case of removing the initialization then?
If any compiler actually complains about the assignment changing it to zero seems reasonable.
Same case.
msvc 64 bits compiler (Level4)
warning C4245: '=': initialization from 'int' to 'XLogSegNo', signed/unsigned mismatch
> Not tested. > > Trivial patch attached.
Please don't quickly update the patch responding to my comments alone. I might be a minority.