From: | Xing Guo <higuoxing(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Aleksander Alekseev <aleksander(at)timescale(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Junwang Zhao <zhjwpku(at)gmail(dot)com> |
Subject: | Re: Don't pass NULL pointer to strcmp(). |
Date: | 2023-11-01 23:45:33 |
Message-ID: | CACpMh+BoYMLj-ZmzCY8LChQWjyxgxxuGLf+Mmoz8NAg2o_3v7w@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi Tom,
There're extensions that set their boot_val to NULL. E.g., postgres_fdw (
https://github.com/postgres/postgres/blob/4210b55f598534db9d52c4535b7dcc777dda75a6/contrib/postgres_fdw/option.c#L582)
plperl (
https://github.com/postgres/postgres/blob/4210b55f598534db9d52c4535b7dcc777dda75a6/src/pl/plperl/plperl.c#L422C13-L422C13,
https://github.com/postgres/postgres/blob/4210b55f598534db9d52c4535b7dcc777dda75a6/src/pl/plperl/plperl.c#L444C12-L444C12,
https://github.com/postgres/postgres/blob/4210b55f598534db9d52c4535b7dcc777dda75a6/src/pl/plperl/plperl.c#L452C6-L452C6)
(Can we treat plperl as an extension?), pltcl (
https://github.com/postgres/postgres/blob/4210b55f598534db9d52c4535b7dcc777dda75a6/src/pl/tcl/pltcl.c#L465C14-L465C14,
https://github.com/postgres/postgres/blob/4210b55f598534db9d52c4535b7dcc777dda75a6/src/pl/tcl/pltcl.c#L472C12-L472C12
).
TBH, I don't know if NULL is a valid boot_val for string variables, I just
came across some extensions that use NULL as their boot_val. If the
boot_val can't be NULL in extensions, we should probably add some
assertions or comments about it?
Best Regards,
Xing
On Wed, Nov 1, 2023 at 11:30 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Xing Guo <higuoxing(at)gmail(dot)com> writes:
> > Thanks for your comments. I have updated the patch accordingly.
>
> I'm leery of accepting this patch, as I see no reason that we
> should consider it valid for an extension to have a string GUC
> with a boot_val of NULL.
>
> I realize that we have a few core GUCs that are like that, but
> I'm pretty sure that every one of them has special-case code
> that initializes the GUC to something non-null a bit later on
> in startup. I don't think there are any cases where a string
> GUC's persistent value will be null, and I don't like the
> idea of considering that to be an allowed case. It would
> open the door to more crash situations, and it brings up the
> old question of how could a user tell NULL from empty string
> (via SHOW or current_setting() or whatever). Besides, what's
> the benefit really?
>
> regards, tom lane
>
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2023-11-01 23:49:25 | Re: document deviation from standard on REVOKE ROLE |
Previous Message | David Rowley | 2023-11-01 23:42:51 | Properly pathify the union planner |