Jeff Davis <[email protected]> writes:
> On Wed, 2026-08-05 at 16:47 +0530, Amit Kapila wrote:
>> BTW, I had also looked at the overall patch series, the idea and
>> high-level code looks good to me. Though I haven't done a detailed
>> testing or review of the same but as Shlok and Kuroda-San seem to
>> have
>> reviewed/tested these patches, I think we can go-ahead with these
>> fixes.
> Thank you all, pushed.
Coverity thinks there is a hole in this logic:
/srv/coverity/git/pgsql-git/postgresql/src/backend/commands/subscriptioncmds.c:
875 in CreateSubscription()
869 values[Anum_pg_subscription_submaxretention - 1] =
870 Int32GetDatum(opts.maxretention);
871 values[Anum_pg_subscription_subretentionactive - 1] =
872 BoolGetDatum(opts.retaindeadtuples);
873 values[Anum_pg_subscription_subserver - 1] =
ObjectIdGetDatum(serverid);
874 if (!OidIsValid(serverid))
>>> CID 1699896: Null pointer dereferences (FORWARD_NULL)
>>> Passing null pointer "conninfo" to "cstring_to_text", which
>>> dereferences it.
875 values[Anum_pg_subscription_subconninfo - 1] =
876 CStringGetTextDatum(conninfo);
877 else
878 nulls[Anum_pg_subscription_subconninfo - 1] = true;
879 if (opts.slot_name)
880 values[Anum_pg_subscription_subslotname - 1] =
AFAICS, it's right: if stmt->servername is set while opts.connect is
not, we'll arrive at this step with serverid filled in but conninfo
still NULL. Even if there's some upstream reason why that combination
can't occur, this is pretty fragile-looking code. It's far from clear
why serverid has anything to do with conninfo being available.
regards, tom lane
