[Freeipmi-devel] fiid reimplementation notes

[Albert Chu]

With the exception of one SDR bug that's making ipmi-sensors say "cat
ate teh fish" on a corner case, the core work is complete.  Its in the
branch 'al_fiid_rearchitect_branch'.  

Notes:

1) Good stuff I remember when re-working code to use the new API

assemble/unassemble:

Using the new fiid_obj_set_block(), fiid_obj_get_block(),
fiid_obj_set_all(), and fiid_obj_get_all() functions, there is no more
need to calculate various offsets.  The code (to me) looks a lot cleaner
than it did before.

No more passing around templates:

Reduces data to manage.

Additional parameter checking:

Template information is now stored in the fiid_obj_t, so we now
(finally, after me harping about this for probably 2 years :-)) have the
ability to ensure that an object passed to a function has the correct
length and is the object of the correct type.

LAN Session Header:

The multiple session header templates (tmpl_hdr_session,
tmpl_hdr_session_auth, tmpl_hdr_session_auth_calc) are now just one
(tmpl_hdr_session) b/c of optional field support for the auth_code.

SDR record lengths:

Workarounds are no longer needed to get around the unexpected length of
record types using the Get SDR record command.

SDR record types:

Workarounds are no longer needed to get around the variable length
string sensor/device names at the ends of SDR record types.

I think there were a few more workarounds that were eliminated b/c of
suport for optional/variable length data, but I can't remember them.

2) Not so good stuff I noticed when re-working the code

No more alloca:

If we want to abstract away the underlying data structures, the casting
of fiid_obj_t to uint8_t * has to go away.

Longer code:

Some code ended up a bit longer.  Some longer code was needed b/c of
cleanup logic that was needed.  Parameter checking made things longer,
but it really should have been there from the start.

Reserved fields have to be set:

With optional fields being supported, reserved fields must be set in
fill_* functions.  Otherwise the new fiid interface assumes the data
isn't set, is optional, and thus shouldn't be stuffed in a packet.

3) TODO stuff

I certainly don't know every subsection of the IPMI spec by heart.  I
may  have guessed some fields were required & fixed length, when in fact
they were optional or variable length.  Hopefully anything will come up
during additional testing.

Some code cleanup is needed to remove duplicate code.  Now that
templates  do not need to be passed around to fiid functions, a lot more
duplicate code exists, which can be collapsed into functions.

Some code ended up a lot longer b/c earlier code worked off the
assumption that fiid_obj_t's were char *buffers.  The code logic used
from before didn't fit in with the new fiid API too well, so a lot of
additional code was added to get around it.  Once the code logic is
fixed, the code should be clean and happy again.

I don't have a machine with PEF, so all of my reimplementation needs to
be tested.

4) Maybe todo

I'm still debating the naming of a few of the fiid functions.

Also, I need to rethink the packet templates formats.  The common case
is for fields to be a fixed length and required for transmission.  So it
would be wiser for me to make that the default, and optional or variable
length fields the exception. 

Al

--
Albert Chu
address@hidden
925-422-5311
Computer Scientist
High Performance Systems Division
Lawrence Livermore National Laboratory