Re: [llnl-devel] Re: [Freeipmi-devel] kcs byte array model - validation

[Mark A. Grondona]

> > Mark Grondona made an excellent suggestion of hiding the marshalling
> > call behind. I meets both of our goals.
> 
> This is what I considered in the beginning, but I felt it would be
> a little messier when we do unassemble.  The unassemble function would
> be something like this:
> 
> unassemble_pkt(ipmi_hdr_t *hdr, void *cmd, void*buf, int buflen) {
>   
>    unmarshall_hdr(hdr, buf, buflen);
>    buf += HDR_LEN;  /* whatever, you get the idea */
>    
>    if (buf[0] == IPMI_FOO_PKT_TYPE)
>       struct foo_type *foo = (foo_type *)cmd;
>       foo->val1 = buf[0];
>       foo->val2 = buf[1] & MY_SPECIAL_MASK;
>       foo->val3 = buf[1] & MY_SPECIAL_MASK2; 
>    }
> }
> 
> The reason I don't like this is because what should we do cmd is a
> pointer to type FOO, and buf[0] is packet type BAR??  
> Do we pass in "type" id that tells us what kind of pointer "cmd" is??
> Do we write an unassemble routine for each command type??  Do we ignore
> the packet if it isn't the type we're expecting?? 

Yeah, I did mean to say that receiving messages is more difficult 
than sending data. In fact, I'd say that sending network protocol
data is near trivial compared to receiving.

Something I am experimenting with now is an interface that looks
similar to:


 struct foo_msghdr {
    foo_msg_type_t msg_type;
    void *data;
 };

 ...

 struct foo_msghdr hdr[1];
  
 foo_recvmsg (foo_connection_t server, hdr);

 switch (hdr->msg_type) {
    case FOO_MSG_RESPONSE:
       process_foo_msg_resp ((foo_msg_resp_t *)hdr->data);
    break;
 ...

 foo_free_msg_data (hdr);
 
 
In this implementation, the hdr is filled in with the type of message
most recently received and data contains the message which has already
been unmarshalled from a network buffer. The message data is freed with
foo_free_msg (), which contains no reference to how the memory was
allocated, so that this implementation detail can change without 
breaking the API.

foo_msg_type_t can also contain other message events such as
FOO_MSG_ERROR, FOO_MSG_EOF, etc for indicating non-message events
over the connection `c'.

Of course the above implementation may be too functionally terse for
your project, but I'd thought I'd share.

mark
 
> I just thought of this, maybe it would be better to malloc() within
> the unassemble function, and return a pointer to a buffer containing
> the unassembled/unmarshalled packet??
> 
> if (buf[0] == IPMI_FOO_PKT_TYPE)
>    struct foo_type *foo = malloc(sizeof(struct foo_type));
>    foo->val1 = buf[0];
>    foo->val2 = buf[1] & MY_SPECIAL_MASK;
>    foo->val3 = buf[1] & MY_SPECIAL_MASK2; 
>    return foo;
> }
> 
> I dunno, we're getting into some funny stuff now ... 
> 
> At the same time, there is nothing that says we *have* to have the same
> style with our assemble and unassemble functions.  We can have them
> different ...