Re: A qsort template

Hi Zhihong,

On Thu, Jun 17, 2021 at 8:13 AM Zhihong Yu <zyu(at)yugabyte(dot)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!