Thread: Fix unsigned output for signed values in SLRU error reporting
Hi, hackers!
--
I've noticed that in SRLU error reporting both signed and unsigned values are output as %u. I think it is worth correcting this with the very simple patch attached.
Thanks!
Attachment
Hi, On 2022-03-18 22:52:02 +0400, Pavel Borisov wrote: > I've noticed that in SRLU error reporting both signed and unsigned values > are output as %u. I think it is worth correcting this with the very simple > patch attached. Afaics offset etc can't be negative, so I don't think this really improves matters. I think there's quite a few other places where we use %u to print integers that we know aren't negative. If anything I think we should change the signed integers to unsigned ones. It might be worth doing that as part of https://www.postgresql.org/message-id/CAJ7c6TPDOYBYrnCAeyndkBktO0WG2xSdYduTF0nxq%2BvfkmTF5Q%40mail.gmail.com Greetings, Andres Freund
Afaics offset etc can't be negative, so I don't think this really improves
matters. I think there's quite a few other places where we use %u to print
integers that we know aren't negative.
If anything I think we should change the signed integers to unsigned ones. It
might be worth doing that as part of
https://www.postgresql.org/message-id/CAJ7c6TPDOYBYrnCAeyndkBktO0WG2xSdYduTF0nxq%2BvfkmTF5Q%40mail.gmail.com
That was one of my intentions in the mentioned patch, but I couldn't confirm that the page number (and offset) in SLRU was used signed not by purpose. Thank you for confirming this. I will try to replace int to unsigned where it is relevant in SLRU as part of the mentioned thread. Though it could be a big change worth a separate patch maybe.
Again thanks!
Pavel
пн, 21 мар. 2022 г. в 16:11, Pavel Borisov <pashkin.elfe@gmail.com>:
In the patchset where we're working on making SLRU 64bit [1] we have come to agreement that:Afaics offset etc can't be negative, so I don't think this really improves
matters. I think there's quite a few other places where we use %u to print
integers that we know aren't negative.
If anything I think we should change the signed integers to unsigned ones. It
might be worth doing that as part of
https://www.postgresql.org/message-id/CAJ7c6TPDOYBYrnCAeyndkBktO0WG2xSdYduTF0nxq%2BvfkmTF5Q%40mail.gmail.comThat was one of my intentions in the mentioned patch, but I couldn't confirm that the page number (and offset) in SLRU was used signed not by purpose. Thank you for confirming this. I will try to replace int to unsigned where it is relevant in SLRU as part of the mentioned thread. Though it could be a big change worth a separate patch maybe.
- signed to unsigned change in SLRU page numbering is not needed as maximum SLRU page number is guaranteed to be much more than 2 times less than maximum 64-bit XID.
- change of offset from int format to the wider one is not needed at all as multiple of SLRU_PAGES_PER_SEGMENT
and CLOG_XACTS_PER_PAGE (and similar for commit_ts and mxact) is far less
than 2^32 [2]
and CLOG_XACTS_PER_PAGE (and similar for commit_ts and mxact) is far less
than 2^32 [2]
So the change to printing offset as signed, from this thread, is not going to be included into SLRU 64-bit thread [1].
It's true that offset can not be negative, but printing int value as %u isn't nice even if it is not supposed to be negative. So I'd propose the small patch in this thread be applied separately if none has anything against it.
On 25.03.22 11:49, Pavel Borisov wrote: > It's true that offset can not be negative, but printing int value as %u > isn't nice even if it is not supposed to be negative. So I'd propose the > small patch in this thread be applied separately if none has anything > against it. committed