From: Iwata, Aya/岩田 彩 <iwata.aya@fujitsu.com>
> I update the patch.
> I modified code according to review comments of Tsunakawa san and
> Horiguchi san.
I confirmed that all the previous feedback was reflected. Here are some minor comments:
(45)
void PQtrace(PGconn *conn, FILE *stream);
</synopsis>
</para>
+ <para>
+ Calls <function>PQtraceEx</function> to output with or without a timestamp
+ using <literal>flags</literal>.
+ </para>
+
+ <para>
+ <literal>flags</literal> contains flag bits describing the operating mode
+ of tracing. If (<literal>flags</literal> contains <literal>PQTRACE_SUPPRESS_TIMESTAMPS</literal>),
+ then timestamp is not printed with each message.
The description of PQtrace() should be written independent of PQtraceEx(). It is an unnecessary implementation detail
tothe user that PQtrace() calls PQtraceEx() internally. Plus, a separate entry for PQtraceEx() needs to be added.
(46)
If skipLogging is intended for use with backend -> frontend messages only, shouldn't it be placed in conn->b_msg?
(47)
+ /* Deallocate FE/BE message tracking memory. */
+ if (conn->fe_msg &&
+ /*
+ * If fields is allocated the initial size, we reuse it next time,
+ * because it would be allocated same size and the size is not big.
+ */
+ conn->fe_msg->max_fields != DEF_FE_MSGFIELDS)
I'm not completely sure if other places interpose a block comment like this between if/for/while conditions, but I
thinkit's better to put the comment before if.
Regards
Takayuki Tsunakawa