[Freeipmi-devel] Re: [llnl-devel] checking chksums, possible framework change needed??

[Albert Chu]

> Validity of the comp_code itself depends on the chksum. But location
> of chksum depends on comp_code. There seems to be a cyclic dependency.

I don't really think this is true.  The chcksum is always at a known
location based on the length of the packet read.  In fact, both chksum
values (the one for the response data, and the one for the slave address
& netfn) are both at known locations which can be easily calculated.

> I think it is OK, not to validate the chksum if comp_code is not
> success. In that case we can check comp_code first.

This may be ok from an IPMI standpoint, but I don't think its ok from a
library standpoint.  A user that uses the FreeIPMI library (I would
think) expects a function or macro in which it can pass a buffer and
know that the function can calculate the checksum correctly.  I don't
think we can expect a user to know the order in which to do packet checks.

> We can double-check the chksum and then validate comp_code
> like this

I can't help but find this a little hack-ish.  I think it would be fine
if we hid this within a set of functions and macros.  But I don't think
this is something a user of FreeIPMI should expect.

By the way, I am coming upon a nother issue.  I think it would be good
to have a set of macros such as:

check_net_fn_application_response(ptr)
   ptr->foo->netfn.fn == APPLICATION_NETFN_RESPONSE ? 1 : 0;

or

check_completion_code_normal(ptr)
   ptr->foo->code == NORMAL_COMPLETION ? 1 : 0.

But by doing so, I remove a lot of the modularity you have put into the
library.  These macros require knowledge of more than 1 module
ipmi-msg-support-cmds.h/ipmi-chassis-cmd.h and
ipmi-error.h/ipmi-netfn.h/ipmi-cmd-spec.h.  It will get wierder when/if
we start writing macros to check checksums, b/c we'll need the code fro
ipmi-utils.[ch] too.

I'm thinking perhaps we should put all of the macros into a new file,
and only that one file will load all the .h files it needs?? Or perhaps
we could put the macros all n freeipmi.h??  What do you think??

--
Albert Chu
address@hidden
Lawrence Livermore National Laboratories