>>>>> "David" == David Fetter <da...@fetter.org> writes:
David> +static inline uint32
David> +decimalLength64(const uint64_t v)
Should be uint64, not uint64_t.
Also return an int, not a uint32.
For int vs. int32, my own inclination is to use "int" where the value is
just a (smallish) number, especially one that will be used as an index
or loop count, and use "int32" when it actually matters that it's 32
bits rather than some other size. Other opinions may differ.
David> +{
David> + uint32 t;
David> + static uint64_t PowersOfTen[] = {
uint64 not uint64_t here too.
David> +int32
David> +pg_ltoa_n(uint32 value, char *a)
If this is going to handle only unsigned values, it should probably be
named pg_ultoa_n.
David> + uint32 i = 0, adjust = 0;
"adjust" is not assigned anywhere else. Presumably that's from previous
handling of negative numbers?
David> + memcpy(a, "0", 1);
*a = '0'; would suffice.
David> + i += adjust;
Superfluous?
David> + uint32_t uvalue = (uint32_t)value;
uint32 not uint32_t.
David> + int32 len;
See above re. int vs. int32.
David> + uvalue = (uint32_t)0 - (uint32_t)value;
Should be uint32 not uint32_t again.
For anyone wondering, I suggested this to David in place of the ugly
special casing of INT32_MIN. This method avoids the UB of doing (-value)
where value==INT32_MIN, and is nevertheless required to produce the
correct result:
1. If value < 0, then ((uint32)value) is (value + UINT32_MAX + 1)
2. (uint32)0 - (uint32)value
becomes (UINT32_MAX+1)-(value+UINT32_MAX+1)
which is (-value) as required
David> +int32
David> +pg_lltoa_n(uint64_t value, char *a)
Again, if this is doing unsigned, then it should be named pg_ulltoa_n
David> + if (value == PG_INT32_MIN)
This being inconsistent with the others is not nice.
--
Andrew (irc:RhodiumToad)
