pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Michael Paquier <michael@paquier.xyz>
Cc: Thomas Munro <thomas.munro@enterprisedb.com>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: A few warnings on Windows
Date: Tue, 01 May 2018 16:48:49 -0400
Message-ID: <15083.1525207729@sss.pgh.pa.us> (raw)
In-Reply-To: <20180501083026.GB25848@paquier.xyz>
References: <CAEepm=3=TDYEXUEcHpEx+Twc31wo7PA0oBAiNt6sWmq93MW02A@mail.gmail.com>
	<20180501083026.GB25848@paquier.xyz>

Michael Paquier <michael@paquier.xyz> writes:
> On Tue, May 01, 2018 at 05:40:18PM +1200, Thomas Munro wrote:
>> src/backend/replication/basebackup.c(1470): warning C4146: unary minus
>> operator applied to unsigned type, result still unsigned
>> ... where cnt is size_t.  Perhaps we should use (or cast to) off_t?

> Sounds sensible.

+1.

>> We have:     uint64      result = seed ^ (sizeof(int64) * MM2_MUL);
>> 
>> ... where MM2_MUL is a UINT64CONST.  I checked the upstream source of
>> this code and it's using a runtime multiplicand while here it's a
>> constant so the compiler sees the overflow.  I suppose we could make
>> the warning go away by just defining a constant (which I make out to
>> be 0x35253c9ade8f4ca8).

> Or just enforce it with some casts?

I'm not sure we could silence the warning with casts.  Thomas' proposal
of a hand-computed constant seems reasonable.

>> C:\Program Files (x86)\Microsoft Visual Studio
>> 12.0\VC\include\stdbool.h(11): warning C4005: 'false' : macro
>> redefinition [C:\buildfarm\buildenv\HEAD\pgsql.build\jsonb_plperl.vcxproj]
>> c:\buildfarm\buildenv\head\pgsql.build\src\include\c.h(283)
>> : see previous definition of 'false'
>> C:\Program Files (x86)\Microsoft Visual Studio
>> 12.0\VC\include\stdbool.h(12): warning C4005: 'true' : macro
>> redefinition [C:\buildfarm\buildenv\HEAD\pgsql.build\jsonb_plperl.vcxproj]
>> c:\buildfarm\buildenv\head\pgsql.build\src\include\c.h(279)
>> : see previous definition of 'true'

> Those are caused by the interactions of stdbool.h from MSVC and the
> definitions from c.h.

Yeah.  In the wake of Peter's changes to use <stdbool.h> on other
platforms, should we be enabling HAVE_STDBOOL_H for Windows?


On more or less the same topic, I just scraped all the compiler warnings
for HEAD from the buildfarm database, and there seem to be a few other
things worth cleaning up.  One that I'm looking at is that recent gcc
has a -Wimplicit-fallthrough warning for switch branches not separated
by a "break" or similar.  It can be silenced with a comment similar to
/* FALLTHROUGH */, but we have not been entirely consistent about
providing such comments.  I'm inclined to run around and fix those
omissions.  Perhaps at some point we should have configure turn that
warning on if available?

			regards, tom lane




view thread (21+ messages)  latest in thread

Message-ID: <15083.1525207729@sss.pgh.pa.us>
Permalink:  ../15083.1525207729@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/15083.1525207729@sss.pgh.pa.us

 · 

reply

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-hackers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, michael@paquier.xyz, thomas.munro@enterprisedb.com
  Subject: Re: A few warnings on Windows
  In-Reply-To: <15083.1525207729@sss.pgh.pa.us>

* 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