Nathan Bossart <nathandbossart@gmail.com> writes: > On Thu, Feb 08, 2024 at 11:59:54AM -0800, Andres Freund wrote: >> I'd put these static inlines into common/int.h. I don't think this is common >> enough to warrant being in c.h. Probably also doesn't hurt to have a not quite >> as generic name as INT_CMP, I'd not be too surprised if that's defined in some >> library. >> >> I think it's worth following int.h's pattern of including [s]igned/[u]nsigned >> in the name, an efficient implementation for signed might not be the same as >> for unsigned. And if we use static inlines, we need to do so for correct >> semantics anyway.
> Seems reasonable to me.
+1 here also.
Here is a new version introducing pg_cmp_s32 and friends and use them instead of the INT_CMP macro introduced before. It also moves the definitions to common/int.h and adds that as an include to all locations using these functions.
Note that for integers with sizes less than sizeof(int), C standard conversions will convert the values to "int" before doing the arithmetic, so no casting is *necessary*. I did not force the 16-bit functions to return -1 or 1 and have updated the comment accordingly.
The types "int" and "size_t" are treated as s32 and u32 respectively since that seems to be the case for most of the code, even if strictly not correct (size_t can be an unsigned long int for some architecture).
I also noted that in many situations size_t values are treated as "int" so there is an overflow risk here, even if small. For example, the result of "list_length" is assigned to an integer. I do not think this is an immediate concern, but just wanted to mention it.