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

Reply via email to