Re: [XEN PATCH 4/4] xen: address violations of MISRA C:2012 Rule 13.1

From: Simone Ballarin <simone.ballarin@bugseng.com>
To: Jan Beulich <jbeulich@suse.com>,
	Stefano Stabellini <sstabellini@kernel.org>
Cc: xen-devel@lists.xenproject.org, consulting@bugseng.com,
	George Dunlap <george.dunlap@citrix.com>,
	Dario Faggioli <dfaggioli@suse.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Julien Grall <julien@xen.org>, Wei Liu <wl@xen.org>
Subject: Re: [XEN PATCH 4/4] xen: address violations of MISRA C:2012 Rule 13.1
Date: Thu, 19 Oct 2023 13:12:30 +0200	[thread overview]
Message-ID: <27fcfc87-aa8f-4bfb-863a-91a75c763050@bugseng.com> (raw)
In-Reply-To: <78a7b73a-fefb-6884-90c5-abad2860a521@suse.com>

On 19/10/23 11:35, Jan Beulich wrote:
> On 19.10.2023 03:06, Stefano Stabellini wrote:
>> On Wed, 18 Oct 2023, Simone Ballarin wrote:
>>> --- a/xen/common/sched/core.c
>>> +++ b/xen/common/sched/core.c
>>> @@ -1504,6 +1504,8 @@ long vcpu_yield(void)
>>>   {
>>>       struct vcpu * v=current;
>>>       spinlock_t *lock;
>>> +    domid_t domain_id;
>>> +    int vcpu_id;
>>>   
>>>       rcu_read_lock(&sched_res_rculock);
>>>   
>>> @@ -1515,7 +1517,9 @@ long vcpu_yield(void)
>>>   
>>>       SCHED_STAT_CRANK(vcpu_yield);
>>>   
>>> -    TRACE_2D(TRC_SCHED_YIELD, current->domain->domain_id, current->vcpu_id);
>>> +    domain_id = current->domain->domain_id;
>>> +    vcpu_id = current->vcpu_id;
>>> +    TRACE_2D(TRC_SCHED_YIELD, domain_id, vcpu_id);
>>
>> Also here it looks like accessing the current pointer is considered a
>> side effect. Why? This is a just a global variable that is only accessed
>> for reading.
> 
> Not exactly. It's a per-CPU variable access on Arm, but involves a
> function call on x86. Still that function call has no other side
> effects, but I guess Misra wouldn't honor this.
> 
> I'm afraid though that the suggested change violates rule 2.2, as
> the two new assignments are dead code when !CONFIG_TRACEBUFFER.
> 
> Jan

I confirm that there is no problem in X86: I will simply propose a patch
adding __attribute_pure__ to get_cpu_info.

-- 
Simone Ballarin, M.Sc.

Field Application Engineer, BUGSENG (https://bugseng.com)