From: David Steele <david@pgmasters.net>
To: Magnus Hagander <magnus@hagander.net>
To: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-committers <pgsql-committers@lists.postgresql.org>
Subject: Re: pgsql: Avoid -Wconversion warnings when using checksum_impl.h
Date: Thu, 5 Mar 2020 19:32:59 -0500
Message-ID: <7916219a-96c6-4298-cb36-c97c6bd3930c@pgmasters.net> (raw)
In-Reply-To: <CABUevEzoWeR_q-JztqacmATBvVMUL9rtOcpM5RACubKcmhmk3Q@mail.gmail.com>
References: <E1j9ioh-0005Kn-4O@gemulon.postgresql.org>
<1cf30561-7dad-dc6e-9fc3-5c456948cfeb@pgmasters.net>
<20200305233014.GA2501@paquier.xyz>
<CABUevEzoWeR_q-JztqacmATBvVMUL9rtOcpM5RACubKcmhmk3Q@mail.gmail.com>
On 3/5/20 7:14 PM, Magnus Hagander wrote:
> On Thu, Mar 5, 2020 at 3:30 PM Michael Paquier <michael@paquier.xyz> wrote:
>>
>> On Thu, Mar 05, 2020 at 12:51:50PM -0500, David Steele wrote:
>>> Looks like you changed 65535 -> 65536 before commit. I checked the original
>>> patch and it has 65535.
>>
>> That's my fault, thanks. I have been playing with the surroundings
>> while looking at your patch.
>
> :/
>
> That's a pretty dangerous mistake, and moreso because we don't have a
> test around it. We should really find a way to add such a test -- just
> a hardcoded page calculation that should always return the same value
> perhaps?
FWIW, we use static values in our unit tests (which are written in C),
then test against packaged versions of Postgres for integration tests.
When I saw the commit I pulled it in so I could remove instructions for
the manual step to add the cast. So in this case the issue was apparent
really quickly. Normally we only pull in new code from PostgreSQL once
a year.
We think our unit tests against static values may have endianess issues
but we have not verified that one way or the other. Here's what they
look like:
https://github.com/pgbackrest/pgbackrest/blob/e55443c890181ea63a350275447885331c8254e4/test/src/modu...
Regards,
--
-David
david@pgmasters.net
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-committers@postgresql.org
Cc: david@pgmasters.net, magnus@hagander.net, michael@paquier.xyz, pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Avoid -Wconversion warnings when using checksum_impl.h
In-Reply-To: <7916219a-96c6-4298-cb36-c97c6bd3930c@pgmasters.net>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox