On 2020-Oct-09, Kyotaro Horiguchi wrote:
> I saw that version and have some comments.
>
> +pqGetProtocolMsgType(unsigned char c, PGCommSource commsource)
> +{
> + const char *message_type;
>
> Compiler complains that this is unused.
>
> +static const char *
> +pqGetProtocolMsgType(unsigned char c, PGCommSource commsource)
> +{
> ...
> + else
> + return "UnknownCommand";
> +}
>
> Compiler complains as "control reached end of non-void function"
Yeah, those two warnings are caused by the same problem, namely that I
was editing this function to make it simpler and apparently the patch
version I sent does not include all such changes. The fix is to remove
the message_type variable and have the two assignments be "return".
> +pqLogMsgString(PGconn *conn, const char *v, int length, PGCommSource commsource)
> +{
> + if (length < 0)
> + length = strlen(v) + 1;
> +
> pqLogMsgString(conn, str, -1, FROM_*) means actual length may be
> different from the caller thinks, but the pqLogLineBreak() subtracts
> that value from the message length rememberd in in logging_message.
> Anyway AFAICS the patch doesn't use the code path so we should remove
> the first two lines.
True, +1 for removing it.
> By the way, appendBinaryPQExpBuffer() enlarges its buffer by the size
> of the exact length of the given data, but appends '\0' at the end of
> the copied data. Couldn't that leads to an memory overrun?
Doesn't enlargePQExpBuffer() include room for the trailing zero? I
think it does.