[PATCH 2/3] score: Simplify _Objects_Initialize_information()

[Sebastian Huber]

On 07/11/2018 01:11, Chris Johns wrote:
> On 06/11/2018 23:37, Sebastian Huber wrote:
>> There is no need to make the minimum identifier dependent on the maximum
>> per allocation.
>> ---
>>   cpukit/score/src/objectinitializeinformation.c | 4 +---
>>   1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/cpukit/score/src/objectinitializeinformation.c b/cpukit/score/src/objectinitializeinformation.c
>> index 23c7819bfa..9905349213 100644
>> --- a/cpukit/score/src/objectinitializeinformation.c
>> +++ b/cpukit/score/src/objectinitializeinformation.c
>> @@ -39,7 +39,6 @@ void _Objects_Do_initialize_information(
>>   )
>>   {
>>     static Objects_Control *null_local_table = NULL;
>> -  uint32_t                minimum_index;
>>     Objects_Maximum         maximum_per_allocation;
>>   
>>     information->the_api            = the_api;
>> @@ -88,9 +87,8 @@ void _Objects_Do_initialize_information(
>>     /*
>>      *  Calculate minimum and maximum Id's
>>      */
>> -  minimum_index = (maximum_per_allocation == 0) ? 0 : 1;
> I am not sure you can remove this line. Something does not feel right, it was
> long ago I wrote the original change and I seem to remember I thought the same
> things and added this code after lots of testing. It may be this is not needed
> with the way the code initialises managers now, I am not sure. What if someone
> pulls in a manager but does not configure any resources?

I added spconfig02 to test managers with a maximum object count of zero.

This change has no impact on the overall test results. I did also a test 
run with the realview_pbx_a9_qemu BSP since it catches NULL pointer 
accesses.

A potential use case of this

minimum_index = (maximum_per_allocation == 0) ? 0 : 1;

could be in:

Objects_Control *_Objects_Get(
   Objects_Id                 id,
   ISR_lock_Context          *lock_context,
   const Objects_Information *information
)
{
   uint32_t index;

   index = id - information->minimum_id + 1;

   if ( information->maximum >= index ) {
     Objects_Control *the_object;

     _ISR_lock_ISR_disable( lock_context );

     the_object = information->local_table[ index ];
     if ( the_object != NULL ) {
       /* ISR disabled on behalf of caller */
       return the_object;
     }

     _ISR_lock_ISR_enable( lock_context );
   }

   return NULL;
}

With a minimum index of 0 you don't end up in the information->maximum >= index case for all indexes 0..65535. This would prevent a NULL pointer access if you do this before the corresponding _Objects_Initialize_information(). No matter how you set minimum_id you can always construct an invalid identifier to end up in this case.

I would like to statically initialize the object information structures in a follow up patch, so this would be no longer an issue.

-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber at embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.