[PATCH] Fixes bitmap allocation accounting logic in rtems-rfs-bitmaps.c

[Chris Johns]

On 12/10/17 11:46 am, Fan Deng wrote:
> Thanks Chris!
> 
> First of all let's make sure a few points are clarified:
> 
> 1) What should rtems_rfs_bitmap_map_set(control, bit) do?
> - 'control' is a handle to the bitmap.
> - 'bit' is the offset of the bit to set in the bitmap. 'bit' should be in the
> range of [0, control->size - 1].
> 
> 2) How does the bitmap store bits?
> By storing bits in an array of rtems_rfs_bitmap_element objects.
> Each rtems_rfs_bitmap_element is 32bit. So the number of elements is
> control->size / 32.
> 
> 3) What is map[index]?
> 'index' is calculated as 'bit / 32' and is the offset into the block buffer of
> the element that contains the 'bit'. So map[index] is the value of the element
> that contains the 'bit'.
> 
> 4) What is 'offset'?
> 'offset' is calculated as 'bit % 32'. It locates the 'bit' in the element that
> contains it.
> 
> 5) What is RTEMS_RFS_BITMAP_ELEMENT_SET
> This is a constant. If map[index] == RTEMS_RFS_BITMAP_ELEMENT_SET, all bits in
> this element (map[index]) are set. It is used to check that condition and
> _update the search map_.

Yes the match is doing something like this. The actual match is made more
complicated because the physical state of a set bit can be controlled.

> Now let's see what your change does:
> 
>  -  map[index] = rtems_rfs_bitmap_set (map[index], 1 << offset);
>  -  if (rtems_rfs_bitmap_match(map[index], RTEMS_RFS_BITMAP_ELEMENT_SET))
>  +  if (!rtems_rfs_bitmap_match(map[index], RTEMS_RFS_BITMAP_ELEMENT_SET))
>     {
> 
> Your change:
> 1. First updates map[index] to set the bit.

I moved the set which is a bug in the code in master in my patch I posed. I did
not paste that bit to the next post, I am sorry about that. Here is the fragment
with the moved set included:

-  map[index] = rtems_rfs_bitmap_set (map[index], 1 << offset);
-  if (rtems_rfs_bitmap_match(map[index], RTEMS_RFS_BITMAP_ELEMENT_SET))
+  if (!rtems_rfs_bitmap_match(map[index], RTEMS_RFS_BITMAP_ELEMENT_SET))
   {
+    map[index] = rtems_rfs_bitmap_set (map[index], 1 << offset);

> 2. Compares the updated map[index] with RTEMS_RFS_BITMAP_ELEMENT_SET. Note this:
>     - *DOES NOT* check whether the original bit was set.

It does because the set was moved.

>     - *DOES NOT* have a way to check if the bit was previously set, as
> map[index] has already been overwritten.
>     - *DOES* check whether all bits in map[index] are set.
> 
> Please let me know if you agree with my assertions in #2 above. Note the name of
> "RTEMS_RFS_BITMAP_ELEMENT_SET" is slightly misleading. See my points #5 for a
> clarification.

As above the set state of a bit can be a physical 0 or 1 depending on the build
configuration of the RFS.

Chris