RE: libpq debug log
От | Iwata, Aya |
---|---|
Тема | RE: libpq debug log |
Дата | |
Msg-id | 71E660EB361DF14299875B198D4CE5423DF18A38@g01jpexmbkw25 обсуждение исходный текст |
Ответ на | Re: libpq debug log (Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>) |
Ответы |
RE: libpq debug log
|
Список | pgsql-hackers |
Hi Horiguchi-san, Thank you for your reviewing. I updated patch. Please see my attached patch. > +/* protocol message name */ > +static char *command_text_b[] = { > > Couldn't the name be more descriptive? The comment just above doesn't seem > consistent with the variable. The tables are very sparse. I think the > definition could be in more compact form. Thank you. I changed the description more clear. > > + /* y */ 0, > + /* z */ 0 > +}; > +#define COMMAND_BF_MAX (sizeof(command_text_bf) / > +sizeof(*command_text_bf)) > > It seems that at least the trailing 0-elements are not required. Sure. I removed. > + * message_get_command_text: > + * Get Protocol message text from byte1 identifier > + */ > +static char * > +message_get_command_text(unsigned char c, CommunicationDirection > +direction) > .. > +message_nchar(PGconn *conn, const char *v, int length, > +CommunicationDirection direction) > > Also the function names are not very descriptive. Thank you. I fixed function names and added descriptions. > > +message_Int(PGconn *conn, int v, int length, CommunicationDirection > +direct) > > We are not using names mixing CamelCase and undercored there. > > > + if (c >= 0 && c < COMMAND_BF_MAX) > + { > + text = command_text_bf[c]; > + if (text) > + return text; > + } > + > + if (direction == FROM_BACKEND && c >= 0 && c < COMMAND_B_MAX) > + { > + text = command_text_b[c]; > + if (text) > .. > + if (direction == FROM_FRONTEND && c >= 0 && c < COMMAND_F_MAX) > > > This code is assuming that elements of command_text_bf is mutually exclusive > with command_text_b or _bf. That is, the first has an element for 'C', others > don't have an element in the same position. But _bf[C] = "CommandComplete" > and _f[C] = "Close". Is it working correctly? Elements sent from both the backend and the frontend are 'c' and 'd'. There is no same elements in protocol_message_type_b and _bf. The same applies to protocol_message_type_f and _bf too. So I think it is working correctly. > +typedef enum CommunicationDirection > > The type CommunicationDirection is two-member enum which is equivalent to > just a boolean. Is there a reason you define that? > > +typedef enum State > +typedef enum Type > > The name is too generic. > +typedef struct _LoggingMsg > ... > +} LoggingMsg; > > Why the tag name is prefixed with an underscore? > > +typedef struct _Frontend_Entry > > The name doesn't give an idea of its characteristics. Thank you. I fixed. Regards, Aya Iwata
Вложения
В списке pgsql-hackers по дате отправления: