[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Freeipmi-devel] fiid_obj_get bugs
From: |
Albert Chu |
Subject: |
Re: [Freeipmi-devel] fiid_obj_get bugs |
Date: |
Mon, 16 Jan 2006 17:01:57 -0800 |
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
>
>
>