From: | Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> |
---|---|
To: | Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> |
Cc: | Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Signed vs. Unsigned (some) |
Date: | 2021-06-15 10:38:57 |
Message-ID: | CAEudQAr4qTQVNX3NFFy-9nAxCbGqwAZsVfLjjU_pyKyrRx0k9g@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi Kyotaro,
Thanks for taking a look.
Em ter., 15 de jun. de 2021 às 05:17, Kyotaro Horiguchi <
horikyota(dot)ntt(at)gmail(dot)com> escreveu:
> At Fri, 11 Jun 2021 23:05:29 -0300, Ranier Vilela <ranier(dot)vf(at)gmail(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Ranier Vilela | 2021-06-15 10:40:46 | Re: [PATCH] Fix buffer not null terminated on (ecpg lib) |
Previous Message | tsunakawa.takay@fujitsu.com | 2021-06-15 09:51:07 | RE: Transactions involving multiple postgres foreign servers, take 2 |