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

Re: [XEN PATCH v3] arm/mem_access: add conditional build of mem_access.c



On 2024-05-10 22:59, Julien Grall wrote:
Hi,

On 10/05/2024 13:32, Alessandro Zucchelli wrote:
In order to comply to MISRA C:2012 Rule 8.4 for ARM the following
changes are done:
revert preprocessor conditional changes to xen/mem_access.h which
had it build unconditionally, add conditional build for xen/mem_access.c

I am afraid, I don't understand this one as you don't seem to modify xen/mem_access.h. Is this meant to be part of the changelog?

You also don't seem to mention the change in Makefile. This is the one I was asking for in the previous version. So what about:

"xen/arm: mem_access: Conditionally compile mem_access.c

Commit 634cfc8beb ("Make MEM_ACCESS configurable") intended to make MEM_ACCESS configurable on Arm to reduce the code size when the user doesn't need it.

However, this didn't cover the arch specific code. None of the code in arm/mem_access.c is necessary when MEM_ACCESS=n, so it can be compiled out. This will require to provide some stub for functions called by the common code.

This is also fixing violation of the MISRA C:2012 Rule 8.4 reported by ECLAIR.
"

The patch itself loks good so once we agree on the commit message, then I am happy to update it on commit.

Hi,
Thanks for the feedback,

I am afraid, I don't understand this one as you don't seem to modify xen/mem_access.h. Is this meant to be part of the changelog?

you are right, this should be part of the changelog as it referes to the revert of a previous patch's changes.
I approve of the commit message you provided.

Cheers,

--
Alessandro Zucchelli, B.Sc.

Software Engineer, BUGSENG (https://bugseng.com)



 


Rackspace

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