Hi, On 2022-05-24 11:41:49 -0700, Nathan Bossart wrote: > On Mon, May 23, 2022 at 11:39:16PM -0500, Steve Chavez wrote: > > The DefineCustomStringVariable function(or any > > other DefineCustomXXXVariable) has a short_desc parameter that can be > > NULL and it's not apparent that this will lead to a segfault when SHOW ALL > > is used. > > This happens because the ShowAllGUCConfig function expects a non-NULL > > short_desc. > > > > This happened for the Supabase supautils extension( > > https://github.com/supabase/supautils/issues/24) and any other extension > > that uses the DefineCustomXXXVariable has the same bug risk. > > > > This patch does an Assert on the short_desc(also on the name as an extra > > measure), so a postgres built with --enable-cassert can prevent the above > > issue. > > I would actually ERROR on this so that we aren't relying on > --enable-cassert builds to catch it.
How about adding pg_nonnull(...) (ending up as __attribute__((nonnull(...))? Then code passing NULLs would get compiler warnings? It'd be useful in quite a few more places. > That being said, if there's no strong reason to enforce that a short > description be provided, then why not adjust ShowAllGUCConfig() to set that > column to NULL when short_desc is missing? There's a bunch more places that'd need to be adjusted, if we go that way. I don't really have an opinion on it. Greetings, Andres Freund
