Iwata-san,
(25)
+ conn->fe_msg = malloc(sizeof(offsetof(pqFrontendMessage, fields)) +
+ DEF_FE_MSGFIELDS * sizeof(pqFrontendMessageField));
It's incorrect that offsetof() is nested in sizeof(). See my original review comment.
(26)
+bool
+pqTraceInit(PGconn *conn, bits32 flags)
+{
...
+ conn->be_msg->state = LOG_FIRST_BYTE;
+ conn->be_msg->length = 0;
+ }
...
+ conn->be_msg->state = LOG_FIRST_BYTE;
+ conn->be_msg->length = 0;
The former is redundant?
(27)
+/*
+ * Deallocate frontend-message tracking memory. We only do this because
+ * it can grow arbitrarily large, and skip it in the initial state, because
+ * it's likely pointless.
+ */
+void
+pqTraceUninit(PGconn *conn)
+{
+ if (conn->fe_msg &&
+ conn->fe_msg->num_fields != DEF_FE_MSGFIELDS)
+ {
+ pfree(conn->fe_msg);
+ conn->fe_msg = NULL;
+ }
+}
I thought this function plays the reverse role of pqTraceInit(), but it only frees memory for frontend messages. Plus,
usefree() instead of pfree(), because malloc() is used to allocate memory.
(28)
+void PQtrace(PGconn *conn, FILE *stream, bits32 flags);
bits32 can't be used because it's a data type defined in c.h, which should not be exposed to applications. I think you
canjust int or something.
Considering these and the compilation error Kirk-san found, I'd like you to do more self-review before I resume this
review.
Regards
Takayuki Tsunakawa
