From b2def22ab1b9ebdbf5c6475de7872ababe5022e9 Mon Sep 17 00:00:00 2001 From: Jelte Fennema Date: Fri, 20 Nov 2020 16:02:03 +0100 Subject: [PATCH] Fix possible uninitialized variable warning (#4334) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I got this warning when compiling citus: ``` ../columnar/write_state_management.c: In function ‘PendingWritesInUpperTransactions’: ../columnar/write_state_management.c:364:20: warning: ‘entry’ may be used uninitialized in this function [-Wmaybe-uninitialized] if (found && entry->writeStateStack != NULL) ~~~~~^~~~~~~~~~~~~~~~ ``` I fixed this by checking by always initializing entry, by using an early return if `WriteStateMap` didn't exist. Instead of using the `found` variable to check for existence of the key, I now simply check the `entry` variable itself. To quote the postgres comment on the hash_enter function: > If foundPtr isn't NULL, then *foundPtr is set true if we found an > existing entry in the table, false otherwise. This is needed in the > HASH_ENTER case, but is redundant with the return value otherwise. --- src/backend/columnar/write_state_management.c | 30 ++++++++----------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/src/backend/columnar/write_state_management.c b/src/backend/columnar/write_state_management.c index 99b7aa9e9..30b93a5d0 100644 --- a/src/backend/columnar/write_state_management.c +++ b/src/backend/columnar/write_state_management.c @@ -200,17 +200,16 @@ cstore_init_write_state(RelFileNode relfilenode, TupleDesc tupdesc, void FlushWriteStateForRelfilenode(Oid relfilenode, SubTransactionId currentSubXid) { - WriteStateMapEntry *entry = NULL; - bool found = false; - - if (WriteStateMap) + if (WriteStateMap == NULL) { - entry = hash_search(WriteStateMap, &relfilenode, HASH_FIND, &found); + return; } - Assert(!found || !entry->dropped); + WriteStateMapEntry *entry = hash_search(WriteStateMap, &relfilenode, HASH_FIND, NULL); - if (found && entry->writeStateStack != NULL) + Assert(!entry || !entry->dropped); + + if (entry && entry->writeStateStack != NULL) { SubXidWriteState *stackEntry = entry->writeStateStack; if (stackEntry->subXid == currentSubXid) @@ -315,16 +314,14 @@ DiscardWriteStateForAllRels(SubTransactionId currentSubXid, SubTransactionId par void MarkRelfilenodeDropped(Oid relfilenode, SubTransactionId currentSubXid) { - bool found = false; - if (WriteStateMap == NULL) { return; } WriteStateMapEntry *entry = hash_search(WriteStateMap, &relfilenode, HASH_FIND, - &found); - if (!found || entry->dropped) + NULL); + if (!entry || entry->dropped) { return; } @@ -353,15 +350,14 @@ NonTransactionDropWriteState(Oid relfilenode) bool PendingWritesInUpperTransactions(Oid relfilenode, SubTransactionId currentSubXid) { - WriteStateMapEntry *entry; - bool found = false; - - if (WriteStateMap) + if (WriteStateMap == NULL) { - entry = hash_search(WriteStateMap, &relfilenode, HASH_FIND, &found); + return false; } - if (found && entry->writeStateStack != NULL) + WriteStateMapEntry *entry = hash_search(WriteStateMap, &relfilenode, HASH_FIND, NULL); + + if (entry && entry->writeStateStack != NULL) { SubXidWriteState *stackEntry = entry->writeStateStack;