Horton

Thread

Commits

  1. Avoid -Wconversion warnings when using checksum_impl.h

  1. Cast to uint16 in pg_checksum_page()

    David Steele <david@pgmasters.net> — 2020-03-03T23:37:36Z

    Hackers,

The current code in checksum_impl.h does not play nice with -Wconversion 
on gcc:

warning: conversion to 'uint16 {aka short unsigned int}' from 'uint32 
{aka unsigned int}' may alter its value [-Wconversion]
                                           return (checksum % 65535) + 1;
                                                  ~~~~~~~~~~~~~~~~~~~^~~

It seems like an explicit cast to uint16 would be better?

Regards,
-- 
-David
david@pgmasters.net

  2. Re: Cast to uint16 in pg_checksum_page()

    Michael Paquier <michael@paquier.xyz> — 2020-03-04T04:41:05Z

    On Tue, Mar 03, 2020 at 06:37:36PM -0500, David Steele wrote:
> Hackers,
> 
> The current code in checksum_impl.h does not play nice with -Wconversion on
> gcc:
> 
> warning: conversion to 'uint16 {aka short unsigned int}' from 'uint32 {aka
> unsigned int}' may alter its value [-Wconversion]
>                                           return (checksum % 65535) + 1;
>                                                  ~~~~~~~~~~~~~~~~~~~^~~
> 
> It seems like an explicit cast to uint16 would be better?

Attempting to compile the backend code with -Wconversion leads to many
warnings, still there has been at least one fix in the past to ease
the use of the headers in this case, with b5b3229 (this made the code
more readable).  Should we really care about this case?
--
Michael

  3. Re: Cast to uint16 in pg_checksum_page()

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-03-04T06:05:03Z

    Michael Paquier <michael@paquier.xyz> writes:
> On Tue, Mar 03, 2020 at 06:37:36PM -0500, David Steele wrote:
>> It seems like an explicit cast to uint16 would be better?

> Attempting to compile the backend code with -Wconversion leads to many
> warnings, still there has been at least one fix in the past to ease
> the use of the headers in this case, with b5b3229 (this made the code
> more readable).  Should we really care about this case?

Per the commit message for b5b3229, it might be worth getting rid of
such messages for code that's exposed in header files, even if removing
all of those warnings would be too much work.  Perhaps David's use-case
is an extension that's using that header?

			regards, tom lane

  4. Re: Cast to uint16 in pg_checksum_page()

    David Steele <david@pgmasters.net> — 2020-03-04T12:02:43Z

    On 3/4/20 1:05 AM, Tom Lane wrote:
> Michael Paquier <michael@paquier.xyz> writes:
>> On Tue, Mar 03, 2020 at 06:37:36PM -0500, David Steele wrote:
>>> It seems like an explicit cast to uint16 would be better?
> 
>> Attempting to compile the backend code with -Wconversion leads to many
>> warnings, still there has been at least one fix in the past to ease
>> the use of the headers in this case, with b5b3229 (this made the code
>> more readable).  Should we really care about this case?
> 
> Per the commit message for b5b3229, it might be worth getting rid of
> such messages for code that's exposed in header files, even if removing
> all of those warnings would be too much work.  Perhaps David's use-case
> is an extension that's using that header?

Yes, this is being included in an external project. Previously we have 
used a highly marked-up version but we are now trying to pull in the 
header more or less verbatim.

Since this header is specifically designated as something external 
projects may want to use I think it makes sense to fix the warning.

Regards,
-- 
-David
david@pgmasters.net

  5. Re: Cast to uint16 in pg_checksum_page()

    Michael Paquier <michael@paquier.xyz> — 2020-03-04T12:52:08Z

    On Wed, Mar 04, 2020 at 07:02:43AM -0500, David Steele wrote:
> Yes, this is being included in an external project. Previously we have used
> a highly marked-up version but we are now trying to pull in the header more
> or less verbatim.
> 
> Since this header is specifically designated as something external projects
> may want to use I think it makes sense to fix the warning.

This sounds like a sensible argument, similar to the ones raised on
the other thread, so no objections from me to improve things here.  I
can look at that tomorrow, except if somebody else beats me to it.
--
Michael

  6. Re: Cast to uint16 in pg_checksum_page()

    Michael Paquier <michael@paquier.xyz> — 2020-03-05T05:13:26Z

    On Wed, Mar 04, 2020 at 09:52:08PM +0900, Michael Paquier wrote:
> This sounds like a sensible argument, similar to the ones raised on
> the other thread, so no objections from me to improve things here.  I
> can look at that tomorrow, except if somebody else beats me to it.

And done.
--
Michael