Skip site navigation (1) Skip section navigation (2)

Re: [PATCH] Compile without warning with gcc's -Wtype-limits, -Wempty-body

From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Peter Eisentraut <peter_e(at)gmx(dot)net>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PATCH] Compile without warning with gcc's -Wtype-limits, -Wempty-body
Date: 2013-01-15 11:36:25
Message-ID: 20130115113624.GF5115@awork2.anarazel.de (view raw or flat)
Thread:
Lists: pgsql-hackers
On 2013-01-14 22:26:39 -0500, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > On 2013-01-14 20:39:05 -0500, Peter Eisentraut wrote:
> >> On Tue, 2013-01-15 at 00:29 +0100, Andres Freund wrote:
> >>> Independently from this patch, should we add -Wtype-limits to the
> >>> default parameters?
> 
> >> I think we have had a discussion along this line before.  I am against
> >> fixing warnings from this option, because those changes would hide
> >> errors if a variable's type changed from signed to unsigned or vice
> >> versa, which could happen because of refactoring or it might be
> >> dependent on system headers.
> 
> > Well, I already found a bug (although with very limited consequences) in
> > the walsender code and one with graver consequences in code I just
> > submitted. So I don't really see that being on-par with some potential
> > future refactoring...
> 
> FWIW, I agree with Peter --- in particular, warning against "x >= MIN"
> just because MIN happens to be zero and x happens to be unsigned is the
> sort of nonsense up with which we should not put.  Kowtowing to that
> kind of warning makes the code less robust, not more so.

Oh, I agree, that warning is pointless in itself.

But in general doing a comparison like > 0 *can* show a programming
error as evidenced e.g. by
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=3f4b1749a8168893558f70021be4f40c650bbada
and just about the same error I made in xlogdump.

I just think that the price of fixing a single Assert() that hasn't
changed in years where the variable isn't likely to ever get signed is
acceptable.

> It's a shame that the compiler writers have not figured this out and
> separated misguided pedantry from actually-useful warnings.  If I assign
> -1 to an unsigned variable, by all means tell me about *that*.  Don't
> tell me your opinion of whether an assertion check is necessary.

Yea, but I have to admit its damned hard hard to automatically discern
the above actually valid warning and the bogus Assert...

Greetings,

Andres Freund

-- 
 Andres Freund	                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


In response to

Responses

pgsql-hackers by date

Next:From: Peter EisentrautDate: 2013-01-15 12:04:52
Subject: Re: replace plugins directory with GUC
Previous:From: Andres FreundDate: 2013-01-15 10:59:41
Subject: Re: logical changeset generation v4

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group