[rtems commit] Using the generic atomic ops to implement UP mode atomic for all architectures . SMP atomic port will be later.

[Gedare Bloom]

Momentum. We can consider making this change for our internal headers.
There is a possible project in the future to redo headers to
consolidate some, reduce the number we install on the development
host, and to separate between "public" and "private" headers per
subsystem/API. Perhaps when we get to this project we should consider
renaming some of these longer-named header files.
-Gedare

On Wed, May 1, 2013 at 1:28 PM, Claus, Ric <claus at slac.stanford.edu> wrote:
> Again I would like to beat my little drum about code readability.  Ralf's confusion could have been eliminated if header file names composed of multiple words were joined with camel case or underscores.  What is the reluctance to do so about?
>
> Ric
>
> ________________________________________
> From: rtems-devel-bounces at rtems.org [rtems-devel-bounces at rtems.org] On Behalf Of Gedare Bloom [gedare at rtems.org]
> Sent: Wednesday, May 01, 2013 8:30 AM
> To: Ralf Corsepius
> Cc: rtems-devel at rtems.org
> Subject: Re: [rtems commit] Using the generic atomic ops to implement UP mode atomic for all architectures . SMP atomic port will be later.
>
> On Wed, May 1, 2013 at 11:19 AM, Ralf Corsepius
> <ralf.corsepius at rtems.org> wrote:
>> On 05/01/2013 05:00 PM, Joel Sherrill wrote:
>>>
>>> On 5/1/2013 9:44 AM, Ralf Corsepius wrote:
>>>>
>>>> On 05/01/2013 04:23 PM, Gedare Bloom wrote:
>>>>>
>>>>> Module:    rtems
>>>>> Branch:    master
>>>>> Commit:    9b605b28b497f398682a8ffd673f6860340957f8
>>>>> Changeset:
>>>>> http://git.rtems.org/rtems/commit/?id=9b605b28b497f398682a8ffd673f6860340957f8
>>>>>
>>>>> Author:    WeiY <wei.a.yang at gmail.com>
>>>>> Date:      Wed Apr 24 00:34:16 2013 +0800
>>>>>
>>>>> Using the generic atomic ops to implement UP mode atomic for all
>>>>> architectures. SMP atomic port will be later.
>>>>> +++ b/cpukit/score/cpu/arm/rtems/score/cpuatomic.h
>>>>> @@ -0,0 +1,40 @@
>>>>> +/**
>>>>> + * @file  rtems/score/cpuatomic.h
>>>>> + *
>>>>> + * This include file implements the atomic operations for arm and
>>>>> defines
>>>>> + * atomic data types which are used by the atomic operations API file.
>>>>> This
>>>>> + * file should use fixed name cpuatomic.h and should be included in
>>>>> atomic
>>>>> + * operations API file atomic.h. If the architecture works at the UP
>>>>> mode it
>>>>> + * will use a generic atomic ops using disable/enable-IRQ simulated. If
>>>>> the
>>>>> + * the architecture works at SMP mode, most of the parts of
>>>>> implementations
>>>>> + * are imported from FreeBSD kernel.
>>>>> + */
>>>>> +
>>>>> +#ifndef _RTEMS_SCORE_ATOMIC_CPU_H
>>>>> +#define _RTEMS_SCORE_ATOMIC_CPU_H
>>>>> +
>>>>> +#include <rtems/score/genericcpuatomic.h>
>>>>> +
>>>>> +#ifdef __cplusplus
>>>>> +extern "C" {
>>>>> +#endif
>>>>> +
>>>>> +/**
>>>>> + * @defgroup RTEMS atomic implementation
>>>>> + *
>>>>> + */
>>>>> +
>>>>> +/**@{*/
>>>>> +
>>>>> +#if !defined(RTEMS_SMP)
>>>>> +#include <rtems/score/genericatomicops.h>
>>>>> +#else
>>>>> +#endif
>>>>> +
>>>>> +#ifdef __cplusplus
>>>>> +}
>>>>> +#endif
>>>>> +
>>>>> +/**@}*/
>>>>> +#endif
>>>>> +/*  end of include file */
>>>>
>>>> Would somebody please explain the code above?
>>>>
>>>> The #ifdef __cplusplus as well as the #if !defined(RTEMS_SMP)
>>>> should be completely superflous. If not, they'd indicate bugs elsewhere.
>>>
>>> AFAIK The !defined(RTEMS_SMP) just says use the generic uniprocessor
>>> implementation when not SMP. What's the specific issue?
>>
>> The code includes
>> <rtems/score/genericatomicops.h>
>> twice. Once unconditionally and once conditionally.
>> This is redundant.
>>
> No, it includes genericcpuatomic.h unconditionally, and
> genericatomicops.h conditionally.
>
>>
>>>
>>> And I also don't see what the issue is with the C++ check.
>>
>> The c++ guard is entirely superfluous in this case, because the file does
>> not define any symbol.
>>
> OK. I will have the contributor clean up these guards. I agree with
> Thomas' interpretation (and repeated below by Ralf) about the layering
> of guards, although since we do not permit C++ anywhere inside of
> score then from a technical perspective it should not matter. But we
> should keep our source clean. :)
>
>>
>>
>>> A quick
>>> grep of score shows 122 files or 333 .h files have the C++ guard.
>>> Do you have a rationale for when one should be present or not?
>>
>> The guards are required when files defines/declares C symbols. Files which
>> don't, don't have to.
>>
>> Finally, files should set c++ guards in ways they are self-contained. The
>> construct above guards an included file, which means this guard is either
>> redundant (in case the other file is self-guarded) or the included file
>> isn't self-guarded (which would mean the include file is poorly implemented
>> == broken).
>>
>>
>> Ralf
>>
>> _______________________________________________
>> rtems-devel mailing list
>> rtems-devel at rtems.org
>> http://www.rtems.org/mailman/listinfo/rtems-devel
> _______________________________________________
> rtems-devel mailing list
> rtems-devel at rtems.org
> http://www.rtems.org/mailman/listinfo/rtems-devel