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

From: David Steele <david(at)pgmasters(dot)net>
To: Magnus Hagander <magnus(at)hagander(dot)net>, Michael Paquier <michael(at)paquier(dot)xyz>
Cc: pgsql-committers <pgsql-committers(at)lists(dot)postgresql(dot)org>
Subject: Re: pgsql: Avoid -Wconversion warnings when using checksum_impl.h
Date: 2020-03-06 00:32:59
Message-ID: 7916219a-96c6-4298-cb36-c97c6bd3930c@pgmasters.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

On 3/5/20 7:14 PM, Magnus Hagander wrote:
> On Thu, Mar 5, 2020 at 3:30 PM Michael Paquier <michael(at)paquier(dot)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(at)pgmasters(dot)net

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message David Steele 2020-03-06 00:34:56 Re: pgsql: Avoid -Wconversion warnings when using checksum_impl.h
Previous Message Magnus Hagander 2020-03-06 00:14:20 Re: pgsql: Avoid -Wconversion warnings when using checksum_impl.h