Re: libpq debug log

From: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
To: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
Cc: iwata(dot)aya(at)fujitsu(dot)com, pgsql-hackers(at)postgresql(dot)org, tgl(at)sss(dot)pgh(dot)pa(dot)us, robertmhaas(at)gmail(dot)com, pchampion(at)pivotal(dot)io, jdoty(at)pivotal(dot)io, raam(dot)soft(at)gmail(dot)com, nagaura(dot)ryohei(at)fujitsu(dot)com, nagata(at)sraoss(dot)co(dot)jp, peter(dot)eisentraut(at)2ndquadrant(dot)com, horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp, k(dot)jamison(at)fujitsu(dot)com
Subject: Re: libpq debug log
Date: 2020-10-09 14:48:59
Message-ID: 20201009144859.GA31144@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2020-10-09 15:08:32 Re: Wrong example in the bloom documentation
Previous Message Ranier Vilela 2020-10-09 14:38:07 Re: Uninitialized var utilized (src/backend/tsearch/spell.c)