At Wed, 3 Feb 2021 01:26:41 +0000, "tsunakawa.ta...@fujitsu.com" <tsunakawa.ta...@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