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

[Claus, Ric]

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