Re: pgsql: Avoid -Wconversion warnings when using checksum_impl.h - Mailing list pgsql-committers

From David Steele
Subject Re: pgsql: Avoid -Wconversion warnings when using checksum_impl.h
Date
Msg-id 7916219a-96c6-4298-cb36-c97c6bd3930c@pgmasters.net
Whole thread Raw
In response to Re: pgsql: Avoid -Wconversion warnings when using checksum_impl.h  (Magnus Hagander <magnus@hagander.net>)
Responses Re: pgsql: Avoid -Wconversion warnings when using checksum_impl.h
List pgsql-committers
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/module/postgres/interfaceTest.c#L182

Regards,
-- 
-David
david@pgmasters.net



pgsql-committers by date:

Previous
From: Magnus Hagander
Date:
Subject: Re: pgsql: Avoid -Wconversion warnings when using checksum_impl.h
Next
From: David Steele
Date:
Subject: Re: pgsql: Avoid -Wconversion warnings when using checksum_impl.h