[RTEMS Project] #1747: Heap extend allows discontinuous memory regions.

[RTEMS trac]

#1747: Heap extend allows discontinuous memory regions.
--------------------+------------------------------
 Reporter:  chrisj  |       Owner:  sebastian.huber
     Type:  defect  |      Status:  new
 Priority:  normal  |   Milestone:  4.11
Component:  cpukit  |     Version:  4.11
 Severity:  normal  |  Resolution:
 Keywords:          |
--------------------+------------------------------

Old description:

> The check in (cpukit/ChangeLog) states:
>
>  2010-06-07      Sebastian Huber <sebastian.huber at embedded-brains.de>
>
>      * score/src/heapextend.c: Implemented support for scattered heap
> areas.
>
> The heap cannot support scattered blocks because the
> _Heap_Is_block_in_heap assumes the region is continuous between the first
> and last blocks of the heap. Making the gaps in the regions passed to the
> heap extend call used is questionable and makes the
> _Heap_Is_block_in_heap test not really perform the task it's name states.
> This is an issue because it is this check that determines if a heap free
> of NULL should proceed. This issue is covered in another PR.
>
> I also wonder about a heap free call to an address that maps to one of
> the "in-use" gap regions. The previous heap code knew if an address was
> in the heap and therefore it was kind of safe to probe for a valid block.
> This assumption is now not valid.
>
> The former heap extend code:
>
> http://www.rtems.org/viewvc/rtems/cpukit/score/src/heapextend.c?revision=1.7&view=markup
>
> clearly states the type of memory that can be added to an existing heap.
> The current code has no restrictions. The user manual is not great in
> this area. It would also be useful if comments are added to the heap
> extend code.
>
> The heap extend code is used by the rtems_region_extend call and this
> call clearly states in the manual that the memory region must be
> continuous. If this has changed  we should discuss the API change and
> make better note of it. I also suspect the testsuite will need additions
> to test any API changes.

New description:

 The check in (cpukit/ChangeLog) states:

  2010-06-07      Sebastian Huber <sebastian.huber at embedded-brains.de>

      * score/src/heapextend.c: Implemented support for scattered heap
 areas.

 The heap cannot support scattered blocks because the
 _Heap_Is_block_in_heap assumes the region is continuous between the first
 and last blocks of the heap. Making the gaps in the regions passed to the
 heap extend call used is questionable and makes the _Heap_Is_block_in_heap
 test not really perform the task it's name states. This is an issue
 because it is this check that determines if a heap free of NULL should
 proceed. This issue is covered in another PR.

 I also wonder about a heap free call to an address that maps to one of the
 "in-use" gap regions. The previous heap code knew if an address was in the
 heap and therefore it was kind of safe to probe for a valid block. This
 assumption is now not valid.

 The former heap extend code:

 http://www.rtems.org/viewvc/rtems/cpukit/score/src/heapextend.c?revision=1.7&view=markup

 clearly states the type of memory that can be added to an existing heap.
 The current code has no restrictions. The user manual is not great in this
 area. It would also be useful if comments are added to the heap extend
 code.

 The heap extend code is used by the rtems_region_extend call and this call
 clearly states in the manual that the memory region must be continuous. If
 this has changed  we should discuss the API change and make better note of
 it. I also suspect the testsuite will need additions to test any API
 changes.

--

Comment (by sebastian.huber):

 Where in the documentation should this heap extension stuff be mentioned?

--
Ticket URL: <http://devel.rtems.org/ticket/1747#comment:3>
RTEMS Project <http://www.rtems.org/>
RTEMS Project