Re: Using defines for protocol characters

On Fri, 4 Aug 2023 at 03:44, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:

> On 2023-Aug-03, Dave Cramer wrote:
>
> > New patch attached which uses PREPARED_SUB_COMMAND and
> > PORTAL_SUB_COMMAND instead
>
> Hmm, I would keep the prefix in this case and make the message type a
> second prefix, with the subtype last -- PQMSG_CLOSE_PREPARED,
> PQMSG_DESCRIBE_PORTAL and so on.
>
I used

#define PQMSG_REQ_PREPARED 'S'
#define PQMSG_REQ_PORTAL 'P'

Duplicating them for CLOSE PREPARED|PORTAL seems awkward

>
> You define PASSWORD and GSS both with 'p', which I think is bogus; in
> some place that isn't doing GSS, you've replaced 'p' with the GSS one
> (CheckSASLAuth). I think it'd be better to give 'p' a single name, and
> not try to distinguish which is PASSWORD and which is GSS, because
> ultimately it's not important.
>
Done

>
> There are some unpatched places, such as basebackup_copy.c -- there are
> several matches for /'.'/ that correspond to protocol chars in that file.
> Also CopySendEndOfRow has one 'd', and desc.c has two 'C'.
>
> I think fe-trace.c will require further adjustment of the comments for
> each function. We could change them to be the symbol for each char, like
> so:
>
> /* PQMSG_RESP_READY_FOR_QUERY */
> static void
> pqTraceOutputZ(FILE *f, const char *message, int *cursor)
>
>
Thanks for reviewing see attached for fixes

Dave