[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [XEN PATCH v3] xen: replace occurrences of SAF-1-safe with asmlinkage attribute



Hi Nicola,

On 16/11/2023 09:15, Nicola Vetrini wrote:
On 2023-11-16 10:08, Nicola Vetrini wrote:
The comment-based justifications for MISRA C:2012 Rule 8.4 are replaced
by the asmlinkage pseudo-attribute, for the sake of uniformity.

Add missing 'xen/compiler.h' #include-s where needed.

The text in docs/misra/deviations.rst and docs/misra/safe.json
is modified to reflect this change.

Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
---
This patch should be applied after patch 2 of this series.
The request made by Julien to update the wording is
contained in the present patch.
https://lore.kernel.org/all/9ad7f6210c15f520297aac00e8af0e64@xxxxxxxxxxx/

Concerns about efi_multiboot2 will be dealt with separately.

Changes in v2:
- Edit safe.json.
- Remove mention of SAF-1-safe in deviations.rst.
Changes in v3:
- Sorted #include-s and rebased against
7ad0c774e474 ("x86/boot: tidy #include-s")
---
 docs/misra/deviations.rst   |  5 ++---
 docs/misra/safe.json        |  2 +-
 xen/arch/arm/cpuerrata.c    |  7 +++----
 xen/arch/arm/setup.c        |  5 ++---
 xen/arch/arm/smpboot.c      |  3 +--
 xen/arch/arm/traps.c        | 21 +++++++--------------
 xen/arch/x86/boot/cmdline.c |  5 +++--
 xen/arch/x86/boot/reloc.c   |  6 +++---
 xen/arch/x86/extable.c      |  3 +--
 xen/arch/x86/setup.c        |  3 +--
 xen/arch/x86/traps.c        | 27 +++++++++------------------
 xen/common/efi/boot.c       |  5 ++---
 12 files changed, 35 insertions(+), 57 deletions(-)


In hindsight I should have added an

Acked-by: Julien Grall <jgrall@xxxxxxxxxx>

given that the comment has been addressed in my opinion.

I am a bit confused how you considered it was addressed. I see no update in safe.json when I clearly asked for some (I wouldn't have bothered to comment in v2 otherwise and just gave an ack).

To be explicit, I requested to:
  1. update the description in [1] to clarify that SAF-1 is deprecated.
2. This patch is rebased on top and therefore remove completely the mention of SAF-1.

I am well-aware that the end result is technically the same. But patches are meant to be self-contained so if we revert the latest, then the meaning is still the same.

This patch is unlikely to be removed and this is now the nth time I asked it the same (maybe it was not clear enough?). So I am going to content with the current proposal because this is not worth to go further. But I will at least express my discontent how this is handled.

TBH, there are far too many MISRA patches on the ML spread across multiple threads. Some are based on top of the others. This makes extremely difficult to follow and know what is addressed or not. Can we at least try to condense some of work in similar area in the same series? For instance, this patch could have been included in the other series [1].

Lastly, right now, I have 300 emails (31 threads) with MISRA in the title in my inbox. It is a little unclear what has been committed/review or require input. I am concerned to miss key series (the patch to compile in docs/ was nearly missed).

Do we track anywhere which series are still inflights? Can we consider to pause or at least slow down the rate of new MISRA patches until the backlog is cleared? (Adding more patches is not really helping).

Cheers,

[1] https://lore.kernel.org/all/a1b5c3b273145c35535fed3647bf850d9ae5db7f.1698829473.git.nicola.vetrini@xxxxxxxxxxx/

I pointed out that the patch in



--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.