Horton

Re: libpq debug log

Alvaro Herrera <alvherre@alvh.no-ip.org>

View thread View on postgr.es
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: iwata.aya@fujitsu.com, pgsql-hackers@postgresql.org, tgl@sss.pgh.pa.us, robertmhaas@gmail.com, pchampion@pivotal.io, jdoty@pivotal.io, raam.soft@gmail.com, nagaura.ryohei@fujitsu.com, nagata@sraoss.co.jp, peter.eisentraut@2ndquadrant.com, horiguchi.kyotaro@lab.ntt.co.jp, k.jamison@fujitsu.com
Date: 2020-10-09T14:48:59Z
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.

Commits

  1. Rename PQtraceSetFlags() to PQsetTraceFlags().

  2. Suppress length of Notice/Error msgs in PQtrace regress mode

  3. Strip file names reported in error messages on Windows, too.

  4. Fix setvbuf()-induced crash in libpq_pipeline

  5. libpq_pipeline: Must strdup(optarg) to avoid crash

  6. Remove setvbuf() call from PQtrace()

  7. Initialize conn->Pfdebug to NULL when creating a connection

  8. Disable force_parallel_mode in libpq_pipeline

  9. libpq_pipeline: add PQtrace() support and tests

  10. Improve PQtrace() output format

  11. Re-simplify management of inStart in pqParseInput3's subroutines.