[PATCH] score: Fix priority message queue insert

[Chris Johns]

On 23/04/2015 5:51 am, Sebastian Huber wrote:
> 
> ----- Gedare Bloom <gedare at rtems.org> schrieb:
>> Joel,
>>
>> On Tue, Apr 21, 2015 at 12:13 PM, Joel Sherrill
>> <joel.sherrill at oarcorp.com> wrote:
>>>
>>>
>>> On 4/21/2015 2:24 AM, Sebastian Huber wrote:
>>>> Move the linear search into a critical section to avoid corruption due
>>>> to higher priority interrupts.  The interrupt disable time depends now
>>>> on the count of pending messages.
>>>>
>>>> Close #2328.
>>>> ---
>>>>  cpukit/score/include/rtems/score/coremsgimpl.h |  32 +------
>>>>  cpukit/score/src/coremsginsert.c               | 113 +++++++++++--------------
>>>>  2 files changed, 50 insertions(+), 95 deletions(-)
>>
>>>> +#if defined(RTEMS_SCORE_COREMSG_ENABLE_NOTIFICATION)
>>>> +  notify = the_message_queue->number_of_pending_messages == 0;
>>>> +#endif
>>> Add parentheses around expression to make it clear that is the intent.
>> Our coding conventions say to avoid unnecessary parens. If you'd like
>> to make some adjustment to the conventions please start a separate
>> thread.
> 
> Ok, I tried to figure out if a
> 
> x = ( y = 0 );
> 
> triggers a warning which I thought was the reason for Joel to suggest them. It doesn't. So these parentheses are quite useless.  

I see this warning from time to time. It might vary for some weird gcc
reason.

I feel this question of removing unneeded parens should be reviewed. If
someone adds them we should leave them when within reason, i.e. x =
((((y)))); is silly. All coding standards issues should be viewed as
'within reason'. Plus there is a whole grey area when it comes to macros
where in general more is better.

This reduction is against a number of other parts where we encourage
more for clarity such as extra white space, vertical space and on a more
specific example this one:

 const char* p = NULL;
 p = malloc(100);
 if (p == NULL)
  ....

The (p == NULL) is redundant as (!p) and (p) is also acceptable. I do
not mind we code for (p == NULL) or (p != NULL) and I will not argue
otherwise and likewise if someone wishes to explicitly add parens within
reason I also accept that as ok. I will always prefer explicit over
implicit. I have better things to do with my aging brain cells than play
precedent magic in my head. :)

> The testsuite would catch an accidential:
> 
>   notify = the_message_queue->number_of_pending_messages = 0;
> 

I agree with the need to add parens in the '==' case. I would not notice ...

 notify = the_message_queue->number_of_pending_messages = 0;

but ...

 notify = (the_message_queue->number_of_pending_messages = 0);

does not look correct to me and so I would wonder about the missing '='.

Chris