Re: Consistently use the XLogRecPtrIsInvalid() macro

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,
>