From: | Peter Smith <smithpb2250(at)gmail(dot)com> |
---|---|
To: | Justin Pryzby <pryzby(at)telsasoft(dot)com> |
Cc: | Michael Paquier <michael(at)paquier(dot)xyz>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Nathan Bossart <nathandbossart(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: GUC values - recommended way to declare the C variables? |
Date: | 2022-10-26 07:31:56 |
Message-ID: | CAHut+PuCHjYXiTGdTOvHvDnjpbivLLr49gWVS+8VwnfoM4hJTw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Thanks for the feedback. PSA the v5 patch.
On Wed, Oct 26, 2022 at 7:04 AM Justin Pryzby <pryzby(at)telsasoft(dot)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
Attachment | Content-Type | Size |
---|---|---|
v5-0001-GUC-C-variable-sanity-check.patch | application/octet-stream | 11.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Bharath Rupireddy | 2022-10-26 08:11:12 | Re: Some regression tests for the pg_control_*() functions |
Previous Message | Michael Paquier | 2022-10-26 07:18:47 | Re: Some regression tests for the pg_control_*() functions |