Re: [XEN PATCH 3/4] xen/include: add pure and const attributes

From: Simone Ballarin <simone.ballarin@bugseng.com>
To: Stefano Stabellini <sstabellini@kernel.org>
Cc: xen-devel@lists.xenproject.org, consulting@bugseng.com,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	George Dunlap <george.dunlap@citrix.com>,
	Jan Beulich <jbeulich@suse.com>, Julien Grall <julien@xen.org>,
	Wei Liu <wl@xen.org>
Subject: Re: [XEN PATCH 3/4] xen/include: add pure and const attributes
Date: Thu, 19 Oct 2023 11:07:20 +0200	[thread overview]
Message-ID: <915d14c8-31fe-4381-bce6-3213da284115@bugseng.com> (raw)
In-Reply-To: <alpine.DEB.2.22.394.2310181801350.965337@ubuntu-linux-20-04-desktop>

On 19/10/23 03:02, Stefano Stabellini wrote:
> On Wed, 18 Oct 2023, Simone Ballarin wrote:
>> Add const and pure attributes to address reports
>> of MISRA C:2012 Rule 13.1: Initializer lists shall
>> not contain persistent side effects
>>
>> Add pure attribute to function pdx_to_pfn.
>> Add const attribute to functions generated by TYPE_SAFE.
>>
>> These functions are used in initializer lists: adding
>> the attributes ensures that no effect will be performed
>> by them.
>>
>> Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com>
> 
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> 
> However one comment below...
> 
>> ---
>> Function attributes pure and const do not need to be added as GCC
>> attributes, they can be added using ECLAIR configurations.
>> ---
>>   xen/include/xen/pdx.h      | 2 +-
>>   xen/include/xen/typesafe.h | 4 ++--
>>   2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/include/xen/pdx.h b/xen/include/xen/pdx.h
>> index f3fbc4273a..5d1050967a 100644
>> --- a/xen/include/xen/pdx.h
>> +++ b/xen/include/xen/pdx.h
>> @@ -164,7 +164,7 @@ static inline unsigned long pfn_to_pdx(unsigned long pfn)
>>    * @param pdx Page index
>>    * @return Obtained pfn after decompressing the pdx
>>    */
>> -static inline unsigned long pdx_to_pfn(unsigned long pdx)
>> +static inline __attribute_pure__ unsigned long pdx_to_pfn(unsigned long pdx)
>>   {
>>       return (pdx & pfn_pdx_bottom_mask) |
>>              ((pdx << pfn_pdx_hole_shift) & pfn_top_mask);
>> diff --git a/xen/include/xen/typesafe.h b/xen/include/xen/typesafe.h
>> index 7ecd3b4a8d..615df6f944 100644
>> --- a/xen/include/xen/typesafe.h
>> +++ b/xen/include/xen/typesafe.h
>> @@ -21,8 +21,8 @@
>>   
>>   #define TYPE_SAFE(_type, _name)                                         \
>>       typedef struct { _type _name; } _name##_t;                          \
>> -    static inline _name##_t _##_name(_type n) { return (_name##_t) { n }; } \
>> -    static inline _type _name##_x(_name##_t n) { return n._name; }
>> +    static inline __attribute_const__ _name##_t _##_name(_type n) { return (_name##_t) { n }; } \
>> +    static inline __attribute_const__ _type _name##_x(_name##_t n) { return n._name; }
>>   
>>   #else
> 
> I think we should also add __attribute_const__ in the NDEBUG definitions
> just below.

Ok.

-- 
Simone Ballarin, M.Sc.

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