Re: Windows warnings from VS 2017

From: Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>
To: Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Windows warnings from VS 2017
Date: 2017-09-21 12:11:35
Message-ID: bfa18917-9cfa-57ce-1806-043d478c9519@2ndQuadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 09/21/2017 02:53 AM, Haribabu Kommi wrote:
>
>
> On Thu, Sep 21, 2017 at 12:26 PM, Andrew Dunstan
> <andrew(dot)dunstan(at)2ndquadrant(dot)com
> <mailto:andrew(dot)dunstan(at)2ndquadrant(dot)com>> wrote:
>
>
>
> On 09/20/2017 08:18 PM, Andrew Dunstan wrote:
> >
> > On 09/20/2017 07:54 PM, Tom Lane wrote:
> >> Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com
> <mailto:andrew(dot)dunstan(at)2ndquadrant(dot)com>> writes:
> >>> It's also warning that it will copy 16 bytes to a 13 byte
> structure at
> >>> lines 518, 1293 and 1294 of src/backend/commands/dbcommands.c.
> I haven't
> >>> seen any ill effects of this so far, but it seems to indicate that
> >>> something is possibly amiss on this compiler with the MemSet
> macros.
> >> That's weird.  Is it too stupid to figure out that the if() inside
> >> MemSet evaluates to constant false in these calls?  It seems
> hard to
> >> see how it would realize that the loop will write 16 bytes if
> it doesn't
> >> propagate the constant value forward.
> >>
> >> However ... on some other compilers, I've noticed that the
> compiler seems
> >> more likely to make "obvious" deductions of that sort if the
> variables in
> >> question are marked const.  Does it help if you do
> >>
> >> -            void   *_vstart = (void *) (start); \
> >> -            int             _val = (val); \
> >> -            Size    _len = (len); \
> >> +            void   * const _vstart = (void *) (start); \
> >> +            const int       _val = (val); \
> >> +            const Size      _len = (len); \
> >>
> >>
> >> I don't think there's any strong reason not to just do
> s/MemSet/memset/
> >> in these calls and nearby ones, but it would be good to
> understand just
> >> what's wrong here.  And why it's only showing up in that file;
> seems
> >> nearly certain that we have similar coding elsewhere.
> >>
> >>
> >
> > I'll test it.
> >
>
>
> Doesn't make a difference. I agree it would be good to understand
> what's
> going on.
>
>
> These warnings are not produced when the build is run in DEBUG mode.
> Because of this I didn't see these warning when I was working with VS
> 2017.
>
> This may be an issue with VS 2017 compiler.
>

Yeah, it seems like it, particularly when identical code in another
function in the same file doesn't generate a complaint.

The speed of memset is hardly going to be the dominating factor in a
'CREATE DATABASE' command, so we could certainly afford to change to
plain memset calls here.

I'll see if I can find someone to report this to :-)

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ashutosh Bapat 2017-09-21 12:21:30 Re: Partition-wise join for join between (declaratively) partitioned tables
Previous Message Rajkumar Raghuwanshi 2017-09-21 11:30:46 Re: Partition-wise aggregation/grouping