From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
---|---|
To: | Peter Eisentraut <peter(at)eisentraut(dot)org> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Remove PointerIsValid() |
Date: | 2025-09-17 08:11:40 |
Message-ID: | ABE8F86E-67C2-47F9-946F-DEBFC9C55310@gmail.com |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
> On Sep 17, 2025, at 13:21, Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:
>
> I think there is agreement that the PointerIsValid() macro is pretty useless. This patch proposes to remove it. There have been a few recent mini-discussions in other threads that appear to support this. [0][1]
>
> There were the usual concerns about code churn and backpatching and so on, but I think in the end the change is not that big and it's in pretty boring positions. Also, you can backpatch code without PointerIsValid() just fine. You might run into trouble if you forward-patch. :-/
>
> While converting the code, I tried to find a balance of style of
>
> if (PointerIsValid(foo))
>
> to
>
> if (foo)
>
> or
>
> if (foo != NULL)
>
> but there is no deterministic reason.
>
> (Note that when you convert the first form to the third form, you have to flip the overall sense of the logic, which might look confusing in some places.)
>
>
> [0]: https://www.postgresql.org/message-id/CA+hUKG+NFKnr=K4oybwDvT35dW=VAjAAfiuLxp+5JeZSOV3nBg@mail.gmail.com
> [1]: https://www.postgresql.org/message-id/bccf2803-5252-47c2-9ff0-340502d5bd1c@iki.fi<0001-Remove-PointerIsValid.patch>
Given the context of replacing PointerIsValid(x), I think if (foo != NULL) is slightly better than if (x), because that explicitly shows the intent of checking pointers, while if (x) works for both pointers and integers.
In this patch, you mix using the both forms, should we just use a single form?
Regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
From | Date | Subject | |
---|---|---|---|
Next Message | Oleg Tselebrovskiy | 2025-09-17 08:22:48 | Re: ICU warnings during make installcheck and text_extensions test |
Previous Message | Chao Li | 2025-09-17 07:41:40 | Re: Add SPLIT PARTITION/MERGE PARTITIONS commands |