[PATCH 5/9] posix: Convert to inline function

[Ralf Corsepius]

On 07/18/2013 07:48 PM, Rempel, Cynthia wrote:
>> ________________________________________
>> From: rtems-devel-bounces at rtems.org [rtems-devel-bounces at rtems.org] on behalf of Gedare Bloom [gedare at rtems.org]
>> Sent: Thursday, July 18, 2013 9:47 AM
>> To: Ralf Corsepius
>> Cc: RTEMS Devel
>> Subject: Re: [PATCH 5/9] posix: Convert to inline function
>>
>> On Thu, Jul 18, 2013 at 12:43 PM, Ralf Corsepius
>> <ralf.corsepius at rtems.org> wrote:
>>> On 07/17/2013 04:18 PM, Sebastian Huber wrote:
>>>>
>>>> ---
>>>>    cpukit/posix/include/rtems/posix/muteximpl.h |   20 ++++++++++++++++----
>>>>    cpukit/posix/src/mutextranslatereturncode.c  |   22 +---------------------
>>>>    2 files changed, 17 insertions(+), 25 deletions(-)
>>>>
>>>> diff --git a/cpukit/posix/include/rtems/posix/muteximpl.h
>>>> b/cpukit/posix/include/rtems/posix/muteximpl.h
>>>> index d4673aa..29e93c2 100644
>>>> --- a/cpukit/posix/include/rtems/posix/muteximpl.h
>>>> +++ b/cpukit/posix/include/rtems/posix/muteximpl.h
>>>> @@ -38,6 +40,8 @@ POSIX_EXTERN Objects_Information
>>>> _POSIX_Mutex_Information;
>>>>
>>>>    POSIX_EXTERN pthread_mutexattr_t _POSIX_Mutex_Default_attributes;
>>>>
>>>> +extern const int _POSIX_Mutex_Return_codes[CORE_MUTEX_STATUS_LAST + 1];
>>>> +
>>>>    /*
>>>>     *  @brief POSIX Mutex Manager Initialization
>>>>     *
>>>> @@ -144,11 +148,19 @@ int _POSIX_Mutex_Lock_support(
>>>>     * willing to block but the operation was unable to complete within the
>>>> time
>>>>     * allotted because the resource never became available.
>>>>     */
>>>> -
>>>> -int _POSIX_Mutex_Translate_core_mutex_return_code(
>>>> +RTEMS_INLINE_ROUTINE int _POSIX_Mutex_Translate_core_mutex_return_code(
>>>>      CORE_mutex_Status  the_mutex_status
>>>> -);
>>>> -
>>>> +)
>>>> +{
>>>> +  /*
>>>> +   *  Internal consistency check for bad status from SuperCore
>>>> +   */
>>>> +  #if defined(RTEMS_DEBUG)
>>>> +    if ( the_mutex_status > CORE_MUTEX_STATUS_LAST )
>>>> +      return EINVAL;
>>>> +  #endif
>>>> +  return _POSIX_Mutex_Return_codes[the_mutex_status];
>>>> +}
>>>>
>>>>    /*
>>>>     *  _POSIX_Mutex_Get
>>>
>>> I don't like this kind of changes, because they contradict to the working
>>> principles of data abstraction and encapsulation.
>>>
>>> They expose internal implementation details and symbols, no user should see.
>>>
>> I think this file and it's structures/functions are not meant to be
>> part of the user API.
> That's correct...
I can't check ATM and don't know off-head

> muteximpl.h is supposed to be a private header...
> we should probably verify the header doesn't get installed (see patch 1/9), if it does, we need to modify the patch-sets so the private headers don't get installed...
> probably a Makefile.am change...
>>> Many people, comprising me, consider this to be very bad coding style.
> Hmm... is that because it's defining a function in (what is intended to be) a private header?
> Interesting catch... We'll have to try this patch-set out to see if the private headers are being installed...
> Could you give us pointers to a coding style guide?
> We could probably incorporate it into our Coding Style, http://www.rtems.org/wiki/index.php/Coding_Conventions
The problem with global vars is them being global, which allows nasty 
tricks (direct access and casts).

Global arrays are one magnitude more evil (range checking).

The worst feature about both is them being very hard to debug, because 
you can't set breakpoints on them.

One way to work around all this is to systematically make all globals 
static and to access them only though accessors-function. Naturally, 
this comes at a performance penalty, but this is the price for 
code-safety and debug-abilty.

I know, in RTEMS this approach is disliked, because this contradicts the 
"use C as macro-asm" school some people around here prefer.

Ralf