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_4mqbgiqxaggo5nfyv3...@mail.gmail.com
Regards,
