[Xen-devel,10/12] xen/arm: mm: Mark check_memory_layout_alignment_constraints as unused

Message ID 20190327184531.30986-11-julien.grall@arm.com
State Superseded
Headers show
Series
  • xen/arm: Add support to build with clang
Related show

Commit Message

Julien Grall March 27, 2019, 6:45 p.m.
Clang will throw an error if a function is unused unless you tell
to ignore it. This can be done using __maybe_unused.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/mm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Andrew Cooper March 27, 2019, 7:10 p.m. | #1
On 27/03/2019 18:45, Julien Grall wrote:
> Clang will throw an error if a function is unused unless you tell
> to ignore it. This can be done using __maybe_unused.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
>  xen/arch/arm/mm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 01ae2cccc0..d37925051a 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -160,7 +160,8 @@ unsigned long total_pages;
>  extern char __init_begin[], __init_end[];
>  
>  /* Checking VA memory layout alignment. */
> -static inline void check_memory_layout_alignment_constraints(void) {
> +static __maybe_unused void check_memory_layout_alignment_constraints(void)

As you're already changing this...

The style used elsewhere is

static void __init __maybe_unused build_assertions(void)

which has the added advantage that it more obvious to future developers
that trying to put code other than BUILD_BUG_ON() in it is a bad idea.

It took me a moment to realise that this function deliberately wasn't
called.

~Andrew

> +{
>      /* 2MB aligned regions */
>      BUILD_BUG_ON(XEN_VIRT_START & ~SECOND_MASK);
>      BUILD_BUG_ON(FIXMAP_ADDR(0) & ~SECOND_MASK);
Julien Grall April 9, 2019, 12:09 p.m. | #2
Hi Andrew,

On 27/03/2019 19:10, Andrew Cooper wrote:
> On 27/03/2019 18:45, Julien Grall wrote:
>> Clang will throw an error if a function is unused unless you tell
>> to ignore it. This can be done using __maybe_unused.
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>> ---
>>   xen/arch/arm/mm.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
>> index 01ae2cccc0..d37925051a 100644
>> --- a/xen/arch/arm/mm.c
>> +++ b/xen/arch/arm/mm.c
>> @@ -160,7 +160,8 @@ unsigned long total_pages;
>>   extern char __init_begin[], __init_end[];
>>   
>>   /* Checking VA memory layout alignment. */
>> -static inline void check_memory_layout_alignment_constraints(void) {
>> +static __maybe_unused void check_memory_layout_alignment_constraints(void)
> 
> As you're already changing this...
> 
> The style used elsewhere is
> 
> static void __init __maybe_unused build_assertions(void)
> 
> which has the added advantage that it more obvious to future developers
> that trying to put code other than BUILD_BUG_ON() in it is a bad idea.

Good point. I will update this patch (and the following one) to rename the 
function as well.

Cheers,

Patch

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 01ae2cccc0..d37925051a 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -160,7 +160,8 @@  unsigned long total_pages;
 extern char __init_begin[], __init_end[];
 
 /* Checking VA memory layout alignment. */
-static inline void check_memory_layout_alignment_constraints(void) {
+static __maybe_unused void check_memory_layout_alignment_constraints(void)
+{
     /* 2MB aligned regions */
     BUILD_BUG_ON(XEN_VIRT_START & ~SECOND_MASK);
     BUILD_BUG_ON(FIXMAP_ADDR(0) & ~SECOND_MASK);