I've finished the review.  Here are the last set of comments.

(16)
(15) is also true for the processing of 'H' message.


(17) pqTraceOutputD
+               for (i = 0; i < nfields; i++)
+               {
+                       len = pqTraceOutputInt32(message + cursor, f);
+                       cursor += 4;
+                       if (len == -1)
+                               break;
+                       pqTraceOutputNchar(message + cursor, f, len);
+                       cursor += len;
+               }

Not break but continue, because non-NULL columns may follow a NULL column.


(18)
+               case 'E':       /* Execute(F) or Error Return(B) */
+                       pqTraceOutputE(message + LogCursor, LogEnd, 
conn->Pfdebug, toServer);
+                       break;

Error Return -> ErrorResponse


(19) pqTraceOutputF
Check the protocol again.  Two for loops should be required, one for format 
codes and another for arguments.


(20)
+       if ( len != -1)

Remove extra space before len.


(21)
I guess you intentionally omitted the following messages because they are only 
used during connection establishment.  I'm fine with it.  I wrote this just in 
case you missed them.

GSSENCRequest (F)
Int32(8)

GSSResponse (F)
Byte1('p')

PasswordMessage (F)
Byte1('p')

SASLInitialResponse (F)
Byte1('p')

SASLResponse (F)
Byte1('p')


(22)
+/* NotificationResponse */
+static void
+pqTraceOutputA(const char *message, int end, FILE *f)
+{
+       int     cursor = 0;
+
+       pqTraceOutputInt16(message + cursor, f);
+       cursor += 2;
+       pqTraceOutputString(message + cursor, f);

Not Int16 but Int32.


Regards
Takayuki Tsunakawa



Reply via email to