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

[Ralf Corsepius]

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.

>
> 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.


> 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