Hello. As I read gw/msg.c I found strange code which looks like:
> Msg *msg_create_real(enum msg_type type, const char *file, long line,
> const char *func)
> {
> Msg *msg;
>
> msg = gw_malloc_trace(sizeof(Msg), file, line, func);
>
> msg->type = type;
> #define INTEGER(name) p->name = MSG_PARAM_UNDEFINED
> #define OCTSTR(name) p->name = NULL
> #define UUID(name) uuid_generate(p->name)
> #define VOID(name) p->name = NULL
> #define MSG(type, stmt) { struct type *p = &msg->type; stmt }
> #include "msg-decl.h"
>
> return msg;
> }
Look attentively to MSG() definition, which means that for any type I requested
msg_create() will run code for all types. There is no switch{} like in
msg_pack/msg_unpack, no if{} like in msg_dump(). Similar "universality" I
observe in msg_destroy() and msg_duplicate() which I think is sorta dangerous
for code like this to be executed in wild. This we call russian roulette.
Bad&dangerous code in my opinion:
> Msg *msg_duplicate(Msg *msg)
> {
> Msg *new;
>
> new = msg_create(msg->type);
>
> #define INTEGER(name) p->name = q->name
> #define OCTSTR(name) \
> if (q->name == NULL) p->name = NULL; \
> else p->name = octstr_duplicate(q->name);
> #define UUID(name) uuid_copy(p->name, q->name)
> #define VOID(name) p->name = q->name
> #define MSG(type, stmt) { \
> struct type *p = &new->type; \
> struct type *q = &msg->type; \
> stmt }
> #include "msg-decl.h"
>
> return new;
> }
>
> void msg_destroy(Msg *msg)
> {
> if (msg == NULL)
> return;
>
> #define INTEGER(name) p->name = 0
> #define OCTSTR(name) octstr_destroy(p->name)
> #define UUID(name) uuid_clear(p->name)
> #define VOID(name)
> #define MSG(type, stmt) { struct type *p = &msg->type; stmt }
> #include "msg-decl.h"
>
> gw_free(msg);
> }
Good&sane code in my opinion (to compare):
> excerpt from msg_dump()
>
> #define MSG(tt, stmt) \
> if (tt == msg->type) \
> { char *t = #tt; struct tt *p = &msg->tt; stmt }
> excerpt from msg_pack()
>
> #define MSG(type, stmt) \
> case type: { struct type *p = &msg->type; stmt } break;
>
> switch (msg->type) {
> #include "msg-decl.h"
> default:
> panic(0, "Internal error: unknown message type %d",
> msg->type);
> }
Btw,
