On Sat, Feb 10, 2024 at 9:53 PM Nathan Bossart <nathandboss...@gmail.com> wrote:
> On Sat, Feb 10, 2024 at 08:59:06AM +0100, Mats Kindahl wrote: > > Split the code into two patches: one that just adds the functions > > (including the new pg_cmp_size()) to common/int.h and one that starts > using > > them. I picked the name "pg_cmp_size" rather than "pg_cmp_size_t" since > > "_t" is usually used as a suffix for types. > > > > I added a comment to the (a > b) - (a < b) return and have also added > casts > > to (int32) for the int16 and uint16 functions (we need a signed int for > > uin16 since we need to be able to get a negative number). > > > > Changed the type of two instances that had an implicit cast from size_t > to > > int and used the new pg_,cmp_size() function. > > > > Also fixed the missed replacements in the "contrib" directory. > > Thanks for the new patches. I think the comparison in resowner.c is > backwards, Thanks for catching that. > and I think we should expand on some of the commentary in int.h. > For example, the comment at the top of int.h seems very tailored to the > existing functions and should probably be adjusted. I rewrote the beginning to the following, does that look good? * int.h * Routines to perform signed and unsigned integer arithmetics, including * comparisons, in an overflow-safe way. > And the "comparison > routines for integers" comment might benefit from some additional details > about the purpose and guarantees of the new functions. > I expanded that into the following. WDYT? /*------------------------------------------------------------------------ * Comparison routines for integers. * * These routines are used to implement comparison functions for, e.g., * qsort(). They are designed to be efficient and not risk overflows in * internal computations that could cause strange results, such as INT_MIN > * INT_MAX if you just return "lhs - rhs". *------------------------------------------------------------------------ Best wishes, Mats Kindahl > -- > Nathan Bossart > Amazon Web Services: https://aws.amazon.com >
