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!
