Re: Signed vs. Unsigned (some)

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

In response to

Browse pgsql-hackers by date

  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