New compiler warning
I am seeing a new gcc 12.2.0 compiler warning from
src/backend/commands/sequence.c:
sequence.c: In function ‘DefineSequence’:
sequence.c:196:35: warning: ‘coldef’ may be used uninitialized [-Wmaybe-uninitialized]
196 | stmt->tableElts = lappend(stmt->tableElts, coldef);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sequence.c:175:29: note: ‘coldef’ was declared here
175 | ColumnDef *coldef;
| ^~~~~~
The code is:
for (i = SEQ_COL_FIRSTCOL; i <= SEQ_COL_LASTCOL; i++)
{
--> ColumnDef *coldef;
switch (i)
{
case SEQ_COL_LASTVAL:
coldef = makeColumnDef("last_value", INT8OID, -1, InvalidOid);
value[i - 1] = Int64GetDatumFast(seqdataform.last_value);
break;
case SEQ_COL_LOG:
coldef = makeColumnDef("log_cnt", INT8OID, -1, InvalidOid);
value[i - 1] = Int64GetDatum((int64) 0);
break;
case SEQ_COL_CALLED:
coldef = makeColumnDef("is_called", BOOLOID, -1, InvalidOid);
value[i - 1] = BoolGetDatum(false);
break;
}
coldef->is_not_null = true;
null[i - 1] = false;
--> stmt->tableElts = lappend(stmt->tableElts, coldef);
}
and I think it is caused by this commit:
commit 1fa9241bdd
Author: Peter Eisentraut <peter@eisentraut.org>
Date: Tue Aug 29 08:41:04 2023 +0200
Make more use of makeColumnDef()
Since we already have it, we might as well make full use of it,
instead of assembling ColumnDef by hand in several places.
Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: /messages/by-id/52a125e4-ff9a-95f5-9f61-b87cf447e4da@eisentraut.org
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
Hi,
I am seeing a new gcc 12.2.0 compiler warning from
src/backend/commands/sequence.c:
Yep, the compiler is just not smart enough to derive that this
actually is not going to happen.
Here is a proposed fix.
--
Best regards,
Aleksander Alekseev
Attachments:
v1-0001-Silence-GCC-12-warning.patchapplication/octet-stream; name=v1-0001-Silence-GCC-12-warning.patchDownload
From 7004dd6c8cd711629f89a5054f4d7244f3e4193e Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <aleksander@timescale.com>
Date: Wed, 30 Aug 2023 15:06:41 +0300
Subject: [PATCH v1] Silence GCC 12 warning
Aleksander Alekseev, reported by Bruce Momjian.
Discussion: https://postgr.es/message-id/ZO8uMPmZ4zb37PpQ%40momjian.us
---
src/backend/commands/sequence.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index 0b0003fcc8..964c8a792b 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -188,6 +188,9 @@ DefineSequence(ParseState *pstate, CreateSeqStmt *seq)
coldef = makeColumnDef("is_called", BOOLOID, -1, InvalidOid);
value[i - 1] = BoolGetDatum(false);
break;
+ default: /* silence the compiler warning */
+ Assert(false);
+ return InvalidObjectAddress;
}
coldef->is_not_null = true;
--
2.42.0
On 8/30/23 08:10, Aleksander Alekseev wrote:
I am seeing a new gcc 12.2.0 compiler warning from
src/backend/commands/sequence.c:Yep, the compiler is just not smart enough to derive that this
actually is not going to happen.Here is a proposed fix.
Here's an alternate way to deal with this which is a bit more efficient
(code not tested):
- case SEQ_COL_CALLED:
- coldef = makeColumnDef("is_called", BOOLOID, -1, InvalidOid);
- value[i - 1] = BoolGetDatum(false);
- break;
+ default:
+ Assert(i == SEQ_COL_CALLED);
+ coldef = makeColumnDef("is_called", BOOLOID, -1, InvalidOid);
+ value[i - 1] = BoolGetDatum(false);
+ break;The downside is that any garbage in i will lead to processing as
SEQ_COL_CALLED. But things are already pretty bad in that case, ISTM,
even with the proposed patch (or the original code for that matter).
Regards,
-David
Peter Eisentraut has applied a patch to fix this.
---------------------------------------------------------------------------
On Wed, Aug 30, 2023 at 10:07:24AM -0400, David Steele wrote:
On 8/30/23 08:10, Aleksander Alekseev wrote:
I am seeing a new gcc 12.2.0 compiler warning from
src/backend/commands/sequence.c:Yep, the compiler is just not smart enough to derive that this
actually is not going to happen.Here is a proposed fix.
Here's an alternate way to deal with this which is a bit more efficient
(code not tested):- case SEQ_COL_CALLED: - coldef = makeColumnDef("is_called", BOOLOID, -1, InvalidOid); - value[i - 1] = BoolGetDatum(false); - break; + default: + Assert(i == SEQ_COL_CALLED); + coldef = makeColumnDef("is_called", BOOLOID, -1, InvalidOid); + value[i - 1] = BoolGetDatum(false); + break;The downside is that any garbage in i will lead to processing as
SEQ_COL_CALLED. But things are already pretty bad in that case, ISTM, even
with the proposed patch (or the original code for that matter).Regards,
-David
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.