Thanks for the feedback. PSA the v5 patch.
On Wed, Oct 26, 2022 at 7:04 AM Justin Pryzby <pry...@telsasoft.com> wrote:
>
> +#ifdef USE_ASSERT_CHECKING
> + sanity_check_GUC_C_var(hentry->gucvar);
> +#endif
>
> => You can conditionally define that as an empty function so #ifdefs
> aren't needed in the caller:
>
> void sanity_check_GUC_C_var()
> {
> #ifdef USE_ASSERT_CHECKING
> ...
> #endif
> }
>Fixed as suggested. > But actually, I don't think you should use my patch. You needed to > exclude update_process_title: > > src/backend/utils/misc/ps_status.c:bool update_process_title = true; > ... > src/backend/utils/misc/guc_tables.c-#ifdef WIN32 > src/backend/utils/misc/guc_tables.c- false, > src/backend/utils/misc/guc_tables.c-#else > src/backend/utils/misc/guc_tables.c- true, > src/backend/utils/misc/guc_tables.c-#endif > src/backend/utils/misc/guc_tables.c- NULL, NULL, NULL > > My patch would also exclude the 16 other GUCs with compile-time defaults > from your check. It'd be better not to exclude them; I think the right > solution is to change the C variable initialization to a compile-time > constant: > > #ifdef WIN32 > bool update_process_title = false; > #else > bool update_process_title = true; > #endif > > Or something more indirect like: > > #ifdef WIN32 > #define DEFAULT_PROCESS_TITLE false > #else > #define DEFAULT_PROCESS_TITLE true > #endif > > bool update_process_title = DEFAULT_PROCESS_TITLE; > > I suspect there's not many GUCs that would need to change - this might > be the only one. If this GUC were defined in the inverse (bool > skip_process_title), it wouldn't need special help, either. > I re-checked all the GUC C vars which your patch flags as GUC_DEFAULT_COMPILE. For some of them, where it was not any trouble, I made the C var assignment use the same preprocessor rules as used by guc_tables. For others (mostly the string ones) I left the GUC C var untouched because the sanity checker function already has a rule not to complain about int GUC C vars which are 0 or string GUC vars which are NULL. ------ Kind Regards, Peter Smith. Fujitsu Australia
v5-0001-GUC-C-variable-sanity-check.patch
Description: Binary data
