From: Iwata, Aya/岩田 彩 <iwata....@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 to the 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 think it's better to put the comment
before if.
Regards
Takayuki Tsunakawa
