Re: [HACKERS] Compiler warnings

[Joe Conway]

On 01/02/2017 10:55 AM, Joe Conway wrote:
> On the 9.2 and 9.3 branches I see two warnings:

> This one once:
> ---
> plancache.c:1197:9: warning: ‘plan’ may be used uninitialized in this
> function [-Wmaybe-uninitialized]
> 
> And this one once per bison file:
> ---
> gram.y:169.1-13: warning: deprecated directive, use ‘%name-prefix’
> [-Wdeprecated]
>  %name-prefix="base_yy"
>  ^

> Starting in 9.5 I only get the plancache.c warning and this one:
> ---
> lwlock.c:1555:5: warning: ‘mode’ may be used uninitialized in this
> function [-Wmaybe-uninitialized]
>   if (mode == LW_EXCLUSIVE)

Peter Eisentraut's Bison deprecation warnings patch (per Tom's reply
nearby in this thread) back-patched to 9.2 and 9.3 branches.
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=55fb759ab3e7543a6be72a35e6b6961455c5b393

Stephen Frosts's plancache.c back-patched to 9.6 through 9.2
and lwlock.c back-patched 9.6 through 9.5:
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=d97b14ddab2059e1d73c0cd17f26bac4ef13e682

> For the sake of completeness, in 9.4. I get the plancache.c warning and
> this one:
> ---
> basebackup.c:1284:6: warning: variable ‘wait_result’ set but not used
> [-Wunused-but-set-variable]
>   int   wait_result;

This one is no longer seen -- I must have neglected to pull before
making that comment.

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] Compiler warnings

[Tom Lane]

Joe Conway  writes:
> On 01/02/2017 11:09 AM, Tom Lane wrote:
>> The bison issue is discussed in
>> https://www.postgresql.org/message-id/flat/E1WpjkB-0003zA-N4%40gemulon.postgresql.org

> Ah, thanks. I vaguely remember that thread now.

> Looks like there was some consensus for applying Peter's patch with the
> addition of a comment, but apparently that never happened. Would we
> still consider that for 9.2 and 9.3 branches?

Sure it did, see 55fb759ab3e7543a6be72a35e6b6961455c5b393.
That's why you don't see the complaints in 9.4 and up.
I'm not sure why Peter didn't back-patch it, but doing so now seems
safe enough.

> Any thoughts on fixing the other warnings?

I'm okay with small, low-risk patches to silence warnings in back
branches.  Like Robert, I'd be concerned about anything invasive.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Compiler warnings

[Joe Conway]

On 01/02/2017 11:09 AM, Tom Lane wrote:
> Joe Conway  writes:
>> If there is agreement on fixing these warnings, other than the bison
>> generated warning, I would be happy to do it. I'd also be happy to look
>> for a fix the bison warning as well if desired, but that should be
>> handled separately I would think.
> 
> The bison issue is discussed in
> https://www.postgresql.org/message-id/flat/E1WpjkB-0003zA-N4%40gemulon.postgresql.org

Ah, thanks. I vaguely remember that thread now.

Looks like there was some consensus for applying Peter's patch with the
addition of a comment, but apparently that never happened. Would we
still consider that for 9.2 and 9.3 branches?

Any thoughts on fixing the other warnings?

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] Compiler warnings

[Tom Lane]

Joe Conway  writes:
> If there is agreement on fixing these warnings, other than the bison
> generated warning, I would be happy to do it. I'd also be happy to look
> for a fix the bison warning as well if desired, but that should be
> handled separately I would think.

The bison issue is discussed in
https://www.postgresql.org/message-id/flat/E1WpjkB-0003zA-N4%40gemulon.postgresql.org

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Compiler warnings

[Joe Conway]

On 01/02/2017 10:18 AM, Robert Haas wrote:
> On Thu, Dec 29, 2016 at 10:47 AM, Joe Conway  wrote:
>> Shouldn't this be back-patched? The plancache warning goes back through
>> 9.2 (at least) and the lwlocks warning through 9.5 (or maybe it was 9.4).
> 
> Warnings are going to be different for each individual developer, but
> I am cautiously in favor making more of an effort to fix back-branch
> warnings provided that it doesn't generate too much code churn.  For
> example, if your toolchain generates only these two warnings on 9.2
> then, sure, let's back-port these two fixes; making things
> warning-clean is great.  But if there are dozens or hundreds of
> warnings currently, fixing only a handful of those warnings probably
> isn't valuable, and fixing all of them is probably a little more risk
> than we necessarily want to take.  Someone could goof and make a bug.
> On my MacBook Pro with my toolchain, we're warning-clean back to 9.3
> or 9.4, and before that there are some problems -- most annoyingly the
> fact that 73b416b2e41237b657d29d8f42a4bb34bf700928 couldn't be easily
> backported to older branches.  I don't think it would be crazy to try
> to get all of the warnings I see fixed up and it would be convenient
> for me, but I haven't been willing to do the work, either.
> 

FWIW, I'm running Mint 18 which is based on Unbuntu 16.04 I believe. On
the 9.2 and 9.3 branches I see two warnings:

This one once:
---
plancache.c:1197:9: warning: ‘plan’ may be used uninitialized in this
function [-Wmaybe-uninitialized]

And this one once per bison file:
---
gram.y:169.1-13: warning: deprecated directive, use ‘%name-prefix’
[-Wdeprecated]
 %name-prefix="base_yy"
 ^

The plancache.c fix in Stephen's patch was really simple: initialize
plan = NULL and add an assert. To me that seems like something we should
definitely back-patch.

On the other hand, seems like we have had bison related warnings of one
kind or another since I first got involved with Postgres. So while it
would be nice to make that one go away, it somehow bothers me less. It
also goes away starting in 9.4 anyway.


Starting in 9.5 I only get the plancache.c warning and this one:
---
lwlock.c:1555:5: warning: ‘mode’ may be used uninitialized in this
function [-Wmaybe-uninitialized]
  if (mode == LW_EXCLUSIVE)
 ^

That is the other warning fixed in Stephens commit to master.

For the sake of completeness, in 9.4. I get the plancache.c warning and
this one:
---
basebackup.c:1284:6: warning: variable ‘wait_result’ set but not used
[-Wunused-but-set-variable]
  int   wait_result;
  ^

Seems like that should be easily fixed.

If there is agreement on fixing these warnings, other than the bison
generated warning, I would be happy to do it. I'd also be happy to look
for a fix the bison warning as well if desired, but that should be
handled separately I would think.

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] Compiler warnings

[Robert Haas]

On Thu, Dec 29, 2016 at 10:47 AM, Joe Conway  wrote:
> Shouldn't this be back-patched? The plancache warning goes back through
> 9.2 (at least) and the lwlocks warning through 9.5 (or maybe it was 9.4).

Warnings are going to be different for each individual developer, but
I am cautiously in favor making more of an effort to fix back-branch
warnings provided that it doesn't generate too much code churn.  For
example, if your toolchain generates only these two warnings on 9.2
then, sure, let's back-port these two fixes; making things
warning-clean is great.  But if there are dozens or hundreds of
warnings currently, fixing only a handful of those warnings probably
isn't valuable, and fixing all of them is probably a little more risk
than we necessarily want to take.  Someone could goof and make a bug.
On my MacBook Pro with my toolchain, we're warning-clean back to 9.3
or 9.4, and before that there are some problems -- most annoyingly the
fact that 73b416b2e41237b657d29d8f42a4bb34bf700928 couldn't be easily
backported to older branches.  I don't think it would be crazy to try
to get all of the warnings I see fixed up and it would be convenient
for me, but I haven't been willing to do the work, either.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Compiler warnings

[Joe Conway]

On 12/06/2016 01:59 PM, Robert Haas wrote:
> On Tue, Dec 6, 2016 at 3:46 PM, Stephen Frost  wrote:
>> Good thought, thanks!
>>
>> Updated patch attached with that change and I also added an Assert() to
>> GetCachedPlan(), in case that code gets whacked around later and somehow
>> we end up falling through without actually setting *plan.
>>
>> Thoughts?
> 
> wfm
> 

Shouldn't this be back-patched? The plancache warning goes back through
9.2 (at least) and the lwlocks warning through 9.5 (or maybe it was 9.4).

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] Compiler warnings

[Robert Haas]

On Tue, Dec 6, 2016 at 3:46 PM, Stephen Frost  wrote:
> Good thought, thanks!
>
> Updated patch attached with that change and I also added an Assert() to
> GetCachedPlan(), in case that code gets whacked around later and somehow
> we end up falling through without actually setting *plan.
>
> Thoughts?

wfm

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Compiler warnings

[Stephen Frost]

Robert,

* Robert Haas (robertmh...@gmail.com) wrote:
> On Tue, Dec 6, 2016 at 3:23 PM, Stephen Frost  wrote:
> > Given the lack of screaming, I'll push the attached in a bit, which just
> > initializes the two variables being complained about.  As mentioned,
> > there doesn't appear to be any live bugs here, this is just to silence
> > the compiler warnings.
> 
> In LWLockRelease, why not just move mode = held_lwlocks[i].mode; after
> the if (i < 0) elog(...), instead of initializing with a bogus value?

Good thought, thanks!

Updated patch attached with that change and I also added an Assert() to
GetCachedPlan(), in case that code gets whacked around later and somehow
we end up falling through without actually setting *plan.

Thoughts?

Thanks!

Stephen
From 55ecc3367bd063f05c89b4b1ca881d2b084859f6 Mon Sep 17 00:00:00 2001
From: Stephen Frost 
Date: Tue, 6 Dec 2016 15:19:09 -0500
Subject: [PATCH] Silence compiler warnings

Rearrange a bit of code to ensure that 'mode' in LWLockRelease is
obviously always set, which seems a bit cleaner and avoids a compiler
warning (thanks to Robert for the suggestion!).

In GetCachedPlan(), initialize 'plan' to silence a compiler warning, but
also add an Assert() to make sure we don't ever actually fall through
with 'plan' still being set to NULL, since we are about to dereference
it.

Neither of these appear to be live bugs but at least gcc
5.4.0-6ubuntu1~16.04.4 doesn't quite have the smarts to realize that.

Discussion: https://www.postgresql.org/message-id/20161129152102.GR13284%40tamriel.snowman.net
---
 src/backend/storage/lmgr/lwlock.c   | 9 -
 src/backend/utils/cache/plancache.c | 4 +++-
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c
index 9c6862f..ffb2f72 100644
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -1780,15 +1780,14 @@ LWLockRelease(LWLock *lock)
 	 * be the latest-acquired lock; so search array backwards.
 	 */
 	for (i = num_held_lwlocks; --i >= 0;)
-	{
 		if (lock == held_lwlocks[i].lock)
-		{
-			mode = held_lwlocks[i].mode;
 			break;
-		}
-	}
+
 	if (i < 0)
 		elog(ERROR, "lock %s %d is not held", T_NAME(lock), T_ID(lock));
+
+	mode = held_lwlocks[i].mode;
+
 	num_held_lwlocks--;
 	for (; i < num_held_lwlocks; i++)
 		held_lwlocks[i] = held_lwlocks[i + 1];
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index 884cdab..aa146d6 100644
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -1128,7 +1128,7 @@ CachedPlan *
 GetCachedPlan(CachedPlanSource *plansource, ParamListInfo boundParams,
 			  bool useResOwner)
 {
-	CachedPlan *plan;
+	CachedPlan *plan = NULL;
 	List	   *qlist;
 	bool		customplan;
 
@@ -1210,6 +1210,8 @@ GetCachedPlan(CachedPlanSource *plansource, ParamListInfo boundParams,
 		}
 	}
 
+	Assert(plan != NULL);
+
 	/* Flag the plan as in use by caller */
 	if (useResOwner)
 		ResourceOwnerEnlargePlanCacheRefs(CurrentResourceOwner);
-- 
2.7.4



signature.asc
Description: Digital signature

Re: [HACKERS] Compiler warnings

[Robert Haas]

On Tue, Dec 6, 2016 at 3:23 PM, Stephen Frost  wrote:
> * Stephen Frost (sfr...@snowman.net) wrote:
>> Not sure if anyone else has been seeing these, but I'm getting a bit
>> tired of them.  Neither is a live bug, but they also seem pretty simple
>> to fix.  The attached patch makes both of these warnings go away.  At
>> least for my common build, these are the only warnings that are thrown.
>>
>> I'm building with:
>>
>> gcc (Ubuntu 5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609
>>
>> .../src/backend/storage/lmgr/lwlock.c: In function ‘LWLockRelease’:
>> .../src/backend/storage/lmgr/lwlock.c:1802:5: warning: ‘mode’ may be used 
>> uninitialized in this function [-Wmaybe-uninitialized]
>>   if (mode == LW_EXCLUSIVE)
>>  ^
>> .../src/backend/utils/cache/plancache.c: In function ‘GetCachedPlan’:
>> .../src/backend/utils/cache/plancache.c:1232:9: warning: ‘plan’ may be used 
>> uninitialized in this function [-Wmaybe-uninitialized]
>>   return plan;
>>  ^
>
> Given the lack of screaming, I'll push the attached in a bit, which just
> initializes the two variables being complained about.  As mentioned,
> there doesn't appear to be any live bugs here, this is just to silence
> the compiler warnings.

In LWLockRelease, why not just move mode = held_lwlocks[i].mode; after
the if (i < 0) elog(...), instead of initializing with a bogus value?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Compiler warnings

[Stephen Frost]

All,

* Stephen Frost (sfr...@snowman.net) wrote:
> Not sure if anyone else has been seeing these, but I'm getting a bit
> tired of them.  Neither is a live bug, but they also seem pretty simple
> to fix.  The attached patch makes both of these warnings go away.  At
> least for my common build, these are the only warnings that are thrown.
> 
> I'm building with:
> 
> gcc (Ubuntu 5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609
> 
> .../src/backend/storage/lmgr/lwlock.c: In function ‘LWLockRelease’:
> .../src/backend/storage/lmgr/lwlock.c:1802:5: warning: ‘mode’ may be used 
> uninitialized in this function [-Wmaybe-uninitialized]
>   if (mode == LW_EXCLUSIVE)
>  ^
> .../src/backend/utils/cache/plancache.c: In function ‘GetCachedPlan’:
> .../src/backend/utils/cache/plancache.c:1232:9: warning: ‘plan’ may be used 
> uninitialized in this function [-Wmaybe-uninitialized]
>   return plan;
>  ^

Given the lack of screaming, I'll push the attached in a bit, which just
initializes the two variables being complained about.  As mentioned,
there doesn't appear to be any live bugs here, this is just to silence
the compiler warnings.

Thanks!

Stephen
From afe53a0fa8a53c080b77a7334531ff3a5dc976d4 Mon Sep 17 00:00:00 2001
From: Stephen Frost 
Date: Tue, 6 Dec 2016 15:19:09 -0500
Subject: [PATCH] Silence compiler warnings

Initialize a couple of variables to silence compiler warnings.  There
aren't any cases where these variables could actually end up being used
while uninitialized, but at least gcc 5.4.0-6ubuntu1~16.04.4 doesn't
quite have the smarts to realize that.

Discussion: https://www.postgresql.org/message-id/20161129152102.GR13284%40tamriel.snowman.net
---
 src/backend/storage/lmgr/lwlock.c   | 2 +-
 src/backend/utils/cache/plancache.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c
index 9c6862f..909ff45 100644
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -1770,7 +1770,7 @@ LWLockUpdateVar(LWLock *lock, uint64 *valptr, uint64 val)
 void
 LWLockRelease(LWLock *lock)
 {
-	LWLockMode	mode;
+	LWLockMode	mode = LW_EXCLUSIVE;
 	uint32		oldstate;
 	bool		check_waiters;
 	int			i;
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index 884cdab..a37074b 100644
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -1128,7 +1128,7 @@ CachedPlan *
 GetCachedPlan(CachedPlanSource *plansource, ParamListInfo boundParams,
 			  bool useResOwner)
 {
-	CachedPlan *plan;
+	CachedPlan *plan = NULL;
 	List	   *qlist;
 	bool		customplan;
 
-- 
2.7.4



signature.asc
Description: Digital signature

Re: [HACKERS] Compiler warnings

[Stephen Frost]

* Stephen Frost (sfr...@snowman.net) wrote:
> diff --git a/src/backend/utils/cache/plancache.c 
> b/src/backend/utils/cache/plancache.c
> new file mode 100644
> index 884cdab..b5d97c8
> *** a/src/backend/utils/cache/plancache.c
> --- b/src/backend/utils/cache/plancache.c
> *** GetCachedPlan(CachedPlanSource *plansour
> *** 1196,1204 
>*/
>   qlist = NIL;
>   }
> ! }
> ! 
> ! if (customplan)
>   {
>   /* Build a custom plan */
>   plan = BuildCachedPlan(plansource, qlist, boundParams);
> --- 1196,1203 
>*/
>   qlist = NIL;
>   }
> ! } 
> ! else
>   {
>   /* Build a custom plan */
>   plan = BuildCachedPlan(plansource, qlist, boundParams);

Meh, of course this isn't correct since we could change 'customplan'
inside the first if() block to be true, the right answer is really to
just do:

CachedPlan *plan = NULL;

at the top and keep it simple.

ENEEDMORECOFFEE.

Thanks!

Stephen


signature.asc
Description: Digital signature

[HACKERS] Compiler warnings

[Stephen Frost]

Greetings,

Not sure if anyone else has been seeing these, but I'm getting a bit
tired of them.  Neither is a live bug, but they also seem pretty simple
to fix.  The attached patch makes both of these warnings go away.  At
least for my common build, these are the only warnings that are thrown.

I'm building with:

gcc (Ubuntu 5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609

.../src/backend/storage/lmgr/lwlock.c: In function ‘LWLockRelease’:
.../src/backend/storage/lmgr/lwlock.c:1802:5: warning: ‘mode’ may be used 
uninitialized in this function [-Wmaybe-uninitialized]
  if (mode == LW_EXCLUSIVE)
 ^
.../src/backend/utils/cache/plancache.c: In function ‘GetCachedPlan’:
.../src/backend/utils/cache/plancache.c:1232:9: warning: ‘plan’ may be used 
uninitialized in this function [-Wmaybe-uninitialized]
  return plan;
 ^

Thoughts?

Thanks!

Stephen
diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c
new file mode 100644
index 9c6862f..909ff45
*** a/src/backend/storage/lmgr/lwlock.c
--- b/src/backend/storage/lmgr/lwlock.c
*** LWLockUpdateVar(LWLock *lock, uint64 *va
*** 1770,1776 
  void
  LWLockRelease(LWLock *lock)
  {
! 	LWLockMode	mode;
  	uint32		oldstate;
  	bool		check_waiters;
  	int			i;
--- 1770,1776 
  void
  LWLockRelease(LWLock *lock)
  {
! 	LWLockMode	mode = LW_EXCLUSIVE;
  	uint32		oldstate;
  	bool		check_waiters;
  	int			i;
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
new file mode 100644
index 884cdab..b5d97c8
*** a/src/backend/utils/cache/plancache.c
--- b/src/backend/utils/cache/plancache.c
*** GetCachedPlan(CachedPlanSource *plansour
*** 1196,1204 
  			 */
  			qlist = NIL;
  		}
! 	}
! 
! 	if (customplan)
  	{
  		/* Build a custom plan */
  		plan = BuildCachedPlan(plansource, qlist, boundParams);
--- 1196,1203 
  			 */
  			qlist = NIL;
  		}
! 	} 
! 	else
  	{
  		/* Build a custom plan */
  		plan = BuildCachedPlan(plansource, qlist, boundParams);


signature.asc
Description: Digital signature

Re: [HACKERS] compiler warnings in lwlock

[Tom Lane]

Jeff Janes jeff.ja...@gmail.com writes:
 When building with LOCK_DEBUG but without casserts, I was getting unused
 variable warnings.

 I believe this is the correct way to silence them.

Committed, thanks.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] compiler warnings in lwlock

[Jeff Janes]

When building with LOCK_DEBUG but without casserts, I was getting unused
variable warnings.

I believe this is the correct way to silence them.

Cheers,

Jeff


silence_lwlock_lock_debug.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] compiler warnings in copy.c

[Stephen Frost]

* Andres Freund (and...@2ndquadrant.com) wrote:
 On 2015-01-28 15:05:11 -0500, Stephen Frost wrote:
   Also, you seem to have pushed these commits with a date more than two
   weeks in the past.  Please don't do that!
  
  Oh, wow, sorry about that.  I had expected a rebase to update the date.
 
 It updates the committer, but not the author date. Use --pretty=fuller
 to see all the details. You can pass rebase --ignore-date to also reset
 the author date. Or commit --amend --reset

Thanks for the clarification as to what was happening.  I've modified my
aliases to use 'git am --ignore-date' which appears to have fixed this.

Thanks!

Stephen


signature.asc
Description: Digital signature

[HACKERS] compiler warnings in copy.c

[Robert Haas]

My compiler is unhappy with the latest changes to copy.c:

gcc -Wall -Wmissing-prototypes -Wpointer-arith
-Wdeclaration-after-statement -Wendif-labels
-Wmissing-format-attribute -Wformat-security -fno-strict-aliasing
-fwrapv -fexcess-precision=standard -g -O2 -Wall -Werror
-I../../../src/include -D_GNU_SOURCE -I/usr/include/libxml2   -c -o
copy.o copy.c -MMD -MP -MF .deps/copy.Po
copy.c: In function ‘DoCopy’:
copy.c:924:30: error: ‘rte’ may be used uninitialized in this function
[-Werror=uninitialized]
copy.c:793:17: note: ‘rte’ was declared here
cc1: all warnings being treated as errors

From what I can see, this is a pretty legitimate complaint.  If
stmt-relation == NULL, then rte never gets initialized, but we still
do cstate-range_table = list_make1(rte).  That can't be good.

Also, you seem to have pushed these commits with a date more than two
weeks in the past.  Please don't do that!

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] compiler warnings in copy.c

[Stephen Frost]

* Robert Haas (robertmh...@gmail.com) wrote:
 My compiler is unhappy with the latest changes to copy.c:
 
 gcc -Wall -Wmissing-prototypes -Wpointer-arith
 -Wdeclaration-after-statement -Wendif-labels
 -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing
 -fwrapv -fexcess-precision=standard -g -O2 -Wall -Werror
 -I../../../src/include -D_GNU_SOURCE -I/usr/include/libxml2   -c -o
 copy.o copy.c -MMD -MP -MF .deps/copy.Po
 copy.c: In function ‘DoCopy’:
 copy.c:924:30: error: ‘rte’ may be used uninitialized in this function
 [-Werror=uninitialized]
 copy.c:793:17: note: ‘rte’ was declared here
 cc1: all warnings being treated as errors

Huh, interesting that mine didn't.

 From what I can see, this is a pretty legitimate complaint.  If
 stmt-relation == NULL, then rte never gets initialized, but we still
 do cstate-range_table = list_make1(rte).  That can't be good.

Yeah, I'll fix that.

 Also, you seem to have pushed these commits with a date more than two
 weeks in the past.  Please don't do that!

Oh, wow, sorry about that.  I had expected a rebase to update the date.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] compiler warnings in copy.c

[Andres Freund]

On 2015-01-28 15:05:11 -0500, Stephen Frost wrote:
  Also, you seem to have pushed these commits with a date more than two
  weeks in the past.  Please don't do that!
 
 Oh, wow, sorry about that.  I had expected a rebase to update the date.

It updates the committer, but not the author date. Use --pretty=fuller
to see all the details. You can pass rebase --ignore-date to also reset
the author date. Or commit --amend --reset

Greetings,

Andres Freund

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] compiler warnings under MinGW for 9.4

[Jeff Janes]

In the past, building under MinGW produced so many warnings that I never
bothered to read them.

Now most of them have been removed, so the ones that are left might be
worth reporting.

Using gcc.exe (GCC) 4.6.2 on REL9_4_STABLE
eadd80c08ddfc485db84b9af7cca54a0d50ebe6d I get:

mingwcompat.c:60:1: warning: 'RegisterWaitForSingleObject' redeclared
without dllimport attribute: previous dllimport ignored [-Wattributes]
input.c:382:1: warning: 'saveHistory' defined but not used
[-Wunused-function]

Does anyone have opinions on how to address these?


Cheers,

Jeff

Re: [HACKERS] compiler warnings on the buildfarm

[Peter Eisentraut]

On sön, 2012-07-01 at 19:04 +0200, Stefan Kaltenbrunner wrote:
 seeing some of the latest commits about fixing compiler warnings I
 took a look at the buildfarm to see if there are any interesting ones
 there (in total we have a thousends of warnings on the buildfarm but
 most of those are from very noisy compilers).
 
 so in case anybody is interested those are a selection of the ones
 that at least look somewhat interesting(duplicates mostly removed,
 windows ignored):

Many of these come from ancient compilers, and from minor versions that
are not even the latest for that ancient major release.  They're mostly
not worth worrying about, because evidently the compiler developers
later improved the compilers to not warn about these cases anymore.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] compiler warnings on the buildfarm

[Stefan Kaltenbrunner]

seeing some of the latest commits about fixing compiler warnings I took
a look at the buildfarm to see if there are any interesting ones there
(in total we have a thousends of warnings on the buildfarm but most of
those are from very noisy compilers).

so in case anybody is interested those are a selection of the ones that
at least look somewhat interesting(duplicates mostly removed, windows
ignored):

animal: grebe
Snapshot: 2012-07-01 150224
scan.c: In function 'yy_try_NUL_trans':
scan.c:16243: warning: unused variable 'yyg'
auth.c: In function 'auth_peer':
auth.c:1775: warning: implicit declaration of function 'getpeereid'
ip.c: In function 'getaddrinfo_unix':
ip.c:228: warning: large integer implicitly truncated to unsigned type
Extra instructions are being generated for each reference to a TOC
symbol if the symbol is in the TOC overflow area.
fe-connect.c: In function 'PQconnectPoll':
fe-connect.c:1913: warning: implicit declaration of function 'getpeereid'
ip.c: In function 'getaddrinfo_unix':
ip.c:228: warning: large integer implicitly truncated to unsigned type


animal: spoonbill
Snapshot: 2012-07-01 110005
tuptoaster.c: In function 'heap_tuple_untoast_attr_slice':
tuptoaster.c:198: warning: array size (1) smaller than bound length (16)
tuptoaster.c:198: warning: array size (1) smaller than bound length (16)
tuptoaster.c: In function 'toast_raw_datum_size':
tuptoaster.c:275: warning: array size (1) smaller than bound length (16)
tuptoaster.c:275: warning: array size (1) smaller than bound length (16)
tuptoaster.c: In function 'toast_datum_size':
tuptoaster.c:320: warning: array size (1) smaller than bound length (16)
tuptoaster.c:320: warning: array size (1) smaller than bound length (16)
tuptoaster.c: In function 'toast_save_datum':
tuptoaster.c:1344: warning: array size (1) smaller than bound length (16)
tuptoaster.c:1344: warning: array size (1) smaller than bound length (16)
tuptoaster.c:1458: warning: array size (1) smaller than bound length (16)
tuptoaster.c:1458: warning: array size (1) smaller than bound length (16)
tuptoaster.c: In function 'toast_delete_datum':
tuptoaster.c:1485: warning: array size (1) smaller than bound length (16)
tuptoaster.c:1485: warning: array size (1) smaller than bound length (16)
tuptoaster.c: In function 'toast_fetch_datum':
tuptoaster.c:1610: warning: array size (1) smaller than bound length (16)
tuptoaster.c:1610: warning: array size (1) smaller than bound length (16)
tuptoaster.c: In function 'toast_fetch_datum_slice':
tuptoaster.c:1779: warning: array size (1) smaller than bound length (16)
tuptoaster.c:1779: warning: array size (1) smaller than bound length (16)
relmapper.c: In function 'relmap_redo':
relmapper.c:876: warning: array size (1) smaller than bound length (512)
relmapper.c:876: warning: array size (1) smaller than bound length (512)
elog.c: In function 'write_pipe_chunks':
elog.c:2541: warning: array size (1) smaller than bound length (503)
elog.c:2541: warning: array size (1) smaller than bound length (503)


animal: jaguarundi
Snapshot: 2012-07-01 031500
plpy_exec.c: In function 'PLy_procedure_call':
plpy_exec.c:818: warning: null format string

animal: rover_firefly
Snapshot: 2012-07-01 030400
float.c: In function 'is_infinite':
float.c:167:2: warning: implicit declaration of function 'isinf'
[-Wimplicit-function-declaration]
geo_ops.c: In function 'pg_hypot':
geo_ops.c:5455:2: warning: implicit declaration of function 'isinf'
[-Wimplicit-function-declaration]
execute.c: In function 'sprintf_double_value':
execute.c:473:2: warning: implicit declaration of function 'isinf'
[-Wimplicit-function-declaration]


animal: nightjar
Snapshot: 2012-07-01 023700
In file included from
/usr/home/andrew/bf/root/HEAD/pgsql.66715/../pgsql/src/backend/parser/gram.y:13338:
scan.c: In function 'yy_try_NUL_trans':
scan.c:16243: warning: unused variable 'yyg'
/usr/home/andrew/bf/root/HEAD/pgsql.66715/../pgsql/src/pl/plpython/plpy_exec.c:
In function 'PLy_procedure_call':
/usr/home/andrew/bf/root/HEAD/pgsql.66715/../pgsql/src/pl/plpython/plpy_exec.c:818:
warning: null format string
/usr/home/andrew/bf/root/HEAD/pgsql.66715/../pgsql/src/pl/tcl/pltcl.c:
In function '_PG_init':
/usr/home/andrew/bf/root/HEAD/pgsql.66715/../pgsql/src/pl/tcl/pltcl.c:343:
warning: assignment from incompatible pointer type
/usr/home/andrew/bf/root/HEAD/pgsql.66715/../pgsql/src/pl/tcl/pltcl.c:344:
warning: assignment from incompatible pointer type


animal: locust
Snapshot: 2012-07-01 023122
xlog.c: In function 'StartupXLOG':
xlog.c:5988: warning: 'checkPointLoc' may be used uninitialized in this
function
pgstat.c: In function 'pgstat_report_activity':
pgstat.c:2538: warning: passing argument 1 of
'__dtrace_probe$postgresql$statement__status$v1$63686172202a' discards
qualifiers from pointer target type
In file included from repl_gram.y:172:
postgres.c: In function 'pg_parse_query':
postgres.c:559: warning: passing argument 1 of

[HACKERS] compiler warnings on mingw

[Peter Eisentraut]

I've tried to cross-compile PostgreSQL from Linux to Windows, following
the ideas of Andrew Dunstan [0].  This works quite well.  I see two
compiler warnings altogether, which might be worth getting rid of:

#1

mingwcompat.c:60:1: warning: ‘RegisterWaitForSingleObject’ redeclared without 
dllimport attribute: previous dllimport ignored [-Wattributes]

This can apparently go away with this:

diff --git a/src/backend/port/win32/mingwcompat.c 
b/src/backend/port/win32/mingwcompat.c
index 0978e8c..b1a5ca5 100644
--- a/src/backend/port/win32/mingwcompat.c
+++ b/src/backend/port/win32/mingwcompat.c
@@ -56,6 +56,7 @@
(PHANDLE, HANDLE, WAITORTIMERCALLBACK, PVOID, ULONG, ULONG);
 static __RegisterWaitForSingleObject _RegisterWaitForSingleObject = NULL;
 
+__attribute__((dllimport))
 BOOL   WINAPI
 RegisterWaitForSingleObject(PHANDLE phNewWaitObject,
HANDLE hObject,

Oddly, the mingw buildfarm member[1] complains about

mingwcompat.c:66: warning: no previous prototype for 
'RegisterWaitForSingleObject'

instead.  So there might be some divergent header files around.

Anyone know details about this?

#2

pg_stat_statements.c: In function ‘pgss_ProcessUtility’:
pg_stat_statements.c:840:4: warning: unknown conversion type character ‘l’ in 
format [-Wformat]
pg_stat_statements.c:840:4: warning: too many arguments for format 
[-Wformat-extra-args]

We use a replacement snprintf and set the int64 format to %lld and %llu
based on that.  But pg_stat_statements.c uses sscanf, for which we have
no replacement.  The configure check comments

# MinGW uses '%I64d', though gcc throws an warning with -Wall,
# while '%lld' doesn't generate a warning, but doesn't work.

So assuming that sscanf in the mingw C library works consistently with
snprintf, that might mean that pg_stat_statements is broken on that
platform.  (The claim that %lld doesn't generate a warning is also
questionable here.)


[0]: 
http://people.planetpostgresql.org/andrew/index.php?/archives/264-Cross-compiling-PostgreSQL-for-WIndows.html
[1]: 
http://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=narwhaldt=2012-06-22%2004%3A00%3A05stg=make

PS: Instructions for Debian:

apt-get install gcc-mingw-w64

./configure --build=$(config/config.guess) --host=i686-w64-mingw32 
--without-zlib --without-readline ZIC=/usr/sbin/zic
make world



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] compiler warnings on mingw

[Magnus Hagander]

On Mon, Jun 25, 2012 at 11:42 AM, Peter Eisentraut pete...@gmx.net wrote:
 I've tried to cross-compile PostgreSQL from Linux to Windows, following
 the ideas of Andrew Dunstan [0].  This works quite well.  I see two
 compiler warnings altogether, which might be worth getting rid of:

 #1

 mingwcompat.c:60:1: warning: ‘RegisterWaitForSingleObject’ redeclared without 
 dllimport attribute: previous dllimport ignored [-Wattributes]

 This can apparently go away with this:

 diff --git a/src/backend/port/win32/mingwcompat.c 
 b/src/backend/port/win32/mingwcompat.c
 index 0978e8c..b1a5ca5 100644
 --- a/src/backend/port/win32/mingwcompat.c
 +++ b/src/backend/port/win32/mingwcompat.c
 @@ -56,6 +56,7 @@
            (PHANDLE, HANDLE, WAITORTIMERCALLBACK, PVOID, ULONG, ULONG);
  static __RegisterWaitForSingleObject _RegisterWaitForSingleObject = NULL;

 +__attribute__((dllimport))
  BOOL       WINAPI
  RegisterWaitForSingleObject(PHANDLE phNewWaitObject,
                            HANDLE hObject,

Seems like a proper fix to me - but could be verified by checking what
the actual mingw header looks like. Or maybe that's what you did
already..


 Oddly, the mingw buildfarm member[1] complains about

 mingwcompat.c:66: warning: no previous prototype for 
 'RegisterWaitForSingleObject'

I think that one is just laziness - in the case when we're injecting
that API function into mingw we should declare it in our own headers.
It was likely just left out because the proper API headers already
carry it, it was just missing in mingw.. So it hsould be added to the
port header, under an #ifdef.


 instead.  So there might be some divergent header files around.

 Anyone know details about this?

Perhaps mingw has added it to their api *properly* this time, and the
whole function should go away from mingwcompat.c? In that case it'd
obviously require a configure test, since it doesn't exist in previous
releases.


 #2

 pg_stat_statements.c: In function ‘pgss_ProcessUtility’:
 pg_stat_statements.c:840:4: warning: unknown conversion type character ‘l’ in 
 format [-Wformat]
 pg_stat_statements.c:840:4: warning: too many arguments for format 
 [-Wformat-extra-args]

 We use a replacement snprintf and set the int64 format to %lld and %llu
 based on that.  But pg_stat_statements.c uses sscanf, for which we have
 no replacement.  The configure check comments

 # MinGW uses '%I64d', though gcc throws an warning with -Wall,
 # while '%lld' doesn't generate a warning, but doesn't work.

 So assuming that sscanf in the mingw C library works consistently with
 snprintf, that might mean that pg_stat_statements is broken on that
 platform.  (The claim that %lld doesn't generate a warning is also
 questionable here.)

can't commend on that part without more investigation.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Compiler warnings with stringRelOpts (was WIP: Fast GiST index build)

[Heikki Linnakangas]

On 08.08.2011 22:11, Alvaro Herrera wrote:

Perhaps the easiest way to fix it is as you suggest, by declaring the
struct to take a pointer rather than the value directly.  Not sure how
to make both cases work sanely; the add_string_reloption path will need
updates.


Agreed, I propose the attached patch to do that. Are there any 
extensions out there that use add_string_relopt(), that I could use for 
testing?


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 4657425..900b222 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -371,8 +371,6 @@ allocate_reloption(bits32 kinds, int type, char *name, char *desc)
 	size_t		size;
 	relopt_gen *newoption;
 
-	Assert(type != RELOPT_TYPE_STRING);
-
 	oldcxt = MemoryContextSwitchTo(TopMemoryContext);
 
 	switch (type)
@@ -386,6 +384,9 @@ allocate_reloption(bits32 kinds, int type, char *name, char *desc)
 		case RELOPT_TYPE_REAL:
 			size = sizeof(relopt_real);
 			break;
+		case RELOPT_TYPE_STRING:
+			size = sizeof(relopt_string);
+			break;
 		default:
 			elog(ERROR, unsupported option type);
 			return NULL;		/* keep compiler quiet */
@@ -474,45 +475,29 @@ void
 add_string_reloption(bits32 kinds, char *name, char *desc, char *default_val,
 	 validate_string_relopt validator)
 {
-	MemoryContext oldcxt;
 	relopt_string *newoption;
-	int			default_len = 0;
-
-	oldcxt = MemoryContextSwitchTo(TopMemoryContext);
-
-	if (default_val)
-		default_len = strlen(default_val);
 
-	newoption = palloc0(sizeof(relopt_string) + default_len);
+	/* make sure the validator/default combination is sane */
+	if (validator)
+		(validator) (default_val);
 
-	newoption-gen.name = pstrdup(name);
-	if (desc)
-		newoption-gen.desc = pstrdup(desc);
-	else
-		newoption-gen.desc = NULL;
-	newoption-gen.kinds = kinds;
-	newoption-gen.namelen = strlen(name);
-	newoption-gen.type = RELOPT_TYPE_STRING;
+	newoption = (relopt_string *) allocate_reloption(kinds, RELOPT_TYPE_STRING,
+	 name, desc);
 	newoption-validate_cb = validator;
 	if (default_val)
 	{
-		strcpy(newoption-default_val, default_val);
-		newoption-default_len = default_len;
+		newoption-default_val = MemoryContextStrdup(TopMemoryContext,
+	 default_val);
+		newoption-default_len = strlen(default_val);
 		newoption-default_isnull = false;
 	}
 	else
 	{
-		newoption-default_val[0] = '\0';
+		newoption-default_val = ;
 		newoption-default_len = 0;
 		newoption-default_isnull = true;
 	}
 
-	/* make sure the validator/default combination is sane */
-	if (newoption-validate_cb)
-		(newoption-validate_cb) (newoption-default_val);
-
-	MemoryContextSwitchTo(oldcxt);
-
 	add_reloption((relopt_gen *) newoption);
 }
 
diff --git a/src/include/access/reloptions.h b/src/include/access/reloptions.h
index c7709cc..14f5034 100644
--- a/src/include/access/reloptions.h
+++ b/src/include/access/reloptions.h
@@ -108,7 +108,7 @@ typedef struct relopt_string
 	int			default_len;
 	bool		default_isnull;
 	validate_string_relopt validate_cb;
-	char		default_val[1]; /* variable length, zero-terminated */
+	char	   *default_val;
 } relopt_string;
 
 /* This is the table datatype for fillRelOptions */

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Compiler warnings with stringRelOpts (was WIP: Fast GiST index build)

[Heikki Linnakangas]

On 09.08.2011 13:25, Heikki Linnakangas wrote:

On 08.08.2011 22:11, Alvaro Herrera wrote:

Perhaps the easiest way to fix it is as you suggest, by declaring the
struct to take a pointer rather than the value directly. Not sure how
to make both cases work sanely; the add_string_reloption path will need
updates.


Agreed, I propose the attached patch to do that.


Committed this.

--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Compiler warnings with stringRelOpts (was WIP: Fast GiST index build)

[Alvaro Herrera]

Excerpts from Heikki Linnakangas's message of mar ago 09 08:32:43 -0400 2011:
 On 09.08.2011 13:25, Heikki Linnakangas wrote:
  On 08.08.2011 22:11, Alvaro Herrera wrote:
  Perhaps the easiest way to fix it is as you suggest, by declaring the
  struct to take a pointer rather than the value directly. Not sure how
  to make both cases work sanely; the add_string_reloption path will need
  updates.
 
  Agreed, I propose the attached patch to do that.
 
 Committed this.

Thanks.

I think I vaguely remember that the reason for doing it this way is that
the copy into the relcache worked, i.e. if the originals went away,
there was no dangling pointer.  Did you test this?

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Compiler warnings with stringRelOpts (was WIP: Fast GiST index build)

[Heikki Linnakangas]

On 09.08.2011 18:04, Alvaro Herrera wrote:

Excerpts from Heikki Linnakangas's message of mar ago 09 08:32:43 -0400 2011:

On 09.08.2011 13:25, Heikki Linnakangas wrote:

On 08.08.2011 22:11, Alvaro Herrera wrote:

Perhaps the easiest way to fix it is as you suggest, by declaring the
struct to take a pointer rather than the value directly. Not sure how
to make both cases work sanely; the add_string_reloption path will need
updates.


Agreed, I propose the attached patch to do that.


Committed this.


Thanks.

I think I vaguely remember that the reason for doing it this way is that
the copy into the relcache worked, i.e. if the originals went away,
there was no dangling pointer.  Did you test this?


These strings are never freed, so I don't think it's possible to end up 
with a dangling pointer.


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Compiler warnings with stringRelOpts (was WIP: Fast GiST index build)

[Tom Lane]

Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
 On 09.08.2011 18:04, Alvaro Herrera wrote:
 I think I vaguely remember that the reason for doing it this way is that
 the copy into the relcache worked, i.e. if the originals went away,
 there was no dangling pointer.  Did you test this?

 These strings are never freed, so I don't think it's possible to end up 
 with a dangling pointer.

Well, in that case the relevant question is whether we need to worry
about memory leaks due to multiple copies.

Personally I'm wondering why the function is strdup'ing the default
value at all.  In practice, wouldn't it practically always be a compile
time constant string?  Why create possible issues by designing the code
for a different use-case?  In particular, why not declare the default
value as const char * and specify that it's caller's responsibility
that it live forever if it's not just a constant string?

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Compiler warnings with stringRelOpts (was WIP: Fast GiST index build)

[Alexander Korotkov]

String-formatted relopts was never used before, but I've used it in
buffering GiST index build patch and encountered with following compiler
warnings:

reloptions.c:259: warning: initializer-string for array of chars is too long
reloptions.c:259: warning: (near initialization for
‘stringRelOpts[0].default_val’**)

It is caused by definition of default field of relopt_string structure as
1-length character array. This seems to be a design flaw in the reloptions.c
code. Any thoughts?

--
With best regards,
Alexander Korotkov.

Re: [HACKERS] Compiler warnings with stringRelOpts (was WIP: Fast GiST index build)

[Alvaro Herrera]

Excerpts from Alexander Korotkov's message of lun ago 08 06:27:33 -0400 2011:
 String-formatted relopts was never used before, but I've used it in
 buffering GiST index build patch and encountered with following compiler
 warnings:
 
 reloptions.c:259: warning: initializer-string for array of chars is too long
 reloptions.c:259: warning: (near initialization for
 ‘stringRelOpts[0].default_val’**)
 
 It is caused by definition of default field of relopt_string structure as
 1-length character array. This seems to be a design flaw in the reloptions.c
 code. Any thoughts?

Maybe this needs to use the new FLEXIBLE_ARRAY_MEMBER stuff.  Can you try that 
please?

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Compiler warnings with stringRelOpts (was WIP: Fast GiST index build)

[Alexander Korotkov]

On Mon, Aug 8, 2011 at 7:43 PM, Alvaro Herrera
alvhe...@commandprompt.comwrote:

 Maybe this needs to use the new FLEXIBLE_ARRAY_MEMBER stuff.  Can you try
 that please?


typedef struct relopt_string
{
relopt_gen gen;
int default_len;
 bool default_isnull;
validate_string_relopt validate_cb;
 char default_val[1]; /* variable length, zero-terminated */
} relopt_string;

static relopt_string stringRelOpts[] =
...

I doubt variable-length data structure is possible in this case, because we
don't have array of pointers to relopt_string, but just array
of relopt_string. May be just
char *default_val;
is possible?

--
With best regards,
Alexander Korotkov.

Re: [HACKERS] Compiler warnings with stringRelOpts (was WIP: Fast GiST index build)

[Alvaro Herrera]

Excerpts from Alexander Korotkov's message of lun ago 08 11:50:53 -0400 2011:
 On Mon, Aug 8, 2011 at 7:43 PM, Alvaro Herrera
 alvhe...@commandprompt.comwrote:
 
  Maybe this needs to use the new FLEXIBLE_ARRAY_MEMBER stuff.  Can you try
  that please?
 
 
 typedef struct relopt_string
 {
 relopt_gen gen;
 int default_len;
  bool default_isnull;
 validate_string_relopt validate_cb;
  char default_val[1]; /* variable length, zero-terminated */
 } relopt_string;
 
 static relopt_string stringRelOpts[] =
 ...
 
 I doubt variable-length data structure is possible in this case, because we
 don't have array of pointers to relopt_string, but just array
 of relopt_string. May be just
 char *default_val;
 is possible?

An array of relopt_string?  Isn't that a bit strange?  If I recall
correctly, the point of this was to be able to allocate the
relopt_string struct and the char array itself as a single palloc unit,
in a single call somewhere in the reloptions API (which was convoluted
in some points precisely to let the string case work).  I don't have the
details of this fresh in my mind though.  It certainly worked with more
than one string option when I committed it, IIRC.

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Compiler warnings with stringRelOpts (was WIP: Fast GiST index build)

[Alexander Korotkov]

On Mon, Aug 8, 2011 at 8:27 PM, Alvaro Herrera
alvhe...@commandprompt.comwrote:

 An array of relopt_string?  Isn't that a bit strange?  If I recall
 correctly, the point of this was to be able to allocate the
 relopt_string struct and the char array itself as a single palloc unit,
 in a single call somewhere in the reloptions API (which was convoluted
 in some points precisely to let the string case work).  I don't have the
 details of this fresh in my mind though.  It certainly worked with more
 than one string option when I committed it, IIRC.

Yes, it seems strange. But it also seems that both were added by your commit
of table-based parser:
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=ba748f7a11ef884277b61d1708a17a44acfd1736

--
With best regards,
Alexander Korotkov.

Re: [HACKERS] Compiler warnings with stringRelOpts (was WIP: Fast GiST index build)

[Alvaro Herrera]

Excerpts from Alexander Korotkov's message of lun ago 08 13:21:17 -0400 2011:
 On Mon, Aug 8, 2011 at 8:27 PM, Alvaro Herrera
 alvhe...@commandprompt.comwrote:
 
  An array of relopt_string?  Isn't that a bit strange?  If I recall
  correctly, the point of this was to be able to allocate the
  relopt_string struct and the char array itself as a single palloc unit,
  in a single call somewhere in the reloptions API (which was convoluted
  in some points precisely to let the string case work).  I don't have the
  details of this fresh in my mind though.  It certainly worked with more
  than one string option when I committed it, IIRC.
 
 Yes, it seems strange. But it also seems that both were added by your commit
 of table-based parser:
 http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=ba748f7a11ef884277b61d1708a17a44acfd1736

Oh, you're adding options directly to stringRelOpts?  Hmm, I can't
remember whether I tried to do that; I have memory of testing string
options via add_string_reloption ... and in reflection, it seems obvious
that it would fail.

Perhaps the easiest way to fix it is as you suggest, by declaring the
struct to take a pointer rather than the value directly.  Not sure how
to make both cases work sanely; the add_string_reloption path will need
updates.  I don't have time to work on it right now though.

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] compiler warnings with GCC 4.5

[Peter Eisentraut]

Only these few:

read.c: In function ‘nodeRead’:
read.c:370:3: warning: case value ‘101’ not in enumerated type
‘NodeTag’
read.c:300:3: warning: case value ‘102’ not in enumerated type
‘NodeTag’
read.c:294:3: warning: case value ‘103’ not in enumerated type
‘NodeTag’
read.c:374:3: warning: case value ‘104’ not in enumerated type
‘NodeTag’

This can be fixed by changing

switch (type)

to

switch ((int) type)


preproc.y: In function ‘base_yyparse’:
preproc.y:8043:19: warning: operation on ‘yyval.str’ may be undefined

The code in question looks like

|  SETOF SimpleTypename opt_array_bounds
{   $$ = $$ = cat_str(3, make_str(setof), $2, $3.str); }
^^^

It is converted from the main grammar, so it looks like a bug in the
conversion routine.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] compiler warnings with GCC 4.5

[Tom Lane]

Peter Eisentraut pete...@gmx.net writes:
 read.c: In function ‘nodeRead’:
 read.c:370:3: warning: case value ‘101’ not in enumerated type
 ‘NodeTag’

 This can be fixed by changing
 switch (type)
 to
 switch ((int) type)

No objection from here.  We don't attempt to cover all possible NodeTags
in that switch anyway, so I don't see that we're losing any error
detection capability by adding the cast.

 preproc.y: In function ‘base_yyparse’:
 preproc.y:8043:19: warning: operation on ‘yyval.str’ may be undefined

 The code in question looks like

 |  SETOF SimpleTypename opt_array_bounds
 {   $$ = $$ = cat_str(3, make_str(setof), $2, $3.str); }
 ^^^

 It is converted from the main grammar, so it looks like a bug in the
 conversion routine.

The double assignment looks inefficient, but I bet what it's really unhappy
about is that one of the cat_str arguments has a .str suffix and the
other doesn't --- why is that?

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Compiler warnings fix

[Magnus Hagander]

ITAGAKI Takahiro wrote:
 Here is a patch to surpress compiler warnings in pg_locale.c and pg_regress.c.
 
 There are following warnings if nls is enabled:
 pg_locale.c: In function `pg_perm_setlocale':
 pg_locale.c:161: warning: assignment discards qualifiers from pointer 
 target type
 and if nls is disabled:
 pg_locale.c:615: warning: 'IsoLocaleName' defined but not used
 
 There is also a warning in pg_regress.c:
 pg_regress.c: In function `wait_for_tests':
 pg_regress.c:1367: warning: passing arg 2 of `GetExitCodeProcess' from 
 incompatible pointer type

Applied (as two separate patches since I missed that there were two
files initially).

I agree with other comments that #ifdef:ing on LC_MESSAGES may not be
the greatest-looking solution, but the code that calls it is ifdef:ed
that way, so mimicking that seems like the right thing to do at this time.

//Magnus


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Compiler warnings fix

[Peter Eisentraut]

ITAGAKI Takahiro wrote:

Here is a patch to surpress compiler warnings in pg_locale.c and pg_regress.c.

There are following warnings if nls is enabled:
pg_locale.c: In function `pg_perm_setlocale':
pg_locale.c:161: warning: assignment discards qualifiers from pointer 
target type
and if nls is disabled:
pg_locale.c:615: warning: 'IsoLocaleName' defined but not used

There is also a warning in pg_regress.c:
pg_regress.c: In function `wait_for_tests':
pg_regress.c:1367: warning: passing arg 2 of `GetExitCodeProcess' from 
incompatible pointer type


Which platform, which compiler, what configure options?

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Compiler warnings fix

[ITAGAKI Takahiro]

Peter Eisentraut pete...@gmx.net wrote:

 ITAGAKI Takahiro wrote:
  Here is a patch to surpress compiler warnings in pg_locale.c and 
  pg_regress.c.
  
  There are following warnings if nls is enabled:
  pg_locale.c: In function `pg_perm_setlocale':
  pg_locale.c:161: warning: assignment discards qualifiers from pointer 
  target type
  and if nls is disabled:
  pg_locale.c:615: warning: 'IsoLocaleName' defined but not used
  
  There is also a warning in pg_regress.c:
  pg_regress.c: In function `wait_for_tests':
  pg_regress.c:1367: warning: passing arg 2 of `GetExitCodeProcess' from 
  incompatible pointer type
 
 Which platform, which compiler, what configure options?

It is in mingw, gcc.exe (GCC) 3.4.5 (mingw-vista special r3).

There are same warning on vaquita in buildfarm.
http://www.pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=vaquitadt=2009-01-26%20210011stg=make

Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Compiler warnings fix

[Andrew Dunstan]

ITAGAKI Takahiro wrote:

Peter Eisentraut pete...@gmx.net wrote:

  

ITAGAKI Takahiro wrote:


Here is a patch to surpress compiler warnings in pg_locale.c and pg_regress.c.

There are following warnings if nls is enabled:
pg_locale.c: In function `pg_perm_setlocale':
pg_locale.c:161: warning: assignment discards qualifiers from pointer 
target type
and if nls is disabled:
pg_locale.c:615: warning: 'IsoLocaleName' defined but not used

There is also a warning in pg_regress.c:
pg_regress.c: In function `wait_for_tests':
pg_regress.c:1367: warning: passing arg 2 of `GetExitCodeProcess' from 
incompatible pointer type
  

Which platform, which compiler, what configure options?



It is in mingw, gcc.exe (GCC) 3.4.5 (mingw-vista special r3).

There are same warning on vaquita in buildfarm.
http://www.pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=vaquitadt=2009-01-26%20210011stg=make


  


Wouldn't we be better off using defined(ENABLE_NLS) instead of 
defined(LC_MESSAGES) ?


cheers

andrew





--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Compiler warnings fix

[Tom Lane]

Andrew Dunstan and...@dunslane.net writes:
 There are same warning on vaquita in buildfarm.
 http://www.pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=vaquitadt=2009-01-26%20210011stg=make

 Wouldn't we be better off using defined(ENABLE_NLS) instead of 
 defined(LC_MESSAGES) ?

No, because the purpose of that #if is to prevent choking on the
references to LC_MESSAGES if it's not defined.  Whether ENABLE_NLS
is defined is 100% orthogonal to that.

Given the current usage it seems that the only way to avoid the
'IsoLocaleName' defined but not used warning is to compile it
conditionally on LC_MESSAGES as well as WIN32.  I agree that's
kind of ugly, but that's what the usage is.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Compiler warnings fix

[ITAGAKI Takahiro]

Here is a patch to surpress compiler warnings in pg_locale.c and pg_regress.c.

There are following warnings if nls is enabled:
pg_locale.c: In function `pg_perm_setlocale':
pg_locale.c:161: warning: assignment discards qualifiers from pointer 
target type
and if nls is disabled:
pg_locale.c:615: warning: 'IsoLocaleName' defined but not used

There is also a warning in pg_regress.c:
pg_regress.c: In function `wait_for_tests':
pg_regress.c:1367: warning: passing arg 2 of `GetExitCodeProcess' from 
incompatible pointer type

Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center


fix_warnings.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] compiler warnings in ecpglib/execute.c (uninitialized local variable 'prepname' used)

[Hannes Eder]

while rebuilding pgsql with msvc 2005 I noticed this compiler warning:

.\src\interfaces\ecpg\ecpglib\execute.c(1495): warning C4700: 
uninitialized local variable 'prepname' used


ECPGfree(prepname) is called in line 1495, prepname was not
unitialized befor. Below the line 1495 ECPGfree(prepname) is not
called in the function ECPGdo. I didn't investigate the code in
detail, but I assume that at least in some error conditions (when the
function returns false?) ECPGfree(prepname) should be called.

-Hannes


---(end of broadcast)---
TIP 7: You can help support the PostgreSQL project by donating at

   http://www.postgresql.org/about/donate

Re: [HACKERS] compiler warnings on the buildfarm

[Zdenek Kotala]

Stefan Kaltenbrunner napsal(a):

Zdenek Kotala wrote:

Stefan Kaltenbrunner wrote:

Zdenek Kotala wrote:

Stefan Kaltenbrunner wrote:

Zdenek Kotala wrote:

For sun studio -erroff=E_STATEMENT_NOT_REACHED is useful there. If you
want to determine warning tags for each warning add -errtags.

Is that supported on all versions of sun studio(Sun WorkShop 6, Sun
Studio 8,11) we have on the farm ?

Yes. Also on SS12.

hmm - sure about that ? I was about to submit a patch to disable some
compiler warnings but then I noted the following discussion thread:

http://forum.java.sun.com/thread.jspa?threadID=5163903messageID=9637391

which seems to indicate that at least the compiler installed on kudu:

http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=kududt=2007-07-15%2003:30:01


does NOT support turning of specific warnings.


I tested it on cc version 5.3 and it works. See


ah cool - thanks for testing!

so on my box we would need to add
-erroff=E_EMPTY_TRANSLATION_UNIT,E_STATEMENT_NOT_REACHED,E_END_OF_LOOP_CODE_NOT_REACHED,E_FUNC_HAS_NO_RETURN_STMT,E_LOOP_NOT_ENTERED_AT_TOP

to CFLAGS to get down to the following 2 warnings:

pgstat.c, line 652: warning: const object should have initializer:
all_zeroes (E_CONST_OBJ_SHOULD_HAVE_INITIZR)
pgstat.c, line 2118: warning: const object should have initializer:
all_zeroes (E_CONST_OBJ_SHOULD_HAVE_INITIZR)

the open question is if that is what want or if we would be simply
adding (unnecessary) complexity (or confusion).

comments ?


E_STATEMENT_NOT_REACHED,E_END_OF_LOOP_CODE_NOT_REACHED, E_EMPTY_TRANSLATION_UNIT 
are probably ok to ignore.
E_FUNC_HAS_NO_RETURN_STMT is there because main is leaved by exit() instead 
return. And In another case It should be regular warning.



I think good solution is compare previous warning log with latest build and make 
a diff. If some new warning appears it is probably regular warning.


Zdenek

---(end of broadcast)---
TIP 6: explain analyze is your friend

Re: [HACKERS] compiler warnings on the buildfarm

[Tom Lane]

Zdenek Kotala [EMAIL PROTECTED] writes:
 E_FUNC_HAS_NO_RETURN_STMT is there because main is leaved by exit() instead 
 return. And In another case It should be regular warning.

That should be gone now; I changed the two places that triggered it.
I'd suggest not disabling that warning.

regards, tom lane

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match

Re: [HACKERS] compiler warnings on the buildfarm

[Zdenek Kotala]

Tom Lane napsal(a):

Zdenek Kotala [EMAIL PROTECTED] writes:
E_FUNC_HAS_NO_RETURN_STMT is there because main is leaved by exit() instead 
return. And In another case It should be regular warning.


That should be gone now; I changed the two places that triggered it.
I'd suggest not disabling that warning.


Yes I agree. Did you also clean up on old branches? If not I think we can live 
with this warning on old branches.



Zdenek

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
  subscribe-nomail command to [EMAIL PROTECTED] so that your
  message can get through to the mailing list cleanly

Re: [HACKERS] compiler warnings on the buildfarm

[Tom Lane]

Zdenek Kotala [EMAIL PROTECTED] writes:
 Tom Lane napsal(a):
 That should be gone now; I changed the two places that triggered it.
 I'd suggest not disabling that warning.

 Yes I agree. Did you also clean up on old branches?

No, I'm not interested in doing that kind of fiddling on old branches.
I think we only care about warnings in HEAD.  (Unless an actual bug is
exposed, of course, in which case we'd back-patch the fix as appropriate.)

regards, tom lane

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
   subscribe-nomail command to [EMAIL PROTECTED] so that your
   message can get through to the mailing list cleanly

Re: [HACKERS] compiler warnings on the buildfarm

[Gregory Stark]

Do any of the build farm machines not support 64-bit integers? I just added a
--enable-bigint flag to configure.in and tested building without it and got an
error at xlog.c:

xlog.c: In function 'ValidXLOGHeader':
xlog.c:3240: error: 'UINT64_FORMAT' undeclared (first use in this function)
xlog.c:3240: error: (Each undeclared identifier is reported only once
xlog.c:3240: error: for each function it appears in.)

snprintf(fhdrident_str, sizeof(fhdrident_str), UINT64_FORMAT,
 longhdr-xlp_sysid);
snprintf(sysident_str, sizeof(sysident_str), UINT64_FORMAT,
 ControlFile-system_identifier);

It's possible I've done the autoconf hackery wrong though. Should
UINT64_FORMAT still be defined if there's no int64?

-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com


---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org

Re: [HACKERS] compiler warnings on the buildfarm

[Tom Lane]

Gregory Stark [EMAIL PROTECTED] writes:
 It's possible I've done the autoconf hackery wrong though. Should
 UINT64_FORMAT still be defined if there's no int64?

Yes.

regards, tom lane

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match

Re: [HACKERS] compiler warnings on the buildfarm

[Zdenek Kotala]

Stefan Kaltenbrunner wrote:

Zdenek Kotala wrote:

Stefan Kaltenbrunner wrote:

Zdenek Kotala wrote:

For sun studio -erroff=E_STATEMENT_NOT_REACHED is useful there. If you
want to determine warning tags for each warning add -errtags.

Is that supported on all versions of sun studio(Sun WorkShop 6, Sun
Studio 8,11) we have on the farm ?

Yes. Also on SS12.


hmm - sure about that ? I was about to submit a patch to disable some
compiler warnings but then I noted the following discussion thread:

http://forum.java.sun.com/thread.jspa?threadID=5163903messageID=9637391

which seems to indicate that at least the compiler installed on kudu:

http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=kududt=2007-07-15%2003:30:01

does NOT support turning of specific warnings.



I tested it on cc version 5.3 and it works. See

-bash-3.00$ SUNWspro/SC6.2/bin/cc -V
cc: Sun WorkShop 6 update 2 C 5.3 Patch 111680-09 2003/05/18
usage: cc [ options] files.  Use 'cc -flags' for details
-bash-3.00$ /ws/onnv-tools/SUNWspro/SC6.2/bin/cc pokus.c
pokus.c, line 5: warning: statement not reached
-bash-3.00$ /ws/onnv-tools/SUNWspro/SC6.2/bin/cc -errtags pokus.c
pokus.c, line 5: warning: statement not reached (E_STATEMENT_NOT_REACHED)
-bash-3.00$ /ws/onnv-tools/SUNWspro/SC6.2/bin/cc 
-erroff=E_STATEMENT_NOT_REACHED pokus.c

-bash-3.00$


It works since Sun Workshop 4.2 (cc: WorkShop Compilers 4.2 26 Jun 1997 
C 4.2 patch 105062-01). I tested it also on SunWorkshop 2.0.1 and it 
does not work there, but I belive that nobody uses ten years old 
compiler :-).


Please, can you send me a cc -V output ( I think It should be added in log).

Zdenek

---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

  http://www.postgresql.org/docs/faq

Re: [HACKERS] compiler warnings on the buildfarm

[Stefan Kaltenbrunner]

Zdenek Kotala wrote:
 Stefan Kaltenbrunner wrote:
 Zdenek Kotala wrote:
 Stefan Kaltenbrunner wrote:
 Zdenek Kotala wrote:
 For sun studio -erroff=E_STATEMENT_NOT_REACHED is useful there. If you
 want to determine warning tags for each warning add -errtags.
 Is that supported on all versions of sun studio(Sun WorkShop 6, Sun
 Studio 8,11) we have on the farm ?
 Yes. Also on SS12.

 hmm - sure about that ? I was about to submit a patch to disable some
 compiler warnings but then I noted the following discussion thread:

 http://forum.java.sun.com/thread.jspa?threadID=5163903messageID=9637391

 which seems to indicate that at least the compiler installed on kudu:

 http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=kududt=2007-07-15%2003:30:01


 does NOT support turning of specific warnings.

 
 I tested it on cc version 5.3 and it works. See

ah cool - thanks for testing!

so on my box we would need to add
-erroff=E_EMPTY_TRANSLATION_UNIT,E_STATEMENT_NOT_REACHED,E_END_OF_LOOP_CODE_NOT_REACHED,E_FUNC_HAS_NO_RETURN_STMT,E_LOOP_NOT_ENTERED_AT_TOP

to CFLAGS to get down to the following 2 warnings:

pgstat.c, line 652: warning: const object should have initializer:
all_zeroes (E_CONST_OBJ_SHOULD_HAVE_INITIZR)
pgstat.c, line 2118: warning: const object should have initializer:
all_zeroes (E_CONST_OBJ_SHOULD_HAVE_INITIZR)

the open question is if that is what want or if we would be simply
adding (unnecessary) complexity (or confusion).

comments ?


Stefan

---(end of broadcast)---
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate

Re: [HACKERS] compiler warnings on the buildfarm

[Gregory Stark]

Stefan Kaltenbrunner [EMAIL PROTECTED] writes:

 pgstat.c, line 652: warning: const object should have initializer:
 all_zeroes (E_CONST_OBJ_SHOULD_HAVE_INITIZR)
 pgstat.c, line 2118: warning: const object should have initializer:
 all_zeroes (E_CONST_OBJ_SHOULD_HAVE_INITIZR)

Man, even these are bogus. And that would be an interesting warning too if
they made it not fire when it's bogus. bleagh.

-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com


---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster

Re: [HACKERS] compiler warnings on the buildfarm

[Stefan Kaltenbrunner]

Zdenek Kotala wrote:
 Stefan Kaltenbrunner wrote:
 Zdenek Kotala wrote:
 
 For sun studio -erroff=E_STATEMENT_NOT_REACHED is useful there. If you
 want to determine warning tags for each warning add -errtags.

 Is that supported on all versions of sun studio(Sun WorkShop 6, Sun
 Studio 8,11) we have on the farm ?
 
 Yes. Also on SS12.

hmm - sure about that ? I was about to submit a patch to disable some
compiler warnings but then I noted the following discussion thread:

http://forum.java.sun.com/thread.jspa?threadID=5163903messageID=9637391

which seems to indicate that at least the compiler installed on kudu:

http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=kududt=2007-07-15%2003:30:01

does NOT support turning of specific warnings.


Stefan

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org

Re: [HACKERS] compiler warnings on the buildfarm

[Tom Lane]

Stefan Kaltenbrunner [EMAIL PROTECTED] writes:
 Tom Lane wrote:
 What I suspect is happening is that lionfish is running the buildfarm
 script in a non-C locale, in which flex finds that some high-bit-set
 characters are case-folded by tolower() and accordingly issues this
 complaint.  Now the statements that it assumes you meant the literal
 numeric range and that the behavior is fully determined at compile time
 (ie, no run-time invocations of tolower(), as indeed are not to be seen
 in pl_scan.c) seem to mean that we'll get the behavior we want anyway.
 But the warning is a bit nervous-making.

 hmmm - note that lionfish is not the only box reporting that kind of
 warning - also affected are:
 rosella (which is definitly running in a non-C locale as all the errors
 are in german there)
 wildebeest

I looked at the flex source code and it seems that indeed we *should*
expect to see that warning if we run flex in a locale in which any
characters in the range \200-\377 are letters that case-fold to plain
ASCII.  Turkish ought to meet that criterion (the ol dotted vs dotless
i business) but I'm not sure about German.  I could not get the warning
on plpgsql's scan.l with a local build of flex 2.5.33, though, no matter
what locale I ran it in.  Odd.

Anyway, I tweaked plpgsql's Makefile to force LC_CTYPE=C, which
theoretically should silence this warning.

regards, tom lane

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org

Re: [HACKERS] compiler warnings on the buildfarm

[Tom Lane]

Stefan Kaltenbrunner [EMAIL PROTECTED] writes:
 and this is the initial list for contrib(excluding a lot of duplicate
 warnings and stuff that is a result of invalid compiler flags which I
 will mention seperatly):

I fixed most of these, I believe.  A couple remain untouched:

 animal: cuckoo  warnings: 9
 y.tab.c: In function 'yy_reduce_print':
 y.tab.c:764: warning: passing argument 3 of 'yy_symbol_print' from
 incompatible pointer type

I don't see this on either PPC or Intel Mac.  I think the problem is
probably an old bison version on this buildfarm member.

 animal: dugong  warnings: 21
 ../../src/include/storage/s_lock.h(246): warning #167: argument of type
 volatile slock_t={unsigned int} * is incompatible with
 parameter of type void *
 ret = _InterlockedExchange(lock,1);
^

I see this is not just contrib but throughout the core too on this
machine.  We could possibly suppress it by casting away volatile in the
tas() function, but I wonder if that might have bad side-effects.  Any
thoughts?

 pg_buffercache_pages.c(116): warning #188: enumerated type mixed with
 another type
 LWLockAcquire(FirstBufMappingLock + i, LW_SHARED);
   ^

This warning occurs in too many places to want to fix, also.

regards, tom lane

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster

Re: [HACKERS] compiler warnings on the buildfarm

[Tom Lane]

Gregory Stark [EMAIL PROTECTED] writes:
 Tom Lane [EMAIL PROTECTED] writes:
 Anyway, I tweaked plpgsql's Makefile to force LC_CTYPE=3DC, which
 theoretically should silence this warning.

 This doesn't mean that people were previousy able to use any of these exot=
 ic
 characters like a=DFertion or expla=EFn if they happened to compile in the =
 wrong
 (or right) locale and now they won't be able to does it?

No.

regards, tom lane

---(end of broadcast)---
TIP 6: explain analyze is your friend

Re: [HACKERS] compiler warnings on the buildfarm

[Tom Lane]

Chris Browne [EMAIL PROTECTED] writes:
 Tom Lane wrote:
 animal: grebe   warnings: 45
 xlog.c:651: warning: implicit declaration of function '_check_lock'
 xlog.c:654: warning: implicit declaration of function '_clear_lock'
 hba.c:1449: warning: implicit declaration of function 'getpeereid'

 Someone needs to find out which system headers declare these functions
 on AIX.

 Hmm.  Logging onto grebe:

 /usr/include/sys/socket.h:int getpeereid(int, uid_t *__restrict__, gid_t 
 *__restrict__);

That's pretty strange, because hba.c definitely includes sys/socket.h.
Perhaps getpeereid is hidden within some #ifdef that we aren't setting?

 /usr/include/sys/atomic_op.h:boolean_t _check_lock();
 /usr/include/sys/atomic_op.h:void _clear_lock();

OK, I'll try putting sys/atomic_op.h into the AIX part of s_lock.h.

regards, tom lane

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match

Re: [HACKERS] compiler warnings on the buildfarm

[Stefan Kaltenbrunner]

Tom Lane wrote:
 Stefan Kaltenbrunner [EMAIL PROTECTED] writes:
 animal: lionfishwarnings: 16
 scan.l:180: warning, the character range [80-FF] is ambiguous in a
 case-insensitive scanner
 scan.l:180: warning, the character range [80-FF] is ambiguous in a
 case-insensitive scanner
 scan.l:302: warning, the character range [80-FF] is ambiguous in a
 case-insensitive scanner
 
 This is evidently complaining about plpgsql's scan.l, which specifies
 %option case-insensitive
 and then defines
 ident_start   [A-Za-z\200-\377_]
 which is the way we do it in the main grammar too.  But I've never
 seen this message in any of the flex versions I've used with PG.
 (Which flex version is installed on lionfish anyway?)

$ flex -V
flex 2.5.31



 
 I find some relevant points in the flex manual:
 http://flex.sourceforge.net/manual/Patterns.html
 
   Character classes are expanded immediately when seen in the flex
   input. This means the character classes are sensitive to the locale in
   which flex is executed, and the resulting scanner will not be sensitive
   to the runtime locale. This may or may not be desirable.
   
   Character classes with ranges, such as `[a-Z]', should be used with
   caution in a case-insensitive scanner if the range spans upper or
   lowercase characters. Flex does not know if you want to fold all upper
   and lowercase characters together, or if you want the literal numeric
   range specified (with no case folding). When in doubt, flex will assume
   that you meant the literal numeric range, and will issue a warning. The
   exception to this rule is a character range such as `[a-z]' or `[S-W]'
   where it is obvious that you want case-folding to occur.
 
 What I suspect is happening is that lionfish is running the buildfarm
 script in a non-C locale, in which flex finds that some high-bit-set
 characters are case-folded by tolower() and accordingly issues this
 complaint.  Now the statements that it assumes you meant the literal
 numeric range and that the behavior is fully determined at compile time
 (ie, no run-time invocations of tolower(), as indeed are not to be seen
 in pl_scan.c) seem to mean that we'll get the behavior we want anyway.
 But the warning is a bit nervous-making.

hmmm - note that lionfish is not the only box reporting that kind of
warning - also affected are:

rosella (which is definitly running in a non-C locale as all the errors
are in german there)
wildebeest

Stefan

---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq

Re: [HACKERS] compiler warnings on the buildfarm

[Stefan Kaltenbrunner]

Tom Lane wrote:
 Stefan Kaltenbrunner [EMAIL PROTECTED] writes:
 some more(I have removed duplicates and ones that should be fixed by
 your latest commits though):
 
 I did what I could with this batch.  Some comments:
 
 animal: salamander  warnings: 27
 cash.c: In function `cash_in':
 cash.c:244: warning: subscript has type `char'
 
 I wish we could promote this one to be a hard error :-(.  It typically
 indicates (and did in this case) that someone has unportably forgotten
 to cast the argument of a ctype.h macro to unsigned char.

:-( we can promote certain warnings to an error on sun studio vor
example but salamander is running gcc ...

 
 pg_lzcompress.c: In function `pglz_compress':
 pg_lzcompress.c:378: warning: inlining failed in call to `pglz_find_match'
 
 This is not an error condition, it just means that gcc decided not to do
 inlining because the called function was too big.  IIRC we had some
 discussion whether to specify -Winline or not, and decided to do so in
 order to gather some info about whether we were overstressing inline.
 We could live with it as-is, or document somewhere (where?) that it's
 fine as long as you don't see very many of 'em, or decide that the
 experiment is finished and we should take out -Winline.  Comments?

we have about 5 boxes on the farm with that exact warning (inlining
failed in pglz_find_match) there are no other inline related warnings at
all afaiks.

 animal: lionfishwarnings: 16
 /tmp/cclwN8N9.s: Assembler messages:
 /tmp/cclwN8N9.s:109082: Warning: Macro instruction expanded into
 multiple instructions
 [multiple occurrences]
 
 This is pretty strange.  It seems to occur only in files generated
 from bison and/or flex.  Anybody have a clue?

other than that lionfish is a weird mips box - no :-)


Stefan

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
   subscribe-nomail command to [EMAIL PROTECTED] so that your
   message can get through to the mailing list cleanly

Re: [HACKERS] compiler warnings on the buildfarm

[Stefan Kaltenbrunner]

Tom Lane wrote:
[...]
 animal: clownfish   warnings: 12
 dynloader.c, line 4: warning: empty translation unit
 postgres.c, line 3758: warning: loop not entered at top
 
 The first of these is not a bug, the second seems to be some weird
 aberration in their statement-not-reached detection.

will see about filtering out those

 
 animal: grebe   warnings: 45
 xlog.c:651: warning: implicit declaration of function '_check_lock'
 xlog.c:654: warning: implicit declaration of function '_clear_lock'
 hba.c:1449: warning: implicit declaration of function 'getpeereid'
 
 Someone needs to find out which system headers declare these functions
 on AIX.
 
 ip.c: In function 'getaddrinfo_unix':
 ip.c:254: warning: large integer implicitly truncated to unsigned type
 
 This is complaining about
 
 #ifdef HAVE_STRUCT_SOCKADDR_STORAGE_SS_LEN
   unp-sun_len = sizeof(struct sockaddr_un);
 #endif
 
 I don't know how wide sun_len is on this platform.  It's probably uint8,
 but if we explicitly cast the sizeof to 8 bits, we could conceivably
 break things on other platforms.  Are there any where sockaddr_un is
 longer than 255 bytes?  Anyway I'm inclined to leave this alone.

no idea on AIX but I have added christopher to the CC list - maybe he
can shed some light on those things.

 
 guc.c:2866: warning: 'guc_get_index' defined but not used
 Extra instructions are being generated for each reference to a TOC
 symbol if the symbol is in the TOC overflow area.
 
 This is fairly bizarre, since 'guc_get_index' *is* used in guc-file.c,
 which is included into this same file.  However I don't much like the
 coding method used here (it is certainly not better than using a
 temporary flag bit), so when I get a chance I'll rewrite it out of
 existence.
 
 connect.c:23: warning: missing braces around initializer
 connect.c:23: warning: (near initialization for
 'actual_connection_key_once.__on_word')
 misc.c:67: warning: missing braces around initializer
 misc.c:67: warning: (near initialization for 'sqlca_key_once.__on_word')
 
 I think these are a platform bug.  The spec clearly says that
 
 static pthread_once_t actual_connection_key_once = PTHREAD_ONCE_INIT;
 
 is exactly how you are supposed to do it.  If pthread_once_t is a struct
 on a given platform, that platform ought to be defining
 PTHREAD_ONCE_INIT with the appropriate braces included.  If we added
 braces ourselves we'd break it for platforms where the macro is correct
 already.  Hence, not our problem.

I see


Stefan

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match

Re: [HACKERS] compiler warnings on the buildfarm

[Stefan Kaltenbrunner]

Tom Lane wrote:
 Stefan Kaltenbrunner [EMAIL PROTECTED] writes:
 ok I did that for a few members (removing all the statement not reached
 ones as well as some purely informal notices and all the flex related
 warnings) and came up with something similiar to:
 [snip]
 
 Yeah, this looks like a good list.  I can't readily check the ones from
 eel as they appear to be in Windows-specific code; anyone else want to
 fix those?

and this is the initial list for contrib(excluding a lot of duplicate
warnings and stuff that is a result of invalid compiler flags which I
will mention seperatly):

animal: salamander  warnings: 6
stopword.c: In function `readstoplist':
stopword.c:51: warning: subscript has type `char'

animal: dragonfly   warnings: 4
pgbench.c: In function `main':
pgbench.c:1445: warning: int format, pid_t arg (arg 4)
stopword.c: In function `readstoplist':
stopword.c:51: warning: subscript has type `char'


animal: clownfish   warnings: 12
crc32.c, line 93: warning: initializer does not fit or is out of range: -1
crc32.c, line 102: warning: initializer does not fit or is out of
range: -1
imath.c, line 3202: warning: integer overflow detected: op 
imath.c, line 3206: warning: integer overflow detected: op 
query_cleanup.c, line 179: warning: macro redefined: V_FALSE
crc32.c, line 95: warning: initializer does not fit or is out of range: -1
query_support.c, line 199: warning: syntax error:  empty declaration
query_support.c, line 200: warning: syntax error:  empty declaration
query_support.c, line 201: warning: syntax error:  empty declaration
query_support.c, line 202: warning: syntax error:  empty declaration
query_support.c, line 203: warning: syntax error:  empty declaration
query_support.c, line 204: warning: syntax error:  empty declaration


animal: kuduwarnings: 13
crc32.c, line 93: warning: initializer does not fit or is out of range: -1
crc32.c, line 102: warning: initializer does not fit or is out of
range: -1
oid2name.c, line 579: warning: Function has no return statement : main
pg_standby.c, line 622: warning: Function has no return statement : main
imath.c, line 3202: warning: integer overflow detected: op 
dict_thesaurus.c, line 699: warning: non-constant initializer: op NAME
crc32.c, line 95: warning: initializer does not fit or is out of range: -1
query_support.c, line 199: warning: syntax error:  empty declaration
query_support.c, line 200: warning: syntax error:  empty declaration
query_support.c, line 201: warning: syntax error:  empty declaration
query_support.c, line 202: warning: syntax error:  empty declaration
query_support.c, line 203: warning: syntax error:  empty declaration
query_support.c, line 204: warning: syntax error:  empty declaration

animal: warthog warnings: 396
UX:acomp: WARNING: btreefuncs.c, line 59: no macro replacement within
a string literal
UX:acomp: WARNING: pgstatindex.c, line 50: no macro replacement within
a string literal
UX:acomp: WARNING: xpath.c, line 212: argument #1 incompatible with
prototype: strlen()
UX:acomp: WARNING: xpath.c, line 268: argument #2 incompatible with
prototype: xmlBufferWriteChar()
UX:acomp: WARNING: xpath.c, line 607: argument #1 incompatible with
prototype: xmlStrdup()
UX:acomp: WARNING: xpath.c, line 612: argument #1 incompatible with
prototype: strlen()
UX:acomp: WARNING: xpath.c, line 663: assignment type mismatch
UX:acomp: WARNING: xpath.c, line 738: assignment type mismatch
UX:acomp: WARNING: xpath.c, line 742: argument #1 incompatible with
prototype: strstr()
UX:acomp: WARNING: xpath.c, line 742: argument #2 incompatible with
prototype: strstr()
UX:acomp: WARNING: xpath.c, line 742: assignment type mismatch
UX:acomp: WARNING: xpath.c, line 896: argument #1 incompatible with
prototype: xmlStrdup()
UX:acomp: WARNING: xpath.c, line 904: assignment type mismatch
UX:acomp: WARNING: xslt_proc.c, line 105: argument #1 incompatible
with prototype: xsltParseStylesheetFile()


animal: emperor_mothwarnings: 11
pgbench.c: In function `main':
pgbench.c:1445: warning: int format, pid_t arg (arg 4)
query_cleanup.c:179:1: warning: V_FALSE redefined
In file included from /usr/include/sys/stream.h:22,
 from /usr/include/netinet/in.h:66,
 from /usr/include/netdb.h:98,
 from ../../src/include/port.h:17,
 from ../../src/include/c.h:839,
 from ../../src/include/postgres.h:48,
 from query_cleanup.c:6:
/usr/include/sys/vnode.h:505:1: warning: this is the location of the
previous definition

animal: cuckoo  warnings: 9
y.tab.c: In function 'yy_reduce_print':
y.tab.c:764: warning: passing argument 3 of 'yy_symbol_print' from
incompatible pointer type
y.tab.c: In function 'yydestruct':
y.tab.c:1036: warning: passing argument 3 of 'yy_symbol_print' from
incompatible pointer type
y.tab.c: In function 'cube_yyparse':
y.tab.c:1277: warning: passing argument 3 of 'yy_symbol_print' from

Re: [HACKERS] compiler warnings on the buildfarm

[Tom Lane]

Heikki Linnakangas [EMAIL PROTECTED] writes:
 Zdenek Kotala wrote:
 I don't see any const keyword there.

 Right after that:

 where int conv(int num_msg, const struct pam_message **msg, struct 
 pam_response **resp, void *appdata_ptr); 

 How confusing...

And the pam_start page he cited earlier has a different set of typos in
its version of the struct :-(.  Still, that's two out of three places
that say it's const, and Solaris appears to be the only implementation
that has chosen to read it as not const.

regards, tom lane

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster

Re: [HACKERS] compiler warnings on the buildfarm

[Zdenek Kotala]

Heikki Linnakangas wrote:

Zdenek Kotala wrote:
If I look there 
http://www.opengroup.org/onlinepubs/008329799/chap5.htm#tagcjh_06


in Call Back Information section. The structure is defined as

struct pam_conv{ int (*conv) (int, struct pam_message **, struct 
pam_response **, void *); void *appdata_ptr; };



I don't see any const keyword there.


Right after that:

 where int conv(int num_msg, const struct pam_message **msg, 
struct pam_response **resp, void *appdata_ptr); 


Ups, I overlooked it.


How confusing...


Yes agree.

Zdenek



---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

  http://www.postgresql.org/docs/faq

Re: [HACKERS] compiler warnings on the buildfarm

[Kris Jurka]

On Fri, 13 Jul 2007, Zdenek Kotala wrote:


Tom Lane wrote:

Kris Jurka [EMAIL PROTECTED] writes:

So pam_message ** isn't const.


Ah, thanks.  I see luna_moth is giving the same warning, so it's still
not const in Solaris 11 either.

Is it worth working around this?  It's strictly cosmetic AFAICS.

The main issue in my mind would be how to determine whether to use
const or not.  If all Solaris releases are like this, and can be
expected to stay that way,


I think yes. It is defined as X/Open standard says.



Not according to the link you sent earlier.  My reading says that Solaris 
has it defined wrong and pg has it right.


Kris Jurka

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
  choose an index scan if your joining column's datatypes do not
  match

Re: [HACKERS] compiler warnings on the buildfarm

[Zdenek Kotala]

Stefan Kaltenbrunner wrote:

Zdenek Kotala wrote:



For sun studio -erroff=E_STATEMENT_NOT_REACHED is useful there. If you
want to determine warning tags for each warning add -errtags.


Is that supported on all versions of sun studio(Sun WorkShop 6, Sun
Studio 8,11) we have on the farm ?


Yes. Also on SS12.

Zdenek

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
  choose an index scan if your joining column's datatypes do not
  match

Re: [HACKERS] compiler warnings on the buildfarm

[Heikki Linnakangas]

Zdenek Kotala wrote:
If I look there 
http://www.opengroup.org/onlinepubs/008329799/chap5.htm#tagcjh_06


in Call Back Information section. The structure is defined as

struct pam_conv{ int (*conv) (int, struct pam_message **, struct 
pam_response **, void *); void *appdata_ptr; };



I don't see any const keyword there.


Right after that:

 where int conv(int num_msg, const struct pam_message **msg, struct pam_response **resp, void *appdata_ptr); 


How confusing...

--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings

Re: [HACKERS] compiler warnings on the buildfarm

[Zdenek Kotala]

Kris Jurka wrote:



On Fri, 13 Jul 2007, Zdenek Kotala wrote:


Tom Lane wrote:

Kris Jurka [EMAIL PROTECTED] writes:

So pam_message ** isn't const.


Ah, thanks.  I see luna_moth is giving the same warning, so it's still
not const in Solaris 11 either.

Is it worth working around this?  It's strictly cosmetic AFAICS.

The main issue in my mind would be how to determine whether to use
const or not.  If all Solaris releases are like this, and can be
expected to stay that way,


I think yes. It is defined as X/Open standard says.



Not according to the link you sent earlier.  My reading says that 
Solaris has it defined wrong and pg has it right.


If I look there 
http://www.opengroup.org/onlinepubs/008329799/chap5.htm#tagcjh_06


in Call Back Information section. The structure is defined as

struct pam_conv{ int (*conv) (int, struct pam_message **, struct 
pam_response **, void *); void *appdata_ptr; };



I don't see any const keyword there.


Zdenek


---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
  choose an index scan if your joining column's datatypes do not
  match

Re: [HACKERS] compiler warnings on the buildfarm

[Zdenek Kotala]

Tom Lane wrote:

Kris Jurka [EMAIL PROTECTED] writes:

So pam_message ** isn't const.


Ah, thanks.  I see luna_moth is giving the same warning, so it's still
not const in Solaris 11 either.

Is it worth working around this?  It's strictly cosmetic AFAICS.

The main issue in my mind would be how to determine whether to use
const or not.  If all Solaris releases are like this, and can be
expected to stay that way,


I think yes. It is defined as X/Open standard says.


I'd be inclined to just put a #define
PAM_CONV_PROC_NOT_CONST in include/port/solaris.h and drive the
function declaration off that.  If there's a version dependency
involved then it gets a lot more complicated, and might not be worth
the trouble.


Following patch works for me, but I did not test it on other platform.


retrieving revision 1.153
diff -r1.153 auth.c
61c61
   pam_passwd_conv_proc,
---
   (int (*)())pam_passwd_conv_proc,


Zdenek

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster

Re: [HACKERS] compiler warnings on the buildfarm

[Zdenek Kotala]

Stefan Kaltenbrunner wrote:

Peter Eisentraut wrote:

Am Donnerstag, 12. Juli 2007 15:25 schrieb Stefan Kaltenbrunner:

a lot of those are simply noise (like the LOOP VECTORIZED stuff from the
icc boxes or the statement not reached spam from the sun compilers)
but others might indicate real issues.
To find warnings that might be a real problem we might want to look into
 suppressing those - if possible -  using compiler switches.
It would be good to determine an appropriate set of compiler switches to 
reduce the warnings to a reasonable level.


yeah once we have determined that this whole experiment is useful it
should be pretty easy to tweak the compiler switches for the non-gcc
compilers (mostly icc and sun studio seem to be the ones that generate
excessive output).



For sun studio -erroff=E_STATEMENT_NOT_REACHED is useful there. If you 
want to determine warning tags for each warning add -errtags.



Zdenek

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
  subscribe-nomail command to [EMAIL PROTECTED] so that your
  message can get through to the mailing list cleanly

Re: [HACKERS] compiler warnings on the buildfarm

[Zdenek Kotala]

Tom Lane wrote:

Heikki Linnakangas [EMAIL PROTECTED] writes:

Zdenek Kotala wrote:

I don't see any const keyword there.



Right after that:


where int conv(int num_msg, const struct pam_message **msg, struct pam_response **resp, void *appdata_ptr); 



How confusing...


And the pam_start page he cited earlier has a different set of typos in
its version of the struct :-(.  Still, that's two out of three places
that say it's const, and Solaris appears to be the only implementation
that has chosen to read it as not const.


Yes, Agree. I try to send request to security team for explanation. 
Maybe original author also overlooked it as me today :-).


Zdenek

---(end of broadcast)---
TIP 7: You can help support the PostgreSQL project by donating at

   http://www.postgresql.org/about/donate

Re: [HACKERS] compiler warnings on the buildfarm

[Zdenek Kotala]

Kris Jurka wrote:



On Thu, 12 Jul 2007, Tom Lane wrote:

static int pam_passwd_conv_proc(int num_msg, const struct pam_message 
** msg,

struct pam_response ** resp, void *appdata_ptr);

which exactly matches what my Fedora 6 pam header file says it should
be.  What is it on those Solaris machines?


struct pam_conv {
int (*conv)(int, struct pam_message **,
struct pam_response **, void *);
void *appdata_ptr;  /* Application data ptr */
};

So pam_message ** isn't const.



Yes, according to X/Open XSSO Standard - see 
http://www.opengroup.org/onlinepubs/008329799/

http://www.opengroup.org/onlinepubs/008329799/pam_start.htm#tagcjh_07_32

Zdenek

---(end of broadcast)---
TIP 6: explain analyze is your friend

Re: [HACKERS] compiler warnings on the buildfarm

[Stefan Kaltenbrunner]

Zdenek Kotala wrote:
 Stefan Kaltenbrunner wrote:
 Peter Eisentraut wrote:
 Am Donnerstag, 12. Juli 2007 15:25 schrieb Stefan Kaltenbrunner:
 a lot of those are simply noise (like the LOOP VECTORIZED stuff from
 the
 icc boxes or the statement not reached spam from the sun compilers)
 but others might indicate real issues.
 To find warnings that might be a real problem we might want to look
 into
  suppressing those - if possible -  using compiler switches.
 It would be good to determine an appropriate set of compiler switches
 to reduce the warnings to a reasonable level.

 yeah once we have determined that this whole experiment is useful it
 should be pretty easy to tweak the compiler switches for the non-gcc
 compilers (mostly icc and sun studio seem to be the ones that generate
 excessive output).

 
 For sun studio -erroff=E_STATEMENT_NOT_REACHED is useful there. If you
 want to determine warning tags for each warning add -errtags.

Is that supported on all versions of sun studio(Sun WorkShop 6, Sun
Studio 8,11) we have on the farm ?


Stefan

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
   subscribe-nomail command to [EMAIL PROTECTED] so that your
   message can get through to the mailing list cleanly

Re: [HACKERS] compiler warnings on the buildfarm

[Chris Browne]

[EMAIL PROTECTED] (Stefan Kaltenbrunner) writes:

 Tom Lane wrote:
 [...]
 animal: clownfish   warnings: 12
 dynloader.c, line 4: warning: empty translation unit
 postgres.c, line 3758: warning: loop not entered at top
 
 The first of these is not a bug, the second seems to be some weird
 aberration in their statement-not-reached detection.

 will see about filtering out those

 
 animal: grebe   warnings: 45
 xlog.c:651: warning: implicit declaration of function '_check_lock'
 xlog.c:654: warning: implicit declaration of function '_clear_lock'
 hba.c:1449: warning: implicit declaration of function 'getpeereid'
 
 Someone needs to find out which system headers declare these functions
 on AIX.

Hmm.  Logging onto grebe:

/usr/include/sys/socket.h:int getpeereid(int, uid_t *__restrict__, gid_t 
*__restrict__);

ydb1.int.libertyrms.com(cbbrowne): /home/cbbrowne # egrep '_(check|clear)_lock' 
/usr/include/*/*.h
/usr/include/sys/atomic_op.h:boolean_t _check_lock();
/usr/include/sys/atomic_op.h:void _clear_lock();
/usr/include/sys/atomic_op.h:void _clear_lock_mem();
/usr/include/sys/atomic_op.h:boolean_t _check_lock(atomic_p, int, int);
/usr/include/sys/atomic_op.h:void _clear_lock(atomic_p, int);
/usr/include/sys/atomic_op.h:void _clear_lock_mem(atomic_p, int);

Do those seem apropos?

 ip.c: In function 'getaddrinfo_unix':
 ip.c:254: warning: large integer implicitly truncated to unsigned type
 
 This is complaining about
 
 #ifdef HAVE_STRUCT_SOCKADDR_STORAGE_SS_LEN
  unp-sun_len = sizeof(struct sockaddr_un);
 #endif
 
 I don't know how wide sun_len is on this platform.  It's probably uint8,
 but if we explicitly cast the sizeof to 8 bits, we could conceivably
 break things on other platforms.  Are there any where sockaddr_un is
 longer than 255 bytes?  Anyway I'm inclined to leave this alone.

 no idea on AIX but I have added christopher to the CC list - maybe he
 can shed some light on those things.

/* According to RFC3493 sockaddr_storage structure should be greater than or
equal to the largest sockaddr struct. The size of sockaddr_un structure changed 
to 1025 in order to support long user names. Change _SS_MAXSIZE accordingly
inorder to main compliance to the RFC */
#define _SS_MAXSIZE 1280 /* Implementation specific max size */

Actually, you can take a look at doc/FAQ_AIX; that reports that the
size was updated to 1028 back in 2005, as a result, in fact, of my bug
submission :-).

The comment in the #include seems somewhat nonsensical; the reason for
increasing sockaddr_un was to support IPv6 addresses.  I didn't think
it had anything to do with user names...

[Aside: Sorry, I don't have any flames about EDB/CMD today.  Boy, you
miss reading -advocacy for half a day, and sometimes you miss
something big...]
-- 
output = reverse(gro.mca @ enworbbc)
http://www3.sympatico.ca/cbbrowne/linuxxian.html
One  of my most  often repeated quips was  the one I made when former
Presidents Carter, Ford and Nixon stood by each other at a White House
event. 'There they are,' I said. 'See  no evil, hear  no evil, and ...
evil.' -- Bob Dole, 1983

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster

[HACKERS] compiler warnings on the buildfarm

[Stefan Kaltenbrunner]

What is the official stance on handling compiler warnings?
I got a bit curious today on how many warnings our builds are generating
on the buildfarm.
I have hacked up a small script that (in a very primitive way) parses
the make stage logfiles of all unix boxes reporting on HEAD and prints
the number of lines that appear to be either diagnostics or warnings
emited during a build with the following results:

animal: mongoosewarnings: 2379
animal: warthog warnings: 1368
animal: kuduwarnings: 748
animal: turnip_moth warnings: 736
animal: gypsy_moth  warnings: 736
animal: winter_moth warnings: 700
animal: ghost_moth  warnings: 700
animal: dot_mothwarnings: 700
animal: codlin_moth warnings: 700
animal: dugong  warnings: 371
animal: emu warnings: 265
animal: orangutan   warnings: 198
animal: spoonbill   warnings: 126
animal: zebra   warnings: 124
animal: tucuxi  warnings: 124
animal: lionfishwarnings: 87
animal: wildebeest  warnings: 77
animal: rookwarnings: 76
animal: rosella warnings: 74
animal: dragonfly   warnings: 72
animal: wombat  warnings: 71
animal: Shadwarnings: 71
animal: quagga  warnings: 71
animal: caracarawarnings: 71
animal: bobcat  warnings: 71
animal: barasingha  warnings: 71
animal: grebe   warnings: 50
animal: eel warnings: 43
animal: thrush  warnings: 32
animal: salamander  warnings: 27
animal: clownfish   warnings: 26
animal: osprey  warnings: 24
animal: luna_moth   warnings: 15
animal: emperor_mothwarnings: 15
animal: canary  warnings: 14
animal: impala  warnings: 9
animal: jackal  warnings: 7
animal: bearwarnings: 6
animal: viper   warnings: 5
animal: cuckoo  warnings: 5
animal: cardinalwarnings: 5
animal: boodie  warnings: 5
animal: aubrac  warnings: 5
animal: sponge  warnings: 3
animal: dungbeetle  warnings: 3
animal: tapir   warnings: 0
animal: platypuswarnings: 0
animal: ermine  warnings: 0

a lot of those are simply noise (like the LOOP VECTORIZED stuff from the
icc boxes or the statement not reached spam from the sun compilers)
but others might indicate real issues.
To find warnings that might be a real problem we might want to look into
 suppressing those - if possible -  using compiler switches.

comments ?


Stefan

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org

Re: [HACKERS] compiler warnings on the buildfarm

[Tom Lane]

Stefan Kaltenbrunner [EMAIL PROTECTED] writes:
 What is the official stance on handling compiler warnings?

The compilers I use give me 1 or 2 warnings on HEAD, coming from flex's
sloppiness about not generating unused code.  I wouldn't care to work
with a compiler that generated more than a few.  At the same time,
I'm not prepared to contort the source code unduly to suppress what
you referred to as spam warnings from over-chatty compilers.

What would probably be useful if you want to pursue this is to filter
out the obvious spam like statement-not-reached, and see what's left.

regards, tom lane

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings

Re: [HACKERS] compiler warnings on the buildfarm

[Peter Eisentraut]

Am Donnerstag, 12. Juli 2007 15:25 schrieb Stefan Kaltenbrunner:
 a lot of those are simply noise (like the LOOP VECTORIZED stuff from the
 icc boxes or the statement not reached spam from the sun compilers)
 but others might indicate real issues.
 To find warnings that might be a real problem we might want to look into
  suppressing those - if possible -  using compiler switches.

It would be good to determine an appropriate set of compiler switches to 
reduce the warnings to a reasonable level.

-- 
Peter Eisentraut
http://developer.postgresql.org/~petere/

---(end of broadcast)---
TIP 6: explain analyze is your friend

Re: [HACKERS] compiler warnings on the buildfarm

[Stefan Kaltenbrunner]

Tom Lane wrote:
 Stefan Kaltenbrunner [EMAIL PROTECTED] writes:
 What is the official stance on handling compiler warnings?
 
 The compilers I use give me 1 or 2 warnings on HEAD, coming from flex's
 sloppiness about not generating unused code.  I wouldn't care to work
 with a compiler that generated more than a few.  At the same time,
 I'm not prepared to contort the source code unduly to suppress what
 you referred to as spam warnings from over-chatty compilers.
 
 What would probably be useful if you want to pursue this is to filter
 out the obvious spam like statement-not-reached, and see what's left.

ok I did that for a few members (removing all the statement not reached
ones as well as some purely informal notices and all the flex related
warnings) and came up with something similiar to:


animal: eel warnings: 4
dirmod.c:206: warning: no previous prototype for 'pgsymlink'
dirmod.c:206: warning: no previous prototype for 'pgsymlink'
pg_ctl.c: In function `pgwin32_doRunAsService':
pg_ctl.c:1240: warning: initialization discards qualifiers from pointer
target type

animal: clownfish   warnings: 12
dynloader.c, line 4: warning: empty translation unit
postgres.c, line 3758: warning: loop not entered at top
xml.c, line 2810: warning: integer overflow detected: op 
xml.c, line 2810: warning: integer overflow detected: op UNARY -
xml.c, line 2810: warning: integer overflow detected: op 
dfmgr.c, line 117: warning: assignment type mismatch:
pointer to function(pointer to struct FunctionCallInfoData
{pointer to struct FmgrInfo {..} flinfo, pointer to struct Node {..}
context, pointer to struc
t Node {..} resultinfo, char isnull, short nargs, array[100] of unsigned
long arg, array[100] of char argnull}) returning unsigned long =
pointer to void
dfmgr.c, line 165: warning: return value type mismatch
wchar.c, line 283: warning: integer overflow detected: op 
wchar.c, line 283: warning: integer overflow detected: op 
pg_resetxlog.c, line 76: warning: initializer does not fit or is out
of range: -1
pg_resetxlog.c, line 80: warning: initializer does not fit or is out
of range: -1


animal: jackal  warnings: 2
postmaster.c: In function 'PostmasterMain':
postmaster.c:796: warning: 'DNSServiceRegistrationCreate' is deprecated
(declared at /usr/include/DNSServiceDiscovery/DNSServiceDiscovery.h:114)


animal: impala  warnings: 6
auth.c: In function 'pg_GSS_recvauth':
auth.c:435: warning: format '%u' expects type 'unsigned int', but
argument 3 has type 'size_t'
auth.c:456: warning: format '%u' expects type 'unsigned int', but
argument 5 has type 'size_t'
auth.c:464: warning: format '%u' expects type 'unsigned int', but
argument 3 has type 'size_t'
auth.c: In function 'sendAuthRequest':
auth.c:787: warning: format '%u' expects type 'unsigned int', but
argument 3 has type 'size_t'

animal: grebe   warnings: 45
xlog.c: In function 'XLogInsert':
xlog.c:651: warning: implicit declaration of function '_check_lock'
xlog.c:654: warning: implicit declaration of function '_clear_lock'
hba.c: In function 'ident_unix':
hba.c:1449: warning: implicit declaration of function 'getpeereid'
ip.c: In function 'getaddrinfo_unix':
ip.c:254: warning: large integer implicitly truncated to unsigned type
bgwriter.c: In function 'BackgroundWriterMain':
bgwriter.c:305: warning: implicit declaration of function '_check_lock'
bgwriter.c:308: warning: implicit declaration of function '_clear_lock'
buf_init.c: In function 'InitBufferPool':
buf_init.c:118: warning: implicit declaration of function '_clear_lock'
bufmgr.c: In function 'ReadBuffer_common':
bufmgr.c:249: warning: implicit declaration of function '_check_lock'
bufmgr.c:252: warning: implicit declaration of function '_clear_lock'
freelist.c: In function 'StrategyGetBuffer':
freelist.c:143: warning: implicit declaration of function '_check_lock'
freelist.c:150: warning: implicit declaration of function '_clear_lock'
shmem.c: In function 'InitShmemAllocation':
shmem.c:127: warning: implicit declaration of function '_clear_lock'
shmem.c: In function 'ShmemAlloc':
shmem.c:168: warning: implicit declaration of function '_check_lock'
proc.c: In function 'InitProcGlobal':
proc.c:216: warning: implicit declaration of function '_clear_lock'
proc.c: In function 'InitProcess':
proc.c:247: warning: implicit declaration of function '_check_lock'
lwlock.c: In function 'CreateLWLocks':
lwlock.c:256: warning: implicit declaration of function '_clear_lock'
lwlock.c: In function 'LWLockAssign':
lwlock.c:291: warning: implicit declaration of function '_check_lock'
s_lock.c: In function 's_lock':
s_lock.c:99: warning: implicit declaration of function '_check_lock'
dynahash.c: In function 'init_htab':
dynahash.c:526: warning: implicit declaration of function '_clear_lock'
dynahash.c: In function 'hash_search_with_hash_value':
dynahash.c:876: warning: implicit declaration of function '_check_lock'
guc.c:2866: warning: 'guc_get_index' defined 

Re: [HACKERS] compiler warnings on the buildfarm

[Stefan Kaltenbrunner]

Peter Eisentraut wrote:
 Am Donnerstag, 12. Juli 2007 15:25 schrieb Stefan Kaltenbrunner:
 a lot of those are simply noise (like the LOOP VECTORIZED stuff from the
 icc boxes or the statement not reached spam from the sun compilers)
 but others might indicate real issues.
 To find warnings that might be a real problem we might want to look into
  suppressing those - if possible -  using compiler switches.
 
 It would be good to determine an appropriate set of compiler switches to 
 reduce the warnings to a reasonable level.

yeah once we have determined that this whole experiment is useful it
should be pretty easy to tweak the compiler switches for the non-gcc
compilers (mostly icc and sun studio seem to be the ones that generate
excessive output).


Stefan

---(end of broadcast)---
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate

Re: [HACKERS] compiler warnings on the buildfarm

[Tom Lane]

Stefan Kaltenbrunner [EMAIL PROTECTED] writes:
 ok I did that for a few members (removing all the statement not reached
 ones as well as some purely informal notices and all the flex related
 warnings) and came up with something similiar to:
 [snip]

Yeah, this looks like a good list.  I can't readily check the ones from
eel as they appear to be in Windows-specific code; anyone else want to
fix those?

 animal: jackal  warnings: 2
 postmaster.c: In function 'PostmasterMain':
 postmaster.c:796: warning: 'DNSServiceRegistrationCreate' is deprecated
 (declared at /usr/include/DNSServiceDiscovery/DNSServiceDiscovery.h:114)

This one we knew about; there's been previous discussion of rewriting
the Bonjour support to use a more portable API, but I don't think anyone
feels like doing it right now.

I'll take a look at the rest.

regards, tom lane

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match

Re: [HACKERS] compiler warnings on the buildfarm

[Magnus Hagander]

Tom Lane wrote:
 Stefan Kaltenbrunner [EMAIL PROTECTED] writes:
 ok I did that for a few members (removing all the statement not reached
 ones as well as some purely informal notices and all the flex related
 warnings) and came up with something similiar to:
 [snip]
 
 Yeah, this looks like a good list.  I can't readily check the ones from
 eel as they appear to be in Windows-specific code; anyone else want to
 fix those?

The pg_ctl one is a windows one, I'll deal with that one.

The dirmod one doesn't appear on win32, only cygwin. I don't have a
cygwin to check that against, so I'll have to pass on that one.

//Magnus


---(end of broadcast)---
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate

Re: [HACKERS] compiler warnings on the buildfarm

[Tom Lane]

Magnus Hagander [EMAIL PROTECTED] writes:
 Tom Lane wrote:
 Yeah, this looks like a good list.  I can't readily check the ones from
 eel as they appear to be in Windows-specific code; anyone else want to
 fix those?

 The pg_ctl one is a windows one, I'll deal with that one.

 The dirmod one doesn't appear on win32, only cygwin. I don't have a
 cygwin to check that against, so I'll have to pass on that one.

Eyeing the code, it looks like the issue is that port.h declares
pgsymlink if
#if defined(WIN32)  !defined(__CYGWIN__)
while dirmod.c defines it if
#ifdef WIN32

So this seems like an actual bug, at least to the extent that pgsymlink
is being compiled into code but not used on Cygwin.  But more to the
point, maybe port.h is wrong and we should be using pgsymlink on Cygwin?
In any case these two files need to be put into sync.

regards, tom lane

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org

Re: [HACKERS] compiler warnings on the buildfarm

[Magnus Hagander]

Tom Lane wrote:
 Magnus Hagander [EMAIL PROTECTED] writes:
 Tom Lane wrote:
 Yeah, this looks like a good list.  I can't readily check the ones from
 eel as they appear to be in Windows-specific code; anyone else want to
 fix those?
 
 The pg_ctl one is a windows one, I'll deal with that one.
 
 The dirmod one doesn't appear on win32, only cygwin. I don't have a
 cygwin to check that against, so I'll have to pass on that one.
 
 Eyeing the code, it looks like the issue is that port.h declares
 pgsymlink if
   #if defined(WIN32)  !defined(__CYGWIN__)
 while dirmod.c defines it if
   #ifdef WIN32
 
 So this seems like an actual bug, at least to the extent that pgsymlink
 is being compiled into code but not used on Cygwin.  But more to the
 point, maybe port.h is wrong and we should be using pgsymlink on Cygwin?
 In any case these two files need to be put into sync.

Agreed, but I don't know which should be the master :(

Well, actually. Since it's not #defined in port.h, that should mean it's
not being used anyway? Since the symbol in dirmod.c is pgsymlink, which
shouldn't be referenced directly anywhere. If that's so, then changing
dirmod.c to just exclude the whole thing on CYGWIN should fix the issue.

I don't have a way to test it, but perhaps we should just throw it in
and see if eel breaks on the buildfarm? Unless some poor soul actually
has a cygwin dev box to test on?

//Magnus


---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings

Re: [HACKERS] compiler warnings on the buildfarm

[Stefan Kaltenbrunner]

Tom Lane wrote:
 Stefan Kaltenbrunner [EMAIL PROTECTED] writes:
 ok I did that for a few members (removing all the statement not reached
 ones as well as some purely informal notices and all the flex related
 warnings) and came up with something similiar to:
 [snip]
 
 Yeah, this looks like a good list.  I can't readily check the ones from
 eel as they appear to be in Windows-specific code; anyone else want to
 fix those?
 
 animal: jackal  warnings: 2
 postmaster.c: In function 'PostmasterMain':
 postmaster.c:796: warning: 'DNSServiceRegistrationCreate' is deprecated
 (declared at /usr/include/DNSServiceDiscovery/DNSServiceDiscovery.h:114)
 
 This one we knew about; there's been previous discussion of rewriting
 the Bonjour support to use a more portable API, but I don't think anyone
 feels like doing it right now.
 
 I'll take a look at the rest.

some more(I have removed duplicates and ones that should be fixed by
your latest commits though):

animal: salamander  warnings: 27
cash.c: In function `cash_in':
cash.c:244: warning: subscript has type `char'
pg_lzcompress.c: In function `pglz_compress':
pg_lzcompress.c:378: warning: inlining failed in call to `pglz_find_match'
pg_lzcompress.c:578: warning: called from here


animal: canary  warnings: 14
plpython.c: In function `PLyMapping_ToTuple':
plpython.c:1717: warning: variable `i' might be clobbered by `longjmp'
or `vfork'
plpython.c:1732: warning: variable `value' might be clobbered by
`longjmp' or `vfork'
plpython.c:1733: warning: variable `so' might be clobbered by `longjmp'
or `vfork'
plpython.c: In function `PLySequence_ToTuple':
plpython.c:1797: warning: variable `i' might be clobbered by `longjmp'
or `vfork'
plpython.c:1821: warning: variable `value' might be clobbered by
`longjmp' or `vfork'
plpython.c:1822: warning: variable `so' might be clobbered by `longjmp'
or `vfork'
plpython.c: In function `PLyObject_ToTuple':
plpython.c:1879: warning: variable `i' might be clobbered by `longjmp'
or `vfork'
plpython.c:1892: warning: variable `value' might be clobbered by
`longjmp' or `vfork'
plpython.c:1893: warning: variable `so' might be clobbered by `longjmp'
or `vfork'
plpython.c: In function `PLy_spi_execute_plan':
plpython.c:2434: warning: variable `i' might be clobbered by `longjmp'
or `vfork'

animal: dragonfly   warnings: 67
auth.c:61: warning: initialization from incompatible pointer type
cash.c: In function `cash_in':
cash.c:244: warning: subscript has type `char'
connect.c:23: warning: missing braces around initializer
connect.c:23: warning: (near initialization for
`actual_connection_key_once.__pthread_once_pad')
misc.c:67: warning: missing braces around initializer
misc.c:67: warning: (near initialization for
`sqlca_key_once.__pthread_once_pad')


animal: emperor_mothwarnings: 10
auth.c:61: warning: initialization from incompatible pointer type


animal: osprey  warnings: 22
s_lock.c:222: warning: `tas_dummy' defined but not used
pg_lzcompress.c: In function `pglz_compress':
pg_lzcompress.c:378: warning: inlining failed in call to `pglz_find_match'
pg_lzcompress.c:578: warning: called from here
fmgr.c: In function `fmgr_oldstyle':
fmgr.c:629: warning: assignment makes pointer from integer without a cast
fmgr.c:638: warning: assignment makes pointer from integer without a cast
fmgr.c:641: warning: assignment makes pointer from integer without a cast
fmgr.c:645: warning: assignment makes pointer from integer without a cast
fmgr.c:649: warning: assignment makes pointer from integer without a cast
fmgr.c:654: warning: assignment makes pointer from integer without a cast
fmgr.c:659: warning: assignment makes pointer from integer without a cast
fmgr.c:665: warning: assignment makes pointer from integer without a cast
fmgr.c:671: warning: assignment makes pointer from integer without a cast
fmgr.c:678: warning: assignment makes pointer from integer without a cast
fmgr.c:685: warning: assignment makes pointer from integer without a cast
fmgr.c:693: warning: assignment makes pointer from integer without a cast
fmgr.c:701: warning: assignment makes pointer from integer without a cast
fmgr.c:710: warning: assignment makes pointer from integer without a cast
fmgr.c:719: warning: assignment makes pointer from integer without a cast
fmgr.c:729: warning: assignment makes pointer from integer without a cast
fmgr.c:739: warning: assignment makes pointer from integer without a cast

animal: lionfishwarnings: 16
/tmp/cclwN8N9.s: Assembler messages:
/tmp/cclwN8N9.s:109082: Warning: Macro instruction expanded into
multiple instructions
/tmp/cclwN8N9.s:109246: Warning: Macro instruction expanded into
multiple instructions
pg_lzcompress.c: In function `pglz_compress':
pg_lzcompress.c:378: warning: inlining failed in call to `pglz_find_match'
pg_lzcompress.c:578: warning: called from here
/tmp/ccnsL6Et.s: Assembler messages:
/tmp/ccnsL6Et.s:160502: Warning: Macro instruction expanded into

Re: [HACKERS] compiler warnings on the buildfarm

[Alvaro Herrera]

Stefan showed me via Jabber this warning:

/tmp/ccM7MfqX.s: Assembler messages:
/tmp/ccM7MfqX.s:703: Warning: 0003fffc shortened to fffc
/tmp/ccM7MfqX.s:738: Warning: 0003fffc shortened to fffc

He says that this comes from trgm_op.c file.  I don't get the warning
myself obviously, so I started guessing.

3FFFC = 00
 FFFC =   1100

So the upper 2 bits are being lost (the second number is 32 bits wide).

The only thing that I think is related is the usage of VARSIZE().  It
looks like 0x3FFF (actual constant in the toast code) is shift two
bits left.  I can see no such operation though.

-- 
Alvaro Herrera http://www.flickr.com/photos/alvherre/
Dios hizo a Adán, pero fue Eva quien lo hizo hombre.

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org

Re: [HACKERS] compiler warnings on the buildfarm

[Stefan Kaltenbrunner]

Alvaro Herrera wrote:
 Stefan showed me via Jabber this warning:
 
 /tmp/ccM7MfqX.s: Assembler messages:
 /tmp/ccM7MfqX.s:703: Warning: 0003fffc shortened to fffc
 /tmp/ccM7MfqX.s:738: Warning: 0003fffc shortened to fffc
 
 He says that this comes from trgm_op.c file.  I don't get the warning
 myself obviously, so I started guessing.
 
 3FFFC = 00
  FFFC =   1100
 
 So the upper 2 bits are being lost (the second number is 32 bits wide).
 
 The only thing that I think is related is the usage of VARSIZE().  It
 looks like 0x3FFF (actual constant in the toast code) is shift two
 bits left.  I can see no such operation though.

this one is from
http://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=caracaradt=2007-07-12%2021stg=make-contrib

as I'm going through the warnings in contrib now.


Stefan

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster

Re: [HACKERS] compiler warnings on the buildfarm

[Heikki Linnakangas]

Alvaro Herrera wrote:

Stefan showed me via Jabber this warning:

/tmp/ccM7MfqX.s: Assembler messages:
/tmp/ccM7MfqX.s:703: Warning: 0003fffc shortened to fffc
/tmp/ccM7MfqX.s:738: Warning: 0003fffc shortened to fffc

He says that this comes from trgm_op.c file.  I don't get the warning
myself obviously, so I started guessing.

3FFFC = 00
 FFFC =   1100

So the upper 2 bits are being lost (the second number is 32 bits wide).

The only thing that I think is related is the usage of VARSIZE().  It
looks like 0x3FFF (actual constant in the toast code) is shift two
bits left.  I can see no such operation though.


The shift is in postgres.h, SET_VARSIZE_4B. I have no idea where that 
warning is coming from, though. What's the real source behind ccM7MfqX.s?


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings

Re: [HACKERS] compiler warnings on the buildfarm

[Alvaro Herrera]

Heikki Linnakangas wrote:
 Alvaro Herrera wrote:
 Stefan showed me via Jabber this warning:
 /tmp/ccM7MfqX.s: Assembler messages:
 /tmp/ccM7MfqX.s:703: Warning: 0003fffc shortened to 
 fffc
 /tmp/ccM7MfqX.s:738: Warning: 0003fffc shortened to 
 fffc
 He says that this comes from trgm_op.c file.  I don't get the warning
 myself obviously, so I started guessing.
 3FFFC = 00
  FFFC =   1100
 So the upper 2 bits are being lost (the second number is 32 bits wide).
 The only thing that I think is related is the usage of VARSIZE().  It
 looks like 0x3FFF (actual constant in the toast code) is shift two
 bits left.  I can see no such operation though.

 The shift is in postgres.h, SET_VARSIZE_4B. I have no idea where that 
 warning is coming from, though. What's the real source behind ccM7MfqX.s?

trgm_op.c

I'm not sure that the shift in SET_VARSIZE_4B is applicable here,
because it would have to be passed a len of .

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

---(end of broadcast)---
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate

Re: [HACKERS] compiler warnings on the buildfarm

[Gregory Stark]

Heikki Linnakangas [EMAIL PROTECTED] writes:

 Alvaro Herrera wrote:
 Stefan showed me via Jabber this warning:

 /tmp/ccM7MfqX.s: Assembler messages:
 /tmp/ccM7MfqX.s:703: Warning: 0003fffc shortened to fffc
 /tmp/ccM7MfqX.s:738: Warning: 0003fffc shortened to fffc

 He says that this comes from trgm_op.c file.  I don't get the warning
 myself obviously, so I started guessing.

I've occasionally seen this warning too. It seems to be pretty random though.
Nor have I been able to find any actual problems caused by it.

But istm this has to be a compiler bug since any overflow in the C code should
be handled (and potentially warned) by the compiler long before the assembler
gets to see it.


 So the upper 2 bits are being lost (the second number is 32 bits wide).

 The only thing that I think is related is the usage of VARSIZE().  It
 looks like 0x3FFF (actual constant in the toast code) is shift two
 bits left.  I can see no such operation though.

 The shift is in postgres.h, SET_VARSIZE_4B. I have no idea where that warning 
 is
 coming from, though. What's the real source behind ccM7MfqX.s?

Huh. I wonder.

Could the compiler be incorrectly optimizing cases of 

  SET_VARSIZE(new_datum VARSIZE(olddatum)) 

which amounts to:

  ((olddatum  2)  0x3FFF)  2

If it does constant propagation without handling overflow it could end up
with:

 (olddatum  2  2)  0x3FFFC

note that in fact truncating the high two bits as the assembler did would in
fact be the correct thing to do here which would explain why it doesn't cause
any actual problems.

Also note that it doesn't require an actual single statement of the form
above. The compiler could be doing constant propagation from an earlier
statement. Code which calls VARSIZE(olddatum) and then passes the result to
SET_VARSIZE(newdatum,len) is quite common.

-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com


---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match

Re: [HACKERS] compiler warnings on the buildfarm

[Heikki Linnakangas]

Alvaro Herrera wrote:

Heikki Linnakangas wrote:

Alvaro Herrera wrote:

Stefan showed me via Jabber this warning:
/tmp/ccM7MfqX.s: Assembler messages:
/tmp/ccM7MfqX.s:703: Warning: 0003fffc shortened to 
fffc
/tmp/ccM7MfqX.s:738: Warning: 0003fffc shortened to 
fffc

He says that this comes from trgm_op.c file.  I don't get the warning
myself obviously, so I started guessing.
3FFFC = 00
 FFFC =   1100
So the upper 2 bits are being lost (the second number is 32 bits wide).
The only thing that I think is related is the usage of VARSIZE().  It
looks like 0x3FFF (actual constant in the toast code) is shift two
bits left.  I can see no such operation though.
The shift is in postgres.h, SET_VARSIZE_4B. I have no idea where that 
warning is coming from, though. What's the real source behind ccM7MfqX.s?


trgm_op.c

I'm not sure that the shift in SET_VARSIZE_4B is applicable here,
because it would have to be passed a len of .


Hmm. It looks like I get that warning on my laptop as well. I tracked it 
down to these two places:


Line 209:

while (ptr - GETARR(trg)  ARRNELEM(trg))
{
text   *item = (text *) palloc(VARHDRSZ + 3);

SET_VARSIZE(item, VARHDRSZ + 3);
CPTRGM(VARDATA(item), ptr);

d[ptr - GETARR(trg)] = PointerGetDatum(item);

ptr++;
}


Line 224:

ptr = GETARR(trg);
while (ptr - GETARR(trg)  ARRNELEM(trg))
{

pfree(DatumGetPointer(d[ptr - GETARR(trg)]));

ptr++;
}


The warning seems to be in related array indexing. If you replace ptr - 
GETARR(trg) with a constant, the warning goes away. But having i = ptr 
- GETARR(trg) in there doesn't give a warning.


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings

Re: [HACKERS] compiler warnings on the buildfarm

[Tom Lane]

Heikki Linnakangas [EMAIL PROTECTED] writes:
 Hmm. It looks like I get that warning on my laptop as well. I tracked it 
 down to these two places:

 Line 209:
 while (ptr - GETARR(trg)  ARRNELEM(trg))
 {
  text   *item = (text *) palloc(VARHDRSZ + 3);
  
  SET_VARSIZE(item, VARHDRSZ + 3);
  CPTRGM(VARDATA(item), ptr);
d[ptr - GETARR(trg)] = PointerGetDatum(item);
  ptr++;
 }

I'll betcha the compiler is trying to optimize the repeated calculations
of ptr - GETARR(trg) into a separate variable that it increments along
with ptr.  Maybe it is getting it wrong, or maybe the assembler is just
confused.  Does the warning go away if you dial down the -O level?

regards, tom lane

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
   subscribe-nomail command to [EMAIL PROTECTED] so that your
   message can get through to the mailing list cleanly

Re: [HACKERS] compiler warnings on the buildfarm

[Gregory Stark]

Heikki Linnakangas [EMAIL PROTECTED] writes:

 The warning seems to be in related array indexing. If you replace ptr -
 GETARR(trg) with a constant, the warning goes away. But having i = ptr -
 GETARR(trg) in there doesn't give a warning.

Can you compile with -save-temps and send the corresponding assembly file?

-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com


---(end of broadcast)---
TIP 6: explain analyze is your friend

Re: [HACKERS] compiler warnings on the buildfarm

[Tom Lane]

Gregory Stark [EMAIL PROTECTED] writes:
 If it does constant propagation without handling overflow it could end up
 with:

  (olddatum  2  2)  0x3FFFC

 note that in fact truncating the high two bits as the assembler did would in
 fact be the correct thing to do here which would explain why it doesn't cause
 any actual problems.

Good point, but I also note that the places Heikki saw were inside
loops.  I think it might be some combination of the above and a loop
strength reduction optimization.

regards, tom lane

---(end of broadcast)---
TIP 6: explain analyze is your friend

Re: [HACKERS] compiler warnings on the buildfarm

[Heikki Linnakangas]

Tom Lane wrote:

Heikki Linnakangas [EMAIL PROTECTED] writes:
Hmm. It looks like I get that warning on my laptop as well. I tracked it 
down to these two places:



Line 209:

while (ptr - GETARR(trg)  ARRNELEM(trg))
{
text   *item = (text *) palloc(VARHDRSZ + 3);

SET_VARSIZE(item, VARHDRSZ + 3);
CPTRGM(VARDATA(item), ptr);

d[ptr - GETARR(trg)] = PointerGetDatum(item);

ptr++;
}


I'll betcha the compiler is trying to optimize the repeated calculations
of ptr - GETARR(trg) into a separate variable that it increments along
with ptr.  Maybe it is getting it wrong, or maybe the assembler is just
confused.  Does the warning go away if you dial down the -O level?


FWIW, this patch makes the warnings go away, and makes the code a little 
bit more readable as well. It would be nice to understand why exactly 
it's complaining, though.


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com
Index: trgm_op.c
===
RCS file: /home/hlinnaka/pgcvsrepository/pgsql/contrib/pg_trgm/trgm_op.c,v
retrieving revision 1.8
diff -c -r1.8 trgm_op.c
*** trgm_op.c	28 Feb 2007 22:44:38 -	1.8
--- trgm_op.c	12 Jul 2007 22:28:43 -
***
*** 194,212 
  	Datum	   *d;
  	ArrayType  *a;
  	trgm	   *ptr;
  
  	trg = generate_trgm(VARDATA(in), VARSIZE(in) - VARHDRSZ);
  	d = (Datum *) palloc(sizeof(Datum) * (1 + ARRNELEM(trg)));
  
! 	ptr = GETARR(trg);
! 	while (ptr - GETARR(trg)  ARRNELEM(trg))
  	{
  		text	   *item = (text *) palloc(VARHDRSZ + 3);
  
  		SET_VARSIZE(item, VARHDRSZ + 3);
  		CPTRGM(VARDATA(item), ptr);
! 		d[ptr - GETARR(trg)] = PointerGetDatum(item);
! 		ptr++;
  	}
  
  	a = construct_array(
--- 194,211 
  	Datum	   *d;
  	ArrayType  *a;
  	trgm	   *ptr;
+ 	int			i;
  
  	trg = generate_trgm(VARDATA(in), VARSIZE(in) - VARHDRSZ);
  	d = (Datum *) palloc(sizeof(Datum) * (1 + ARRNELEM(trg)));
  
! 	for (i = 0, ptr = GETARR(trg); i  ARRNELEM(trg); i++, ptr++)
  	{
  		text	   *item = (text *) palloc(VARHDRSZ + 3);
  
  		SET_VARSIZE(item, VARHDRSZ + 3);
  		CPTRGM(VARDATA(item), ptr);
! 		d[i] = PointerGetDatum(item);
  	}
  
  	a = construct_array(
***
*** 218,229 
  		'i'
  		);
  
! 	ptr = GETARR(trg);
! 	while (ptr - GETARR(trg)  ARRNELEM(trg))
! 	{
! 		pfree(DatumGetPointer(d[ptr - GETARR(trg)]));
! 		ptr++;
! 	}
  
  	pfree(d);
  	pfree(trg);
--- 217,224 
  		'i'
  		);
  
! 	for (i = 0; i  ARRNELEM(trg); i++)
! 		pfree(DatumGetPointer(d[i]));
  
  	pfree(d);
  	pfree(trg);

---(end of broadcast)---
TIP 6: explain analyze is your friend

Re: [HACKERS] compiler warnings on the buildfarm

[Heikki Linnakangas]

Tom Lane wrote:

Heikki Linnakangas [EMAIL PROTECTED] writes:
Hmm. It looks like I get that warning on my laptop as well. I tracked it 
down to these two places:



Line 209:

while (ptr - GETARR(trg)  ARRNELEM(trg))
{
text   *item = (text *) palloc(VARHDRSZ + 3);

SET_VARSIZE(item, VARHDRSZ + 3);
CPTRGM(VARDATA(item), ptr);

d[ptr - GETARR(trg)] = PointerGetDatum(item);

ptr++;
}


I'll betcha the compiler is trying to optimize the repeated calculations
of ptr - GETARR(trg) into a separate variable that it increments along
with ptr.  Maybe it is getting it wrong, or maybe the assembler is just
confused.  Does the warning go away if you dial down the -O level?


Yes, I don't get it with -O1 or -O0.

$ gcc --version
gcc (GCC) 4.1.2 20061028 (prerelease) (Debian 4.1.1-19)

Hmm. Prerelease? This version came from debian/testing.

--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

---(end of broadcast)---
TIP 6: explain analyze is your friend

Re: [HACKERS] compiler warnings on the buildfarm

[Tom Lane]

Stefan Kaltenbrunner [EMAIL PROTECTED] writes:
 ok I did that for a few members (removing all the statement not reached
 ones as well as some purely informal notices and all the flex related
 warnings) and came up with something similiar to:

I've cleaned up most of this first batch.  Open issues are:

 animal: eel warnings: 4
 dirmod.c:206: warning: no previous prototype for 'pgsymlink'

Somebody needs to figure out whether we are supposed to be using
pgsymlink on Cygwin.

 animal: clownfish   warnings: 12
 dynloader.c, line 4: warning: empty translation unit
 postgres.c, line 3758: warning: loop not entered at top

The first of these is not a bug, the second seems to be some weird
aberration in their statement-not-reached detection.

 animal: grebe   warnings: 45
 xlog.c:651: warning: implicit declaration of function '_check_lock'
 xlog.c:654: warning: implicit declaration of function '_clear_lock'
 hba.c:1449: warning: implicit declaration of function 'getpeereid'

Someone needs to find out which system headers declare these functions
on AIX.

 ip.c: In function 'getaddrinfo_unix':
 ip.c:254: warning: large integer implicitly truncated to unsigned type

This is complaining about

#ifdef HAVE_STRUCT_SOCKADDR_STORAGE_SS_LEN
unp-sun_len = sizeof(struct sockaddr_un);
#endif

I don't know how wide sun_len is on this platform.  It's probably uint8,
but if we explicitly cast the sizeof to 8 bits, we could conceivably
break things on other platforms.  Are there any where sockaddr_un is
longer than 255 bytes?  Anyway I'm inclined to leave this alone.

 guc.c:2866: warning: 'guc_get_index' defined but not used
 Extra instructions are being generated for each reference to a TOC
 symbol if the symbol is in the TOC overflow area.

This is fairly bizarre, since 'guc_get_index' *is* used in guc-file.c,
which is included into this same file.  However I don't much like the
coding method used here (it is certainly not better than using a
temporary flag bit), so when I get a chance I'll rewrite it out of
existence.

 connect.c:23: warning: missing braces around initializer
 connect.c:23: warning: (near initialization for
 'actual_connection_key_once.__on_word')
 misc.c:67: warning: missing braces around initializer
 misc.c:67: warning: (near initialization for 'sqlca_key_once.__on_word')

I think these are a platform bug.  The spec clearly says that

static pthread_once_t actual_connection_key_once = PTHREAD_ONCE_INIT;

is exactly how you are supposed to do it.  If pthread_once_t is a struct
on a given platform, that platform ought to be defining
PTHREAD_ONCE_INIT with the appropriate braces included.  If we added
braces ourselves we'd break it for platforms where the macro is correct
already.  Hence, not our problem.

regards, tom lane

---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq

Re: [HACKERS] compiler warnings on the buildfarm

[Heikki Linnakangas]

Gregory Stark wrote:

Heikki Linnakangas [EMAIL PROTECTED] writes:


The warning seems to be in related array indexing. If you replace ptr -
GETARR(trg) with a constant, the warning goes away. But having i = ptr -
GETARR(trg) in there doesn't give a warning.


Can you compile with -save-temps and send the corresponding assembly file?


Sure.

--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com


trgm_op.s.gz
Description: GNU Zip compressed data

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
   subscribe-nomail command to [EMAIL PROTECTED] so that your
   message can get through to the mailing list cleanly

Re: [HACKERS] compiler warnings on the buildfarm

[Tom Lane]

Andrew Dunstan [EMAIL PROTECTED] writes:
 Tom Lane wrote:
 Somebody needs to figure out whether we are supposed to be using
 pgsymlink on Cygwin.

 According to port.h:
  *  Cygwin has its own symlinks which work on Win95/98/ME where
  *  junction points don't, so use it instead.  We have no way of
  *  knowing what type of system Cygwin binaries will be run on.

 So it looks like we should not be using pgsymlink() on Cygwin, unless we 
 declare that these platforms under Cygwin are no longer supported.

Fair enough; Cygwin is kind of a legacy port at this point anyway, so
we shouldn't be doing anything to reduce its backwards compatibility.

I'll fix dirmod.c to match port.h.

regards, tom lane

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings

Re: [HACKERS] compiler warnings on the buildfarm

[Andrew Dunstan]

Tom Lane wrote:

animal: eel warnings: 4
dirmod.c:206: warning: no previous prototype for 'pgsymlink'



Somebody needs to figure out whether we are supposed to be using
pgsymlink on Cygwin.
  



According to port.h:

*  Cygwin has its own symlinks which work on Win95/98/ME where
*  junction points don't, so use it instead.  We have no way of
*  knowing what type of system Cygwin binaries will be run on.


So it looks like we should not be using pgsymlink() on Cygwin, unless we 
declare that these platforms under Cygwin are no longer supported.


cheers

andrew


---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings

Re: [HACKERS] compiler warnings on the buildfarm

[Tom Lane]

Heikki Linnakangas [EMAIL PROTECTED] writes:
 FWIW, this patch makes the warnings go away, and makes the code a little 
 bit more readable as well. It would be nice to understand why exactly 
 it's complaining, though.

Let's apply the patch.  We are clearly tickling a bug or near-bug in
gcc, and it may have worse consequences than this on other platforms
or other gcc versions.

At the same time, if anyone wants to trim the existing code down to a
small test case, I'm sure the gcc boys would appreciate a bug report.

regards, tom lane

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster