Re: pgsql: Fix blatantly broken record_image_cmp() logic for pass-by-value

From: Kevin Grittner <kgrittn(at)ymail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "pgsql-committers(at)postgresql(dot)org" <pgsql-committers(at)postgresql(dot)org>
Subject: Re: pgsql: Fix blatantly broken record_image_cmp() logic for pass-by-value
Date: 2013-11-08 16:36:55
Message-ID: 1383928615.97932.YahooMailNeo@web162904.mail.bf1.yahoo.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Fix blatantly broken record_image_cmp() logic for pass-by-value
> fields.
>
> Doesn't anybody here pay attention to compiler warnings?

> http://git.postgresql.org/pg/commitdiff/28858811472f316f73eba0e564837088fc8c6ccd

I don't get a warning on this with either of these compilers,
either with or without asserts enabled:

gcc (Ubuntu/Linaro 4.7.2-2ubuntu1) 4.7.2
Ubuntu clang version 3.0-6ubuntu3 (tags/RELEASE_30/final) (based on LLVM 3.0)

Bruce had reported a warning, and I was trying to establish whether
a particular change eliminated that warning when the above was
committed.  I really don't like the above "fix", since it only
suppresses the warning without fixing the fundamental problem --
which is that if there is a pass-by-value type with a disallowed
length the comparison would not generate an error in a no-assert
build.  The above patch only changes things from an unpredictable
wrong behavior to a predictable wrong behavior in such cases.

I think something like the attached would make more sense.

Can I get confirmation from someone who can create the warning that
the attached fixes it?

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Attachment Content-Type Size
warning-recimgcmp-v2.diff text/x-diff 1.2 KB

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2013-11-08 16:37:43 pgsql: Make contain_volatile_functions/contain_mutable_functions look i
Previous Message Tom Lane 2013-11-08 14:00:05 pgsql: Fix subtly-wrong volatility checking in BeginCopyFrom().

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2013-11-08 16:58:46 Re: pgsql: Fix blatantly broken record_image_cmp() logic for pass-by-value
Previous Message Mike Blackwell 2013-11-08 15:34:40 Re: stats for network traffic WIP