On 12.04.2026 20:05, Tom Lane wrote: > Heikki Linnakangas <[email protected]> writes: >> Pushed 0001 as commit 6f5ad00ab7. > > This commit has caused Coverity to start complaining that > most of ginExtractEntries() is unreachable: > > *** CID 1691468: Control flow issues (DEADCODE) > /srv/coverity/git/pgsql-git/postgresql/src/backend/access/gin/ginutil.c: 495 > in ginExtractEntries() > 489 /* > 490 * Scan the items for any NULLs. All NULLs are considered > equal, so we > 491 * just need to check and remember if there are any. We remove > them from > 492 * the array here, and after deduplication, put back one NULL > entry to > 493 * represent them all. > 494 */ >>>> CID 1691468: Control flow issues (DEADCODE) >>>> Execution cannot reach this statement: "hasNull = false;". > 495 hasNull = false; > 496 if (nullFlags) > 497 { > 498 int32 numNonNulls = 0; > 499 > 500 for (int32 i = 0; i < nentries; i++) > > Evidently, it does not realize that the extractValueFn() can change > nentries from its initial value of zero. I wouldn't be too surprised > if that's related to our casting of the pointer to uintptr_t --- that > may cause it to not see the passed pointer as a potential reference > mechanism.
Curious that we don't see that more frequently for other functions that have output arguments. But maybe there are just too few? > I would just write that off as Coverity not being smart enough, except > that I'm worried that some compiler might make a similar deduction and > break the function completely. Was the switch to a local variable > for nentries really a useful win performance-wise? I haven't benchmarked the variant with using the pointer directly. I can do that. -- David Geier
