public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v3 1/1] Handle a NULL short_desc in ShowAllGUCConfig 2+ messages / 2 participants [nested] [flat]
* [PATCH v3 1/1] Handle a NULL short_desc in ShowAllGUCConfig @ 2022-05-25 04:46 Steve Chavez <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Steve Chavez @ 2022-05-25 04:46 UTC (permalink / raw) Prevent a segfault when using a SHOW ALL, in case a DefineCustomXXXVariable() was defined with a NULL short_desc. --- src/backend/utils/misc/guc.c | 16 +++++++++++++--- src/include/c.h | 10 ++++++++++ src/include/utils/guc.h | 10 +++++----- 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 8e9b71375c..55d41ae7d6 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -9780,7 +9780,16 @@ ShowAllGUCConfig(DestReceiver *dest) isnull[1] = true; } - values[2] = PointerGetDatum(cstring_to_text(conf->short_desc)); + if (conf->short_desc) + { + values[2] = PointerGetDatum(cstring_to_text(conf->short_desc)); + isnull[2] = false; + } + else + { + values[2] = PointerGetDatum(NULL); + isnull[2] = true; + } /* send it to dest */ do_tup_output(tstate, values, isnull); @@ -9792,7 +9801,8 @@ ShowAllGUCConfig(DestReceiver *dest) pfree(setting); pfree(DatumGetPointer(values[1])); } - pfree(DatumGetPointer(values[2])); + if (conf->short_desc) + pfree(DatumGetPointer(values[2])); } end_tup_output(tstate); @@ -10002,7 +10012,7 @@ GetConfigOptionByNum(int varnum, const char **values, bool *noshow) values[3] = _(config_group_names[conf->group]); /* short_desc */ - values[4] = _(conf->short_desc); + values[4] = conf->short_desc != NULL ? _(conf->short_desc) : NULL; /* extra_desc */ values[5] = conf->long_desc != NULL ? _(conf->long_desc) : NULL; diff --git a/src/include/c.h b/src/include/c.h index 4f16e589b3..f253aa930b 100644 --- a/src/include/c.h +++ b/src/include/c.h @@ -144,6 +144,16 @@ #define pg_attribute_no_sanitize_alignment() #endif +/* + * pg_attribute_nonnull means the compiler should warn if the function is called + * with the listed arguments set to NULL. + */ +#if defined(__clang_major__) || defined(__GNUC__) +#define pg_attribute_nonnull(...) __attribute__((nonnull(__VA_ARGS__))) +#else +#define pg_attribute_nonnull(...) +#endif + /* * Append PG_USED_FOR_ASSERTS_ONLY to definitions of variables that are only * used in assert-enabled builds, to avoid compiler warnings about unused diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h index efcbad7842..be691c5e9c 100644 --- a/src/include/utils/guc.h +++ b/src/include/utils/guc.h @@ -303,7 +303,7 @@ extern void DefineCustomBoolVariable(const char *name, int flags, GucBoolCheckHook check_hook, GucBoolAssignHook assign_hook, - GucShowHook show_hook); + GucShowHook show_hook) pg_attribute_nonnull(1, 4); extern void DefineCustomIntVariable(const char *name, const char *short_desc, @@ -316,7 +316,7 @@ extern void DefineCustomIntVariable(const char *name, int flags, GucIntCheckHook check_hook, GucIntAssignHook assign_hook, - GucShowHook show_hook); + GucShowHook show_hook) pg_attribute_nonnull(1, 4); extern void DefineCustomRealVariable(const char *name, const char *short_desc, @@ -329,7 +329,7 @@ extern void DefineCustomRealVariable(const char *name, int flags, GucRealCheckHook check_hook, GucRealAssignHook assign_hook, - GucShowHook show_hook); + GucShowHook show_hook) pg_attribute_nonnull(1, 4); extern void DefineCustomStringVariable(const char *name, const char *short_desc, @@ -340,7 +340,7 @@ extern void DefineCustomStringVariable(const char *name, int flags, GucStringCheckHook check_hook, GucStringAssignHook assign_hook, - GucShowHook show_hook); + GucShowHook show_hook) pg_attribute_nonnull(1, 4); extern void DefineCustomEnumVariable(const char *name, const char *short_desc, @@ -352,7 +352,7 @@ extern void DefineCustomEnumVariable(const char *name, int flags, GucEnumCheckHook check_hook, GucEnumAssignHook assign_hook, - GucShowHook show_hook); + GucShowHook show_hook) pg_attribute_nonnull(1, 4); extern void MarkGUCPrefixReserved(const char *className); -- 2.25.1 --Dxnq1zWXvFF0Q93v-- ^ permalink raw reply [nested|flat] 2+ messages in thread
* Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c) @ 2023-07-06 19:22 Ranier Vilela <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Ranier Vilela @ 2023-07-06 19:22 UTC (permalink / raw) To: Gurjeet Singh <[email protected]>; +Cc: Karina Litskevich <[email protected]>; Richard Guo <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]; pgsql-hackers Em qui., 6 de jul. de 2023 às 16:06, Gurjeet Singh <[email protected]> escreveu: > On Thu, Jul 6, 2023 at 8:01 AM Karina Litskevich > <[email protected]> wrote: > > > > > >> EB_SMGR and EB_REL are macros for making new structs. > >> IMO these are buggy, once make new structs without initializing all > fields. > >> Attached a patch to fix this and make more clear when rel or smgr is > NULL. > > > > > > As long as a structure is initialized, its fields that are not present in > > initialization are initialized to zeros and NULLs depending on their > types. > > See C99 Standard 6.7.8.21 and 6.7.8.10. This behaviour is quite well > known, > > so I don't think this place is buggy. Anyway, if someone else says the > code > > is more readable with these fields initialized explicitly, then go on. > > Even though I am not a fan of the Designated Initializers feature, I > agree with Karina. Per the standard, the unmentioned fields get > initialized to zeroes/NULLs, so the explicit initialization to > zero/null that this additional patch does is unnecessary. Moreover, I > feel that it makes the code less pleasant to read. > > C99, 6.7.8.21: > > If there are fewer initializers in a brace-enclosed list than there are > > elements or members of an aggregate, or fewer characters in a string > literal > > used to initialize an array of known size than there are elements in the > array, > > the remainder of the aggregate shall be initialized implicitly the same > as > > objects that have static storage duration. > > C99, 6.7.8.10: > > If an object that has automatic storage duration is not initialized > explicitly, > > its value is indeterminate. The key points are here. The object is not static storage duration. The object is struct with "automatic storage duration". And not all compilers follow the standards, they tend to vary quite a bit. regards, Ranier Vilela ^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2023-07-06 19:22 UTC | newest] Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2022-05-25 04:46 [PATCH v3 1/1] Handle a NULL short_desc in ShowAllGUCConfig Steve Chavez <[email protected]> 2023-07-06 19:22 Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c) Ranier Vilela <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox