Date: Tue, 6 Dec 2016 15:46:44 -0500
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
Message-ID: <20161206204644.GS23417@tamriel.snowman.net>
References: <20161129152102.GR13284@tamriel.snowman.net>
 <20161206202305.GQ23417@tamriel.snowman.net>
 <CA+Tgmoa5UOy=U_jWVyyJcpR4gqWREcq6KUD+afRbrakqyhe9aQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="ahZICQ7iXVM/oLYH"
Content-Disposition: inline
In-Reply-To: 
 <CA+Tgmoa5UOy=U_jWVyyJcpR4gqWREcq6KUD+afRbrakqyhe9aQ@mail.gmail.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org


--ahZICQ7iXVM/oLYH
Content-Type: multipart/mixed; boundary="JEhTuUFIcUlI65CC"
Content-Disposition: inline


--JEhTuUFIcUlI65CC
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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.
>=20
> In LWLockRelease, why not just move mode =3D 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

--JEhTuUFIcUlI65CC
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="silence-warnings_v3.patch"
Content-Transfer-Encoding: quoted-printable

=46rom 55ecc3367bd063f05c89b4b1ca881d2b084859f6 Mon Sep 17 00:00:00 2001
=46rom: 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%40=
tamriel.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/l=
wlock.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 =3D num_held_lwlocks; --i >=3D 0;)
-	{
 		if (lock =3D=3D held_lwlocks[i].lock)
-		{
-			mode =3D held_lwlocks[i].mode;
 			break;
-		}
-	}
+
 	if (i < 0)
 		elog(ERROR, "lock %s %d is not held", T_NAME(lock), T_ID(lock));
+
+	mode =3D held_lwlocks[i].mode;
+
 	num_held_lwlocks--;
 	for (; i < num_held_lwlocks; i++)
 		held_lwlocks[i] =3D 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 =3D NULL;
 	List	   *qlist;
 	bool		customplan;
=20
@@ -1210,6 +1210,8 @@ GetCachedPlan(CachedPlanSource *plansource, ParamList=
Info boundParams,
 		}
 	}
=20
+	Assert(plan !=3D NULL);
+
 	/* Flag the plan as in use by caller */
 	if (useResOwner)
 		ResourceOwnerEnlargePlanCacheRefs(CurrentResourceOwner);
--=20
2.7.4


--JEhTuUFIcUlI65CC--

--ahZICQ7iXVM/oLYH
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: Digital signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBCgAGBQJYRyO0AAoJEO1sijiDR2RVL8cP/jCa3BTLApFoE+wtfvBxabox
Lhi/fn+veTZ04UPXdbdkVhRBy8IOHw0rPnbvxTLtdkzU1RxK9VWkcwn34eH8NN4K
VHhCkLhNf3OyRYF7S2Ik0EI3jrWcHE8rBVqmDh+xbxvunbNZNbTcVbOXZNVmKLB0
Fl7gKC8lKSJhrzO9a7r0P5ZoiqGMkHfA3wCL0aZSW74DAgHVQttGmSfMfvKhT4xq
nGM9zu/q4FjEBd8yPlRSIYl6jwmhsy2lYsluElvjO2gyEKBKaSH0PWxpgNPjU6oZ
tMVQOSV4r7URE3Nk+q62Z4w+SKSOWMU/sRl6NFFJ3uTFNe6hKvyJtZlVblbmWkIl
2Bs2sgjsCTzlYuDTI80X52DMpxOrVKT3tLGrNU1pLWMRTOi9UGIGnGDlet39ze8+
N9Nw/D8s3L/TasJdaw8Jv2YKJToqmrYrVs4twua4BZnHkk1JCzotuLI4iCMQtQb9
T3y4FWUsEYVjmIDEXXeFYskjgFjKT8LFbJGW5KMINouMfRnc9RidEvrFin/5cjWb
D+ZRbTUaC+w/fxzDVAmviM1QhgX8TfCw1R82ioFkdmt/3tbhp8kPzSINYS0hi9wB
66fGoe4GuV1B6MYeFMwE2+JP2pBX0Dtg0LLCi1BTrqoNiBPuz9Q3SYY1m7Dndo8v
d70F/lhIv+IAYlp+ahER
=+gnK
-----END PGP SIGNATURE-----

--ahZICQ7iXVM/oLYH--