Re: [Freeipmi-devel] fiid_obj_get bugs

[Albert Chu]

Hi Bala,

> I applied your patch and tested it.  It fixes for some cases and 
> not for other cases.

What did you find that didn't work?  I did some sanity/test checking and
didn't find anything.  Of course I may have missed stuff.  Just point me
in the right direction and I'll fix the issues.

Al

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


----- Original Message -----
From: "Bala.A" <address@hidden>
Date: Monday, January 16, 2006 4:52 pm
Subject: Re: [Freeipmi-devel] fiid_obj_get bugs

> Hi Al,
> 
> > I found several bugs in fiid_obj_get() while hunting down the 
> cause for
> > why fiid_obj_get() didn't function properly for a field of length 
> 64.  I
> > believe several corner cases were never hit b/c fiid_obj_get() 
> wasn't> used for anything >= 64 bits.
> 
> Excellent catch and great job.
> 
> > Since this is a pretty critical area for fiid, I wanted to post 
> to get
> > comments before I commit.  Here are the bugs I found:
> >
> > 1) If the field was > 64 bits (i.e. > 8 bytes), the function would
> > return invalid values.  Due to invalid bit shifting (i.e. bit 
> shift 72
> > bits to the right), zeroes would get populated into the 'val' return
> > value.  The fix is to of course limit fiid_obj_get to fields <= 
> 64 bits.
> >
> > 2) The main loop was typically iterated 1 extra time.  If the field
> > length was greater than 56 bits (i.e. > 7 bytes), the loop would 
> iterate> 9 times rather than 8.  The additional iteration would 
> populate in a 0
> > into the last byte of the 'val' return value, destroying some data.
> >
> > 3) The end bit position was computed improperly on corner cases.  It
> > seems the common situation was when a field was a multiple of 8, 
> so the
> > "% 8" computation earlier in the code messed things up.  It seems 
> that> this didn't actually cause any issues due to the additional loop
> > iteration up.
> >
> > Here are my changes.
> >
> 
> I applied your patch and tested it.  It fixes for some cases and 
> not for
> other cases.  Could you please fix them?  I completely forget the 
> logic of
> fiid_obj_get().
> 
> Thanks,
> 
> Bala
> 
> 
>