On Tue, Oct 27, 2020 at 3:23 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> I've been hacking at this patch again. There were a few things I wasn't
> too clear about, so I reordered the code and renamed the routines to try
> to make it easier to follow.
>
Hi,
Hopefully Iwata-san will return to looking at this soon.
I have tried the latest patch a little (using "psql" as my client),
and have a few small comments so far:
- In pqTraceInit(), in the (admittedly rare) case that fe_msg malloc()
fails, I'd NULL out be_msg too after free(), rather than leave it
dangling (because if pgTraceInit() was ever invoked again, as the
comment says it could, it would result in previously freed memory
being accessed ...)
conn->fe_msg = malloc(MAX_FRONTEND_MSGS * sizeof(pqFrontendMessage));
if (conn->fe_msg == NULL)
{
free(conn->be_msg);
conn->be_msg = NULL;
return false;
}
- >3. COPY ... (FORMAT BINARY) emits "invalid protocol" ... not good.
That seemed to happen for me only if COPYing binary format to stdout.
UnknownCommand :::Invalid Protocol
- >5. Error messages are still printing the terminating zero byte. I
>suggest that it should be suppressed.
Perhaps there's a more elegant way of doing it, but I got rid of the
logging of the zero byte using the following change to
pgLogMsgByte1(), though there still seems to be a trailing space
issue:
case LOG_CONTENTS:
- fprintf(conn->Pfdebug, "%c ", v);
+ if (v != '\0')
+ fprintf(conn->Pfdebug, "%c ", v);
pqTraceMaybeBreakLine(sizeof(v), conn);
break;
Regards,
Greg Nancarrow
Fujitsu Australia