Re: [PATCH] Guard `CLOBBER_FREED_MEMORY` & `MEMORY_CONTEXT_CHECKING`

Hmm… so you're worried about people setting the definition to 0 for
disabled and nonzero || present for enabled? - I suppose all the
`#define`s in PostgreSQL could be guarded and checked for both
presence and either empty value or value. But maybe you just don't
want to touch this side of the codebase?

https://gcc.gnu.org/onlinedocs/cpp/If.html
https://learn.microsoft.com/en-us/cpp/preprocessor/hash-if-hash-elif-hash-else-and-hash-endif-directives-c-cpp?view=msvc-170
(assuming similar on clang and other compilers; I'm sure this
behaviour predated C89)

It would be odd for people to depend on behaviour like not defining
CLOBBER_FREED_MEMORY at the CPPFLAGS level. Anyway, I assume it's your
call, so should I withdraw this PATCH then? - Or make a more
comprehensive PATCH checking for definition and (emptiness or
nonzero)?

PS: I did send through a PR to that
build-PostgreSQL-extensions-with-Rust project
https://github.com/pgcentralfoundation/pgrx/pull/1826

Samuel Marks, PhD
https://linkedin.com/in/samuelmarks

On Fri, Aug 23, 2024 at 4:39 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Samuel Marks <samuelmarks(at)gmail(dot)com> writes:
> > It will resolve the large number of these warnings from
> > https://github.com/pgcentralfoundation/pgrx/blob/6dfb9d1/cargo-pgrx/src/command/init.rs#L411-L412:
>
> Hmm, that seems like their problem not ours. It's not very clear
> to me why they'd want to force these flags from the compiler
> command line in the first place, but if they do they should be
> consistent with the more usual ways to set them.
>
> > and yes will be sending them a patch also. But there's no harm in not
> > redefining symbols, so not sure why this is a controversial patch.
>
> The reason I'm resistant to changing it is that the code you want
> to touch has been unchanged since 2003 in the first case, and 2013
> in the second. It's fairly unclear what external code might have
> grown dependencies on the current behavior, but with that much
> history I'm not eager to bet that the answer is "none". Also,
> the present setup makes it clear that you are supposed to test
> "#ifdef CLOBBER_FREED_MEMORY" not "#if CLOBBER_FREED_MEMORY".
> If we stop locking down the expected contents of the macro, bugs
> of that sort could sneak in.
>
> regards, tom lane