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

[Anand Babu]

u_int8_t
assemble_ipmi_kcs_rq_pkt (ipmi_kcs_hdr_t *hdr, void *cmd, 
                         u_int8_t cmd_id, u_int8_t *pkt, u_int_t pkt_len)

u_int8_t
unassemble_ipmi_kcs_rs_pkt (u_int8_t *pkt, u_int8_t pkt_len,
                           ipmi_kcs_hdr_t *hdr, void *cmd, u_int8_t cmd_id)


unassemble function can find the real id from pkt at a fixed index, but
the reason it asks for cmd_id is to match with pkt[CMD_INDX] and make
sure we are not copying to a wrong struct of wrong size.
For assemble function, user needs to tell what that void *cmd is
about thru cmd_id.

So this is how it works

u_int8_t
assemble_ipmi_kcs_rq_pkt (ipmi_kcs_hdr_t *hdr, void *cmd, 
                         u_int8_t cmd_id, u_int8_t *pkt, u_int_t pkt_len)
{ 
  pkt[0] = ipmi_netfn2byte (hdr->net_fn)
  switch (cmd_id)
  {
    case IPMI_CMD_GET_DEV_ID:
    { 
      u_int8_t cmd_buf[IPMI_CMD_GET_DEV_ID_RS_LEN];
      marshall_ipmi_get_dev_id_rq ((ipmi_cmd_get_dev_id_rs_t *) cmd,
          cmd_buf, sizeof (cmd_buf);
      memcpy (pkt + IPMI_KCS_HDR_LEN, cmd_buf, sizeof (cmd_buf));
      break;
    }
    case IPMI_CMD_NEXT_CMD:
    ...

unassemble will use a similar switch(cmd_id) approach.

-ab

,----[ Albert Chu <address@hidden> ]
| 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??
| 
| 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 ...
| 
| Let me think about it a bit .. I'm just sort of rambling now ..
`----


-- 
Anand Babu
CaliforniaDigital.com
Office# +1-510-687-7045
Cell# +1-510-396-0717
Home# +1-510-894-0586

Free as in Freedom <www.gnu.org>