pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedCompiler warnings
17+ messages / 6 participants
[nested] [flat]
* Compiler warnings
@ 2007-04-19 16:08 Bruce Momjian <bruce@momjian.us>
2007-04-19 16:18 ` Re: Compiler warnings Alvaro Herrera <alvherre@commandprompt.com>
0 siblings, 1 reply; 17+ messages in thread
From: Bruce Momjian @ 2007-04-19 16:08 UTC (permalink / raw)
To: pgsql-hackers
I am seeing the following compiler warnings in CVS HEAD:
analyze.c:471: warning: `diff' might be used uninitialized in this function
vacuumlazy.c:220: warning: `diff' might be used uninitialized in this function
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Compiler warnings
2007-04-19 16:08 Compiler warnings Bruce Momjian <bruce@momjian.us>
@ 2007-04-19 16:18 ` Alvaro Herrera <alvherre@commandprompt.com>
2007-04-19 16:32 ` Re: Compiler warnings Bruce Momjian <bruce@momjian.us>
0 siblings, 1 reply; 17+ messages in thread
From: Alvaro Herrera @ 2007-04-19 16:18 UTC (permalink / raw)
To: Bruce Momjian <bruce@momjian.us>; +Cc: pgsql-hackers
Bruce Momjian wrote:
> I am seeing the following compiler warnings in CVS HEAD:
>
> analyze.c:471: warning: `diff' might be used uninitialized in this function
> vacuumlazy.c:220: warning: `diff' might be used uninitialized in this function
Are you using too old a compiler? Mine does not complain
gcc (GCC) 4.1.2 20061115 (prerelease) (Debian 4.1.1-21)
I'll fix them anyway.
--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Compiler warnings
2007-04-19 16:08 Compiler warnings Bruce Momjian <bruce@momjian.us>
2007-04-19 16:18 ` Re: Compiler warnings Alvaro Herrera <alvherre@commandprompt.com>
@ 2007-04-19 16:32 ` Bruce Momjian <bruce@momjian.us>
2007-04-19 16:40 ` Re: Compiler warnings Alvaro Herrera <alvherre@commandprompt.com>
0 siblings, 1 reply; 17+ messages in thread
From: Bruce Momjian @ 2007-04-19 16:32 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@commandprompt.com>; +Cc: pgsql-hackers
Alvaro Herrera wrote:
> Bruce Momjian wrote:
> > I am seeing the following compiler warnings in CVS HEAD:
> >
> > analyze.c:471: warning: `diff' might be used uninitialized in this function
> > vacuumlazy.c:220: warning: `diff' might be used uninitialized in this function
>
> Are you using too old a compiler? Mine does not complain
> gcc (GCC) 4.1.2 20061115 (prerelease) (Debian 4.1.1-21)
Probably:
gcc version 2.95.3 20010315 (release)
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Compiler warnings
2007-04-19 16:08 Compiler warnings Bruce Momjian <bruce@momjian.us>
2007-04-19 16:18 ` Re: Compiler warnings Alvaro Herrera <alvherre@commandprompt.com>
2007-04-19 16:32 ` Re: Compiler warnings Bruce Momjian <bruce@momjian.us>
@ 2007-04-19 16:40 ` Alvaro Herrera <alvherre@commandprompt.com>
0 siblings, 0 replies; 17+ messages in thread
From: Alvaro Herrera @ 2007-04-19 16:40 UTC (permalink / raw)
To: Bruce Momjian <bruce@momjian.us>; +Cc: pgsql-hackers
Bruce Momjian wrote:
> Alvaro Herrera wrote:
> > Bruce Momjian wrote:
> > > I am seeing the following compiler warnings in CVS HEAD:
> > >
> > > analyze.c:471: warning: `diff' might be used uninitialized in this function
> > > vacuumlazy.c:220: warning: `diff' might be used uninitialized in this function
> >
> > Are you using too old a compiler? Mine does not complain
> > gcc (GCC) 4.1.2 20061115 (prerelease) (Debian 4.1.1-21)
>
> Probably:
>
> gcc version 2.95.3 20010315 (release)
The only thing older than that, I think, is Manfred Koizar's bike.
http://archives.postgresql.org/pgsql-patches/2004-05/msg00326.php
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
^ permalink raw reply [nested|flat] 17+ messages in thread
* Compiler warnings
@ 2016-11-29 15:21 Stephen Frost <sfrost@snowman.net>
2016-11-29 15:25 ` Re: Compiler warnings Stephen Frost <sfrost@snowman.net>
2016-12-06 20:23 ` Re: Compiler warnings Stephen Frost <sfrost@snowman.net>
0 siblings, 2 replies; 17+ messages in thread
From: Stephen Frost @ 2016-11-29 15:21 UTC (permalink / raw)
To: pgsql-hackers
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
Attachments:
[text/x-diff] silence-warnings.patch (1.1K, ../../20161129152102.GR13284@tamriel.snowman.net/2-silence-warnings.patch)
download | inline diff:
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);
[application/pgp-signature] signature.asc (818B, ../../20161129152102.GR13284@tamriel.snowman.net/3-signature.asc)
download
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Compiler warnings
2016-11-29 15:21 Compiler warnings Stephen Frost <sfrost@snowman.net>
@ 2016-11-29 15:25 ` Stephen Frost <sfrost@snowman.net>
1 sibling, 0 replies; 17+ messages in thread
From: Stephen Frost @ 2016-11-29 15:25 UTC (permalink / raw)
To: pgsql-hackers
* Stephen Frost (sfrost@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
Attachments:
[application/pgp-signature] signature.asc (818B, ../../20161129152538.GS13284@tamriel.snowman.net/2-signature.asc)
download
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Compiler warnings
2016-11-29 15:21 Compiler warnings Stephen Frost <sfrost@snowman.net>
@ 2016-12-06 20:23 ` Stephen Frost <sfrost@snowman.net>
2016-12-06 20:31 ` Re: Compiler warnings Robert Haas <robertmhaas@gmail.com>
1 sibling, 1 reply; 17+ messages in thread
From: Stephen Frost @ 2016-12-06 20:23 UTC (permalink / raw)
To: pgsql-hackers
All,
* Stephen Frost (sfrost@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
Attachments:
[text/x-diff] silence-warnings_v2.patch (1.5K, ../../20161206202305.GQ23417@tamriel.snowman.net/2-silence-warnings_v2.patch)
download | inline diff:
From afe53a0fa8a53c080b77a7334531ff3a5dc976d4 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
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
[application/pgp-signature] signature.asc (818B, ../../20161206202305.GQ23417@tamriel.snowman.net/3-signature.asc)
download
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Compiler warnings
2016-11-29 15:21 Compiler warnings Stephen Frost <sfrost@snowman.net>
2016-12-06 20:23 ` Re: Compiler warnings Stephen Frost <sfrost@snowman.net>
@ 2016-12-06 20:31 ` Robert Haas <robertmhaas@gmail.com>
2016-12-06 20:46 ` Re: Compiler warnings Stephen Frost <sfrost@snowman.net>
0 siblings, 1 reply; 17+ messages in thread
From: Robert Haas @ 2016-12-06 20:31 UTC (permalink / raw)
To: Stephen Frost <sfrost@snowman.net>; +Cc: pgsql-hackers
On Tue, Dec 6, 2016 at 3:23 PM, Stephen Frost <sfrost@snowman.net> wrote:
> * Stephen Frost (sfrost@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
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Compiler warnings
2016-11-29 15:21 Compiler warnings Stephen Frost <sfrost@snowman.net>
2016-12-06 20:23 ` Re: Compiler warnings Stephen Frost <sfrost@snowman.net>
2016-12-06 20:31 ` Re: Compiler warnings Robert Haas <robertmhaas@gmail.com>
@ 2016-12-06 20:46 ` Stephen Frost <sfrost@snowman.net>
2016-12-06 21:59 ` Re: Compiler warnings Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 17+ messages in thread
From: Stephen Frost @ 2016-12-06 20:46 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: pgsql-hackers
Robert,
* Robert Haas (robertmhaas@gmail.com) wrote:
> On Tue, Dec 6, 2016 at 3:23 PM, Stephen Frost <sfrost@snowman.net> 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
Attachments:
[text/x-diff] silence-warnings_v3.patch (2.3K, ../../20161206204644.GS23417@tamriel.snowman.net/2-silence-warnings_v3.patch)
download | inline diff:
From 55ecc3367bd063f05c89b4b1ca881d2b084859f6 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
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
[application/pgp-signature] signature.asc (818B, ../../20161206204644.GS23417@tamriel.snowman.net/3-signature.asc)
download
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Compiler warnings
2016-11-29 15:21 Compiler warnings Stephen Frost <sfrost@snowman.net>
2016-12-06 20:23 ` Re: Compiler warnings Stephen Frost <sfrost@snowman.net>
2016-12-06 20:31 ` Re: Compiler warnings Robert Haas <robertmhaas@gmail.com>
2016-12-06 20:46 ` Re: Compiler warnings Stephen Frost <sfrost@snowman.net>
@ 2016-12-06 21:59 ` Robert Haas <robertmhaas@gmail.com>
2016-12-29 15:47 ` Re: Compiler warnings Joe Conway <mail@joeconway.com>
0 siblings, 1 reply; 17+ messages in thread
From: Robert Haas @ 2016-12-06 21:59 UTC (permalink / raw)
To: Stephen Frost <sfrost@snowman.net>; +Cc: pgsql-hackers
On Tue, Dec 6, 2016 at 3:46 PM, Stephen Frost <sfrost@snowman.net> 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
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Compiler warnings
2016-11-29 15:21 Compiler warnings Stephen Frost <sfrost@snowman.net>
2016-12-06 20:23 ` Re: Compiler warnings Stephen Frost <sfrost@snowman.net>
2016-12-06 20:31 ` Re: Compiler warnings Robert Haas <robertmhaas@gmail.com>
2016-12-06 20:46 ` Re: Compiler warnings Stephen Frost <sfrost@snowman.net>
2016-12-06 21:59 ` Re: Compiler warnings Robert Haas <robertmhaas@gmail.com>
@ 2016-12-29 15:47 ` Joe Conway <mail@joeconway.com>
2017-01-02 18:18 ` Re: Compiler warnings Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 17+ messages in thread
From: Joe Conway @ 2016-12-29 15:47 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; Stephen Frost <sfrost@snowman.net>; +Cc: pgsql-hackers
On 12/06/2016 01:59 PM, Robert Haas wrote:
> On Tue, Dec 6, 2016 at 3:46 PM, Stephen Frost <sfrost@snowman.net> 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
Attachments:
[application/pgp-signature] signature.asc (818B, ../../e7b6eff6-a9de-fe8b-00ef-6fde5d5d8cf0@joeconway.com/2-signature.asc)
download
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Compiler warnings
2016-11-29 15:21 Compiler warnings Stephen Frost <sfrost@snowman.net>
2016-12-06 20:23 ` Re: Compiler warnings Stephen Frost <sfrost@snowman.net>
2016-12-06 20:31 ` Re: Compiler warnings Robert Haas <robertmhaas@gmail.com>
2016-12-06 20:46 ` Re: Compiler warnings Stephen Frost <sfrost@snowman.net>
2016-12-06 21:59 ` Re: Compiler warnings Robert Haas <robertmhaas@gmail.com>
2016-12-29 15:47 ` Re: Compiler warnings Joe Conway <mail@joeconway.com>
@ 2017-01-02 18:18 ` Robert Haas <robertmhaas@gmail.com>
2017-01-02 18:55 ` Re: Compiler warnings Joe Conway <mail@joeconway.com>
0 siblings, 1 reply; 17+ messages in thread
From: Robert Haas @ 2017-01-02 18:18 UTC (permalink / raw)
To: Joe Conway <mail@joeconway.com>; +Cc: Stephen Frost <sfrost@snowman.net>; pgsql-hackers
On Thu, Dec 29, 2016 at 10:47 AM, Joe Conway <mail@joeconway.com> 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
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Compiler warnings
2016-11-29 15:21 Compiler warnings Stephen Frost <sfrost@snowman.net>
2016-12-06 20:23 ` Re: Compiler warnings Stephen Frost <sfrost@snowman.net>
2016-12-06 20:31 ` Re: Compiler warnings Robert Haas <robertmhaas@gmail.com>
2016-12-06 20:46 ` Re: Compiler warnings Stephen Frost <sfrost@snowman.net>
2016-12-06 21:59 ` Re: Compiler warnings Robert Haas <robertmhaas@gmail.com>
2016-12-29 15:47 ` Re: Compiler warnings Joe Conway <mail@joeconway.com>
2017-01-02 18:18 ` Re: Compiler warnings Robert Haas <robertmhaas@gmail.com>
@ 2017-01-02 18:55 ` Joe Conway <mail@joeconway.com>
2017-01-02 19:09 ` Re: Compiler warnings Tom Lane <tgl@sss.pgh.pa.us>
2017-01-02 22:53 ` Re: Compiler warnings Joe Conway <mail@joeconway.com>
0 siblings, 2 replies; 17+ messages in thread
From: Joe Conway @ 2017-01-02 18:55 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Stephen Frost <sfrost@snowman.net>; pgsql-hackers
On 01/02/2017 10:18 AM, Robert Haas wrote:
> On Thu, Dec 29, 2016 at 10:47 AM, Joe Conway <mail@joeconway.com> 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
Attachments:
[application/pgp-signature] signature.asc (818B, ../../21d0ae19-c280-086f-2109-4b1174c74af6@joeconway.com/2-signature.asc)
download
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Compiler warnings
2016-11-29 15:21 Compiler warnings Stephen Frost <sfrost@snowman.net>
2016-12-06 20:23 ` Re: Compiler warnings Stephen Frost <sfrost@snowman.net>
2016-12-06 20:31 ` Re: Compiler warnings Robert Haas <robertmhaas@gmail.com>
2016-12-06 20:46 ` Re: Compiler warnings Stephen Frost <sfrost@snowman.net>
2016-12-06 21:59 ` Re: Compiler warnings Robert Haas <robertmhaas@gmail.com>
2016-12-29 15:47 ` Re: Compiler warnings Joe Conway <mail@joeconway.com>
2017-01-02 18:18 ` Re: Compiler warnings Robert Haas <robertmhaas@gmail.com>
2017-01-02 18:55 ` Re: Compiler warnings Joe Conway <mail@joeconway.com>
@ 2017-01-02 19:09 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-01-02 19:51 ` Re: Compiler warnings Joe Conway <mail@joeconway.com>
1 sibling, 1 reply; 17+ messages in thread
From: Tom Lane @ 2017-01-02 19:09 UTC (permalink / raw)
To: Joe Conway <mail@joeconway.com>; +Cc: Robert Haas <robertmhaas@gmail.com>; Stephen Frost <sfrost@snowman.net>; pgsql-hackers
Joe Conway <mail@joeconway.com> 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
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Compiler warnings
2016-11-29 15:21 Compiler warnings Stephen Frost <sfrost@snowman.net>
2016-12-06 20:23 ` Re: Compiler warnings Stephen Frost <sfrost@snowman.net>
2016-12-06 20:31 ` Re: Compiler warnings Robert Haas <robertmhaas@gmail.com>
2016-12-06 20:46 ` Re: Compiler warnings Stephen Frost <sfrost@snowman.net>
2016-12-06 21:59 ` Re: Compiler warnings Robert Haas <robertmhaas@gmail.com>
2016-12-29 15:47 ` Re: Compiler warnings Joe Conway <mail@joeconway.com>
2017-01-02 18:18 ` Re: Compiler warnings Robert Haas <robertmhaas@gmail.com>
2017-01-02 18:55 ` Re: Compiler warnings Joe Conway <mail@joeconway.com>
2017-01-02 19:09 ` Re: Compiler warnings Tom Lane <tgl@sss.pgh.pa.us>
@ 2017-01-02 19:51 ` Joe Conway <mail@joeconway.com>
2017-01-02 19:58 ` Re: Compiler warnings Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 17+ messages in thread
From: Joe Conway @ 2017-01-02 19:51 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Robert Haas <robertmhaas@gmail.com>; Stephen Frost <sfrost@snowman.net>; pgsql-hackers
On 01/02/2017 11:09 AM, Tom Lane wrote:
> Joe Conway <mail@joeconway.com> 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
Attachments:
[application/pgp-signature] signature.asc (818B, ../../8bdec5ad-808b-fbb3-a33f-b88cbe19cf3b@joeconway.com/2-signature.asc)
download
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Compiler warnings
2016-11-29 15:21 Compiler warnings Stephen Frost <sfrost@snowman.net>
2016-12-06 20:23 ` Re: Compiler warnings Stephen Frost <sfrost@snowman.net>
2016-12-06 20:31 ` Re: Compiler warnings Robert Haas <robertmhaas@gmail.com>
2016-12-06 20:46 ` Re: Compiler warnings Stephen Frost <sfrost@snowman.net>
2016-12-06 21:59 ` Re: Compiler warnings Robert Haas <robertmhaas@gmail.com>
2016-12-29 15:47 ` Re: Compiler warnings Joe Conway <mail@joeconway.com>
2017-01-02 18:18 ` Re: Compiler warnings Robert Haas <robertmhaas@gmail.com>
2017-01-02 18:55 ` Re: Compiler warnings Joe Conway <mail@joeconway.com>
2017-01-02 19:09 ` Re: Compiler warnings Tom Lane <tgl@sss.pgh.pa.us>
2017-01-02 19:51 ` Re: Compiler warnings Joe Conway <mail@joeconway.com>
@ 2017-01-02 19:58 ` Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 17+ messages in thread
From: Tom Lane @ 2017-01-02 19:58 UTC (permalink / raw)
To: Joe Conway <mail@joeconway.com>; +Cc: Robert Haas <robertmhaas@gmail.com>; Stephen Frost <sfrost@snowman.net>; pgsql-hackers
Joe Conway <mail@joeconway.com> 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
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Compiler warnings
2016-11-29 15:21 Compiler warnings Stephen Frost <sfrost@snowman.net>
2016-12-06 20:23 ` Re: Compiler warnings Stephen Frost <sfrost@snowman.net>
2016-12-06 20:31 ` Re: Compiler warnings Robert Haas <robertmhaas@gmail.com>
2016-12-06 20:46 ` Re: Compiler warnings Stephen Frost <sfrost@snowman.net>
2016-12-06 21:59 ` Re: Compiler warnings Robert Haas <robertmhaas@gmail.com>
2016-12-29 15:47 ` Re: Compiler warnings Joe Conway <mail@joeconway.com>
2017-01-02 18:18 ` Re: Compiler warnings Robert Haas <robertmhaas@gmail.com>
2017-01-02 18:55 ` Re: Compiler warnings Joe Conway <mail@joeconway.com>
@ 2017-01-02 22:53 ` Joe Conway <mail@joeconway.com>
1 sibling, 0 replies; 17+ messages in thread
From: Joe Conway @ 2017-01-02 22:53 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Stephen Frost <sfrost@snowman.net>; pgsql-hackers
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=55fb759ab3e7543a6be72a35e6b6961455c5b...
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=d97b14ddab2059e1d73c0cd17f26bac4ef13e...
> 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
Attachments:
[application/pgp-signature] signature.asc (818B, ../../aa1fbe0c-44fa-0c95-a087-6f176323ed91@joeconway.com/2-signature.asc)
download
^ permalink raw reply [nested|flat] 17+ messages in thread
end of thread, other threads:[~2017-01-02 22:53 UTC | newest]
Thread overview: 17+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2007-04-19 16:08 Compiler warnings Bruce Momjian <bruce@momjian.us>
2007-04-19 16:18 ` Alvaro Herrera <alvherre@commandprompt.com>
2007-04-19 16:32 ` Bruce Momjian <bruce@momjian.us>
2007-04-19 16:40 ` Alvaro Herrera <alvherre@commandprompt.com>
2016-11-29 15:21 Compiler warnings Stephen Frost <sfrost@snowman.net>
2016-11-29 15:25 ` Stephen Frost <sfrost@snowman.net>
2016-12-06 20:23 ` Stephen Frost <sfrost@snowman.net>
2016-12-06 20:31 ` Robert Haas <robertmhaas@gmail.com>
2016-12-06 20:46 ` Stephen Frost <sfrost@snowman.net>
2016-12-06 21:59 ` Robert Haas <robertmhaas@gmail.com>
2016-12-29 15:47 ` Joe Conway <mail@joeconway.com>
2017-01-02 18:18 ` Robert Haas <robertmhaas@gmail.com>
2017-01-02 18:55 ` Joe Conway <mail@joeconway.com>
2017-01-02 19:09 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-01-02 19:51 ` Joe Conway <mail@joeconway.com>
2017-01-02 19:58 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-01-02 22:53 ` Joe Conway <mail@joeconway.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox