Re: [Xen-devel] [PATCH v4 1/7] asm-arm/atomic.h: fix arm32|arm64 macros duplication

[Corneliu ZUZU <czuzu@xxxxxxxxxxxxxxx>]

On 7/15/2016 12:28 PM, Julien Grall wrote:
> Hi Corneliu,
>
> On 15/07/16 07:48, Corneliu ZUZU wrote:
> > Move duplicate macros between asm-arm/arm32/atomic.h and 
> > asm-arm/arm64/atomic.h
> > to asm-arm/atomic.h. Adjust README.LinuxPrimitives in the process.
> > Also empty line fixes.
> Why do you add empty lines?
A little picky today, aren't we? :-)

I added empty lines:
- before local variables block
- between header comment and #ifndef guard
because IMO -it looks better- and is consistent with the format of -most 
of- the rest of the files (thus I assumed that's the preferred format 
and that deviations from that are honest mistakes :-)).
> They are not necessary nor coding style requirement nor in Linux 
> headers. Please don't introduce changes without a valid reason.
I just peeked in the Linux source tree and I noticed there are also 
headers there with an empty line between the file-comment and #ifndef.
Plus, this is the Xen code-base and I don't see why I'd look in the 
Linux source tree to determine rules that apply to the Xen source-tree.
I didn't expect for a rule like this to -need- putting in CODING_STYLE. 
Anyway, they'll be dropped in v5 if it really matters.
> > Signed-off-by: Corneliu ZUZU <czuzu@xxxxxxxxxxxxxxx>
> > Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > ---
> > Changed since v3:
> >    * update README.LinuxPrimitives file
> > ---
> >   xen/arch/arm/README.LinuxPrimitives | 20 ++++++++++++++++++++
> >   xen/include/asm-arm/arm32/atomic.h  | 15 ++-------------
> >   xen/include/asm-arm/arm64/atomic.h  | 15 ++-------------
> >   xen/include/asm-arm/atomic.h        | 14 ++++++++++++++
> >   4 files changed, 38 insertions(+), 26 deletions(-)
> >
> > diff --git a/xen/arch/arm/README.LinuxPrimitives 
> > b/xen/arch/arm/README.LinuxPrimitives
> > index 3115f51..4906593 100644
> > --- a/xen/arch/arm/README.LinuxPrimitives
> > +++ b/xen/arch/arm/README.LinuxPrimitives
> > @@ -23,6 +23,16 @@ atomics: last sync @ v3.16-rc6 (last commit: 
> > 8715466b6027)
> >   linux/arch/arm64/include/asm/atomic.h 
> > xen/include/asm-arm/arm64/atomic.h
> > +The following functions were taken from Linux:
> > +    atomic_add(), atomic_add_return(), atomic_sub(), 
> > atomic_sub_return(),
> > +    atomic_cmpxchg(), __atomic_add_unless()
> > +
> > +Also, the following macros which were in the meantime moved to 
> > asm-arm/atomic.h:
> > +    atomic_xchg(v, new), atomic_inc(v), atomic_dec(v),
> > +    atomic_inc_and_test(v), atomic_dec_and_test(v),
> > +    atomic_inc_return(v), atomic_dec_return(v),
> > +    atomic_sub_and_test(i, v), atomic_add_negative(i,v)
> I would drop the second paragraph because they are moved outside of 
> arm/*atomic.h and you turn into inline. So IHMO they are not sync-able 
> anymore.
Ok.

> > +
> > ---------------------------------------------------------------------
> >
> >   mem*: last sync @ v3.16-rc6 (last commit: d875c9b37240)
> > @@ -91,6 +101,16 @@ atomics: last sync @ v3.16-rc6 (last commit: 
> > 030d0178bdbd)
> >   linux/arch/arm/include/asm/atomic.h xen/include/asm-arm/arm32/atomic.h
> >
> > +The following functions were taken from Linux:
> > +    atomic_add(), atomic_add_return(), atomic_sub(), 
> > atomic_sub_return(),
> > +    atomic_cmpxchg(), __atomic_add_unless()
> > +
> > +Also, the following macros which were in the meantime moved to 
> > asm-arm/atomic.h:
> > +    atomic_xchg(v, new), atomic_inc(v), atomic_dec(v),
> > +    atomic_inc_and_test(v), atomic_dec_and_test(v),
> > +    atomic_inc_return(v), atomic_dec_return(v),
> > +    atomic_sub_and_test(i, v), atomic_add_negative(i,v)
> Ditto.
>
> Regards,
Ack.

Corneliu.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel