I wrote: > ...but arguably the earlier check is close enough that it's silly to assert in the "else" branch, and I'd be okay with just nuking those lines. Another thing that caught my attention is the assumption that unsetting a bit is so expensive that we have to first check if it's set, so we may as well remove "IS_BRACKET(Np->Num)" as well.
The attached is what I mean. I'll commit this this week unless there are objections. -- John Naylor EDB: http://www.enterprisedb.com
From 5dd409e6059be464ad635ce8cd885fc3de5d8e43 Mon Sep 17 00:00:00 2001 From: John Naylor <john.nay...@postgresql.org> Date: Mon, 16 Jan 2023 13:03:22 +0700 Subject: [PATCH v4] Remove dead code in formatting.c Remove some code guarded by IS_MINUS() or IS_PLUS(), where the entire stanza is inside an else-block where both of these are false. This should slightly improve test coverage. While at it, remove coding that apparently assumes that unsetting a bit is so expensive that we have to first check if it's already set in the first place. Reviewed by Justin Pryzby Per Coverity report from Ranier Vilela Discussion: https://www.postgresql.org/message-id/20221223010818.GP1153%40telsasoft.com --- src/backend/utils/adt/formatting.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c index a4b524ea3a..f3f4db5ef6 100644 --- a/src/backend/utils/adt/formatting.c +++ b/src/backend/utils/adt/formatting.c @@ -5664,13 +5664,9 @@ NUM_processor(FormatNode *node, NUMDesc *Num, char *inout, { if (Np->sign != '-') { - if (IS_BRACKET(Np->Num) && IS_FILLMODE(Np->Num)) + if (IS_FILLMODE(Np->Num)) Np->Num->flag &= ~NUM_F_BRACKET; - if (IS_MINUS(Np->Num)) - Np->Num->flag &= ~NUM_F_MINUS; } - else if (Np->sign != '+' && IS_PLUS(Np->Num)) - Np->Num->flag &= ~NUM_F_PLUS; if (Np->sign == '+' && IS_FILLMODE(Np->Num) && IS_LSIGN(Np->Num) == false) Np->sign_wrote = true; /* needn't sign */ -- 2.39.0
