Xenia Ragiadakou (3):
  xen/cpu: Fix MISRA C 2012 Rule 20.7 violation
  xen/cpu: Add missing white space around arithmetic operators
  xen/cpu: Undefine MASK_DECLARE_ macros after their usage

 xen/common/cpu.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

-- 
2.34.1

Hi Xenia,

> On 5 Aug 2022, at 13:44, Xenia Ragiadakou <burzalodowa@gmail.com> wrote:
> 
> Xenia Ragiadakou (3):
>  xen/cpu: Fix MISRA C 2012 Rule 20.7 violation
>  xen/cpu: Add missing white space around arithmetic operators
>  xen/cpu: Undefine MASK_DECLARE_ macros after their usage
> 
> xen/common/cpu.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
> 

Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

But I agree with Jan that some patches could have been
squashed but I do not think we need to have a v3 here.

Cheers
Bertrand

> -- 
> 2.34.1
> 
> 

On 05.08.2022 14:44, Xenia Ragiadakou wrote:
> Xenia Ragiadakou (3):
>   xen/cpu: Fix MISRA C 2012 Rule 20.7 violation
>   xen/cpu: Add missing white space around arithmetic operators
>   xen/cpu: Undefine MASK_DECLARE_ macros after their usage
> 
>  xen/common/cpu.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 

Acked-by: Jan Beulich <jbeulich@suse.com>

However,
- I don't see why patches 1 and 2 needed splitting, when patch 1 already
  touches all those lines. It is the usual thing for us to make cosmetic
  adjustments when touching a line anyway.
- Patch 3, while fine to be separate, wants a Requested-by: or
  Suggested-by: me (which I guess can be taken care of while committing).

Jan

Hi Jan,

On 8/5/22 15:50, Jan Beulich wrote:
> On 05.08.2022 14:44, Xenia Ragiadakou wrote:
>> Xenia Ragiadakou (3):
>>    xen/cpu: Fix MISRA C 2012 Rule 20.7 violation
>>    xen/cpu: Add missing white space around arithmetic operators
>>    xen/cpu: Undefine MASK_DECLARE_ macros after their usage
>>
>>   xen/common/cpu.c | 15 ++++++++++-----
>>   1 file changed, 10 insertions(+), 5 deletions(-)
>>
> 
> Acked-by: Jan Beulich <jbeulich@suse.com>
> 
> However,
> - I don't see why patches 1 and 2 needed splitting, when patch 1 already
>    touches all those lines. It is the usual thing for us to make cosmetic
>    adjustments when touching a line anyway.

In my opinion, the initial patch that added the code should not have 
been accepted in first place without the white spaces around '+'.
But maybe coding style rules came later.
Nevertheless, I continue to consider it unfair to rely on and request 
from new unrelated patches to fix those issues.

> - Patch 3, while fine to be separate, wants a Requested-by: or
>    Suggested-by: me (which I guess can be taken care of while committing).
> 
> Jan

-- 
Xenia

On 05.08.2022 15:05, Xenia Ragiadakou wrote:
> On 8/5/22 15:50, Jan Beulich wrote:
>> On 05.08.2022 14:44, Xenia Ragiadakou wrote:
>>> Xenia Ragiadakou (3):
>>>    xen/cpu: Fix MISRA C 2012 Rule 20.7 violation
>>>    xen/cpu: Add missing white space around arithmetic operators
>>>    xen/cpu: Undefine MASK_DECLARE_ macros after their usage
>>>
>>>   xen/common/cpu.c | 15 ++++++++++-----
>>>   1 file changed, 10 insertions(+), 5 deletions(-)
>>>
>>
>> Acked-by: Jan Beulich <jbeulich@suse.com>
>>
>> However,
>> - I don't see why patches 1 and 2 needed splitting, when patch 1 already
>>    touches all those lines. It is the usual thing for us to make cosmetic
>>    adjustments when touching a line anyway.
> 
> In my opinion, the initial patch that added the code should not have 
> been accepted in first place without the white spaces around '+'.
> But maybe coding style rules came later.

Iirc the goal was to take it unaltered from Linux.

> Nevertheless, I continue to consider it unfair to rely on and request 
> from new unrelated patches to fix those issues.

I can certainly see where you're coming from, but please also try to take
maintainer perspective: By doing trivial and obvious adjustments at the
same time when touching code anyway, there's one less patch to look at
(and later to apply). And please also consider how "git blame" or alike
works: By touching the same line(s) twice in close succession, finding
the "real" change is made needlessly harder. (Putting the cosmetic change
first is generally disliked because it makes backporting harder, in case
such is wanted / needed.)

I'm sorry if you take such requests as unfair - they aren't meant to be.
They're merely meant to make things as easy as possible for everyone
(which may only be on average, yes).

Jan