[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [XEN PATCH 1/4] xen/arm: address violations of MISRA C:2012 Rule 13.1
On Wed, 18 Oct 2023, Julien Grall wrote: > Hi, > > On 18/10/2023 15:18, Simone Ballarin wrote: > > Rule 13.1: Initializer lists shall not contain persistent side effects > > > > This patch moves expressions with side-effects into new variables before > > the initializer lists. > > Looking at the code, I feel the commit message should be a bit more verbose > because they are no apparent side-effects. > > > > > Function calls do not necessarily have side-effects, in these cases the > > GCC pure or const attributes are added when possible. > > You are only adding pure in this patch. > > > > > No functional changes. > > > > Signed-off-by: Simone Ballarin <simone.ballarin@xxxxxxxxxxx> > > > > --- > > Function attributes pure and const do not need to be added as GCC > > attributes, they can be added using ECLAIR configurations. > > --- > > xen/arch/arm/device.c | 6 +++--- > > xen/arch/arm/guestcopy.c | 12 ++++++++---- > > xen/arch/arm/include/asm/current.h | 2 +- > > 3 files changed, 12 insertions(+), 8 deletions(-) > > > > diff --git a/xen/arch/arm/device.c b/xen/arch/arm/device.c > > index 1f631d3274..e9be078415 100644 > > --- a/xen/arch/arm/device.c > > +++ b/xen/arch/arm/device.c > > @@ -319,6 +319,8 @@ int handle_device(struct domain *d, struct > > dt_device_node *dev, p2m_type_t p2mt, > > int res; > > paddr_t addr, size; > > bool own_device = !dt_device_for_passthrough(dev); > > + bool dev_is_hostbridge = is_pci_passthrough_enabled() && > > + device_get_class(dev) == > > DEVICE_PCI_HOSTBRIDGE; > > The commit message suggests that the code is moved because there are > side-effects. But none of them should have any side-effects. > > In fact, if I am not mistaken, both is_pci_passthrough_enabled() and > device_get_class() could be marked as pure. > > > /* > > * We want to avoid mapping the MMIO in dom0 for the following cases: > > * - The device is owned by dom0 (i.e. it has been flagged for > > @@ -329,9 +331,7 @@ int handle_device(struct domain *d, struct > > dt_device_node *dev, p2m_type_t p2mt, > > struct map_range_data mr_data = { > > .d = d, > > .p2mt = p2mt, > > - .skip_mapping = !own_device || > > - (is_pci_passthrough_enabled() && > > - (device_get_class(dev) == DEVICE_PCI_HOSTBRIDGE)), > > + .skip_mapping = !own_device || dev_is_hostbridge, > > .iomem_ranges = iomem_ranges, > > .irq_ranges = irq_ranges > > }; > > diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c > > index 6716b03561..3ec6743bf6 100644 > > --- a/xen/arch/arm/guestcopy.c > > +++ b/xen/arch/arm/guestcopy.c > > @@ -109,27 +109,31 @@ static unsigned long copy_guest(void *buf, uint64_t > > addr, unsigned int len, > > unsigned long raw_copy_to_guest(void *to, const void *from, unsigned int > > len) > > { > > + struct vcpu *current_vcpu = current; > > It is not clear to me what is the perceived side effect here and the others > below. Can you clarify? I am guessing that's because current is a global variable? But only reading (not writing) a global variable should be OK?
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |