Обсуждение: 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: https://www.postgresql.org/message-id/flat/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
Вложения
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.