From: | Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> |
---|---|
To: | Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> |
Cc: | Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Signed vs. Unsigned (some) |
Date: | 2021-06-16 11:51:16 |
Message-ID: | CAEudQAoW4qLQEywSzQj_LCQU-efZedpO5kyvCCzQ6mrq3VbPxA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Em qua., 16 de jun. de 2021 às 05:48, Peter Eisentraut <
peter(dot)eisentraut(at)enterprisedb(dot)com> escreveu:
> On 15.06.21 10:17, Kyotaro Horiguchi wrote:
> > 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 since we have the constants we should use them.
>
> >> 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. Anyway, actually
> > there's no need for initializing the variable at all. So I don't think
> > it's worth changing the initial value. If any compiler actually
> > complains about the assignment changing it to zero seems reasonable.
> >
> >> Not tested.
>
> I think this case needs some analysis and explanation what is going on.
> I agree that the existing code looks a bit fishy, but we shouldn't just
> change it to something else without understanding what is going on.
>
Yes, sure.
I think everyone agrees that they have to understand to change something.
I am acting as a firefighter for small fires.
I believe the real contribution to Postgres would be to convince them to
change the default build flags.
Last night I tested a full build on Ubuntu, with clang 10.
Surprise, no warning, all clear, with -Wall enabled (by default).
No wonder these problems end up inside the code, no one sees them.
Everyone is happy to compile Postgres and not see any warnings.
But add -Wpedantinc and -Wextra and you'll get more trouble than rabbits in
a magician's hat.
Of course most are bogus, but they are there, and the new ones, the result
of the new code that has just been modified, will not enter.
Tom once complained that small scissors don't cut the grass.
But small defects piling up lead to big problems.
I believe Postgres will benefit enormously from enabling all the warnings
at compile time, at least the new little bugs will have some chance of not
getting into the codebase.
regards,
Ranier Vilela
From | Date | Subject | |
---|---|---|---|
Next Message | Alexander Korotkov | 2021-06-16 12:19:08 | Re: unnesting multirange data types |
Previous Message | Ajin Cherian | 2021-06-16 11:42:22 | Re: Added schema level support for publication. |