From: Stephen Frost <sfrost@snowman.net>
To: Robert Haas <robertmhaas@gmail.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Subject: Re: Compiler warnings
Date: Tue, 6 Dec 2016 15:46:44 -0500
Message-ID: <20161206204644.GS23417@tamriel.snowman.net> (raw)
In-Reply-To: <CA+Tgmoa5UOy=U_jWVyyJcpR4gqWREcq6KUD+afRbrakqyhe9aQ@mail.gmail.com>
References: <20161129152102.GR13284@tamriel.snowman.net>
<20161206202305.GQ23417@tamriel.snowman.net>
<CA+Tgmoa5UOy=U_jWVyyJcpR4gqWREcq6KUD+afRbrakqyhe9aQ@mail.gmail.com>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-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.cindex 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.cindex 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: sfrost@snowman.net, robertmhaas@gmail.com
Subject: Re: Compiler warnings
In-Reply-To: <20161206204644.GS23417@tamriel.snowman.net>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
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