RE: libpq debug log - Mailing list pgsql-hackers

Hi Iwata-san,

In addition to Tsunakawa-san's comments,
The compiler also complains:
  fe-misc.c:678:20: error: ‘lenPos’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
    conn->outMsgStart = lenPos;

There's no need for variable lenPos anymore since we have decided *not* to support pre protocol 3.0.
And by that we have to update the description of pqPutMsgStart too.
So you can remove the lenPos variable and the condition where you have to check for protocol version.

diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index 8bc9966..3de48be 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -644,14 +644,12 @@ pqCheckInBufferSpace(size_t bytes_needed, PGconn *conn)
  *
  * The state variable conn->outMsgStart points to the incomplete message's
  * length word: it is either outCount or outCount+1 depending on whether
- * there is a type byte.  If we are sending a message without length word
- * (pre protocol 3.0 only), then outMsgStart is -1.  The state variable
- * conn->outMsgEnd is the end of the data collected so far.
+ * there is a type byte.  The state variable conn->outMsgEnd is the end of
+ * the data collected so far.
  */
 int
 pqPutMsgStart(char msg_type, bool force_len, PGconn *conn)
 {
-       int                     lenPos;
        int                     endPos;

        /* allow room for message type byte */
@@ -661,9 +659,8 @@ pqPutMsgStart(char msg_type, bool force_len, PGconn *conn)
                endPos = conn->outCount;

        /* do we want a length word? */
-       if (force_len || PG_PROTOCOL_MAJOR(conn->pversion) >= 3)
+       if (force_len)
        {
-               lenPos = endPos;
                /* allow room for message length */
                endPos += 4;
        }
@@ -675,7 +672,7 @@ pqPutMsgStart(char msg_type, bool force_len, PGconn *conn)
        if (msg_type)
                conn->outBuffer[conn->outCount] = msg_type;
        /* set up the message pointers */
-       conn->outMsgStart = lenPos;
+       conn->outMsgStart = endPos;
        conn->outMsgEnd = endPos;



At the same time, the one below lacks one more zero. (Only has 8)
There should be 9 as Matsumura-san mentioned.
+    0, 0, 0, 0, 0, 0, 0, 0,     /* \x65 ... \0x6d */


The following can be removed in pqStoreFrontendMsg():
+ *        In protocol v2, we immediately print each message as we receive it.
+ *        (XXX why?)


Maybe the following description can be paraphrased:
+ *        The message length is fixed after putting the last field, but message
+ *        length should be print before printing any fields.So we must store the
+ *        field data in memory.
to:
+ *        The message length is fixed after putting the last field. But message
+ *        length should be printed before printing any field, so we must store
+ *        the field data in memory.


In pqStoreFrontendMsg, pqLogMessagenchar, pqLogMessageString,
pqLogMessageInt, pqLogMessageByte1, maybe it is unneccessary to use
+    if (PG_PROTOCOL_MAJOR(conn->pversion) >= 3)
because you have already indicated in the PQtrace() to return
silently when protocol 2.0 is detected.
+    /* Protocol 2.0 is not supported. */
+    if (PG_PROTOCOL_MAJOR(conn->pversion) < 3)
+        return;


In pqLogMessageInt(),
+    /* We delayed to print message type for special message. */
can be paraphrased to:
      /* We delay printing of the following special message_type */


Regards,
Kirk Jamison