On 10/28/25 4:13 PM, Bertrand Drouvot wrote:
> Hi hackers,
>
> While working on refactoring some code in [1], one of the changes was:
>
> - if (initial_restart_lsn != InvalidXLogRecPtr &&
> - initial_restart_lsn < oldestLSN)
> + XLogRecPtr restart_lsn = s->data.restart_lsn;
> +
> + if (restart_lsn != InvalidXLogRecPtr &&
> + restart_lsn < oldestLSN)
>
> Sawada-san suggested to use the XLogRecPtrIsInvalid() macro here.
>
> But this != InvalidXLogRecPtr check was existing code, so why not consistently
> use XLogRecPtrIsInvalid() where we check equality against InvalidXLogRecPtr?
>
> At the time the current XLogRecPtrIsInvalid() has been introduced (0ab9d1c4b316)
> all the InvalidXLogRecPtr equality checks were done using XLogRecPtrIsInvalid().
>
> But since, it has changed: I looked at it and this is not the case anymore in
> 20 files.
>
> PFA, patches to $SUBJECT. To ease the review, I created one patch per modified
> file.
>
> I suspect the same approach could be applied to some other macros too. Let's
> start with XLogRecPtrIsInvalid() first.
>
Agree. This patch has made the code style more consistent. And using
such macros is also in line with the usual practice. Just like
OidIsValid and TransactionIdIsValid.
> I think that's one of the things we could do once a year, like Peter does with
> his annual "clang-tidy" check ([2]).
>
> Thoughts?
>
> [1]:
https://www.postgresql.org/message-id/CAD21AoB_C6V1PLNs%3DSuOejgGh1o6ZHJMstD7X4X1b_z%3D%3DLdH1Q%40mail.gmail.com
> [2]: https://www.postgresql.org/message-id/CAH2-WzmxPQAF_ZhwrUo3rzVk3yYj_4mqbgiQXAGGO5nFYV3D8Q@mail.gmail.com
>
> Regards,
>
