gw/msg.c

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,