Re: libpq debug log

At Wed, 3 Feb 2021 01:26:41 +0000, "tsunakawa.takay@fujitsu.com" <tsunakawa.takay@fujitsu.com> wrote in 
> (41)
> +void
> +PQtraceEx(PGconn *conn, FILE *debug_port, int flags)
> +{
> +    if (conn == NULL)
> +        return;
> ...
> +    if (!debug_port)
> +        return;
> 
> The if should better be as follows to match the style of existing surrounding code.
> 
> +    if (debug_port == NULL)

I don't have particular preference here, but FWIW the current
PQuntrace is doing this:

> void
> PQuntrace(PGconn *conn)
> {
>     if (conn == NULL)
>         return;
>     if (conn->Pfdebug)
>     {
>         fflush(conn->Pfdebug);

> (44)
> +    if ((conn->traceFlags & PQTRACE_OUTPUT_TIMESTAMPS) == 0)
> +        timestr_p = pqLogFormatTimestamp(timestr, sizeof(timestr));
> 
> == 0 should be removed.

Looking the doc mentioned in the comment #39:

+      <literal>flags</literal> contains flag bits describing the operating mode
+      of tracing.  If (<literal>flags</literal> & <literal>PQTRACE_OUTPUT_TIMESTAMPS</literal>) is
+      true, then timestamp is not printed with each message.

PQTRACE_OUTPUT_TIMESTAMPS is designed to *inhibit* timestamps from
being prepended. If that is actually intended, the symbol name should
be PQTRACE_NOOUTPUT_TIMESTAMP. Otherwise, the doc need to be fixed.

By the way removing "== 0" makes it difficult to tell whether the
condition is correct or not; I recommend to use '!= 0" rather than
removing '== 0'.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center