Thanks for the feedback. PSA the v5 patch.
On Wed, Oct 26, 2022 at 7:04 AM Justin Pryzby <pryzby@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
