Hi Kyotaro, Thanks for taking a look.
Em ter., 15 de jun. de 2021 às 05:17, Kyotaro Horiguchi < horikyota....@gmail.com> escreveu: > At Fri, 11 Jun 2021 23:05:29 -0300, Ranier Vilela <ranier...@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. > Ok. best regards, Ranier Vilela
