On Wed, Jun 16, 2021 at 2:54 PM Thomas Munro <thomas.mu...@gmail.com> wrote:
> Hi Zhihong, > > On Thu, Jun 17, 2021 at 8:13 AM Zhihong Yu <z...@yugabyte.com> wrote: > > In 0001-Add-bsearch-and-unique-templates-to-sort_template.h.patch : > > > > - const ST_ELEMENT_TYPE * > ST_SORT_PROTO_ARG); > > + const ST_ELEMENT_TYPE > *ST_SORT_PROTO_ARG); > > > > It seems there is no real change in the line above. Better keep the > original formation. > > Hmm, well it was only recently damaged by commit def5b065, and that's > because I'd forgotten to put ST_ELEMENT_TYPE into typedefs.list, and I > was correcting that in this patch. (That file is used by > pg_bsd_indent to decide if an identifier is a type or a variable, > which affects whether '*' is formatted like a unary operator/type > syntax or a binary operator.) > > > * - ST_COMPARE_ARG_TYPE - type of extra argument > > * > > + * To say that the comparator returns a type other than int, use: > > + * > > + * - ST_COMPARE_TYPE - an integer type > > > > Since the ST_COMPARE_TYPE is meant to designate the type of the return > value, maybe ST_COMPARE_RET_TYPE would be better name. > > It also goes with ST_COMPARE_ARG_TYPE preceding this. > > Good idea, will do. > > > - ST_POINTER_TYPE *a = (ST_POINTER_TYPE *) data, > > - *pa, > > - *pb, > > - *pc, > > - *pd, > > - *pl, > > - *pm, > > - *pn; > > + ST_POINTER_TYPE *a = (ST_POINTER_TYPE *) data; > > + ST_POINTER_TYPE *pa; > > > > There doesn't seem to be material change for the above hunk. > > In master, you can't write #define ST_ELEMENT_TYPE some_type *, which > seems like it would be quite useful. You can use pointers as element > types, but only with a typedef name due to C parsing rules. some_type > **a, *pa, ... declares some_type *pa, but we want some_type **pa. I > don't want to have to introduce extra typedefs. The change fixes that > problem by not using C's squirrelly variable declaration list syntax. > > > + while (left <= right) > > + { > > + size_t mid = (left + right) / 2; > > > > The computation for midpoint should be left + (right-left)/2. > > Right, my way can overflow. Will fix. Thanks! > Hi, Thanks for giving me background on typedefs. The relevant changes look fine to me. Cheers