On Tue, 02 Apr 2024 at 07:59, Alexander Korotkov <aekorot...@gmail.com> wrote: > On Mon, Apr 1, 2024 at 7:36 PM Tom Lane <t...@sss.pgh.pa.us> wrote: >> Coverity complained about what you did in RelationParseRelOptions >> in c95c25f9a: >> >> *** CID 1595992: Null pointer dereferences (FORWARD_NULL) >> /srv/coverity/git/pgsql-git/postgresql/src/backend/utils/cache/relcache.c: >> 499 in RelationParseRelOptions() >> 493 >> 494 /* >> 495 * Fetch reloptions from tuple; have to use a hardwired >> descriptor because >> 496 * we might not have any other for pg_class yet (consider >> executing this >> 497 * code for pg_class itself) >> 498 */ >> >>> CID 1595992: Null pointer dereferences (FORWARD_NULL) >> >>> Passing null pointer "tableam" to "extractRelOptions", which >> >>> dereferences it. >> 499 options = extractRelOptions(tuple, GetPgClassDescriptor(), >> 500 >> tableam, amoptsfn); >> 501 >> >> I see that extractRelOptions only uses the tableam argument for some >> relkinds, and RelationParseRelOptions does set it up for those >> relkinds --- but Coverity's complaint isn't without merit, because >> those two switch statements are looking at *different copies of the >> relkind*, which in theory could be different. This all seems quite >> messy and poorly factored. Can't we do better? Why do we need to >> involve two copies of allegedly the same pg_class tuple, anyhow? > > I wasn't registered at Coverity yet. Now I've registered and am > waiting for approval to access the PostgreSQL analysis data. > > I wonder why Coverity complains about tableam, but not amoptsfn. > Their usage patterns are very similar. > > It appears that relation->rd_rel isn't the full copy of pg_class tuple > (see AllocateRelationDesc). RelationParseRelOptions() is going to > update relation->rd_options, and thus needs a full pg_class tuple to > fetch options out of it. However, it is really unnecessary to access > both tuples at the same time. We can use a full tuple, not > relation->rd_rel, in both cases. See the attached patch. > > ------ > Regards, > Alexander Korotkov
+ Form_pg_class classForm = (Form_pg_class) GETSTRUCT(tuple); +; There is an additional semicolon in the code. -- Regards, Japin Li
