Re: [Qemu-devel] [PATCH v8 10/37] target/mips: Style improvements in helper.c

[Thomas Huth]

On 8/19/19 2:07 PM, Aleksandar Markovic wrote:
> From: Aleksandar Markovic <address@hidden>
> 
> Fixes mostly errors and warnings reported by 'checkpatch.pl -f'.
> 
> Signed-off-by: Aleksandar Markovic <address@hidden>
> ---
>  target/mips/helper.c | 98 
> ++++++++++++++++++++++++++++++++--------------------
>  1 file changed, 60 insertions(+), 38 deletions(-)
> 
> diff --git a/target/mips/helper.c b/target/mips/helper.c
> index 6e583d3..d7a2c77 100644
> --- a/target/mips/helper.c
> +++ b/target/mips/helper.c
> @@ -39,8 +39,8 @@ enum {
>  #if !defined(CONFIG_USER_ONLY)
>  
>  /* no MMU emulation */
> -int no_mmu_map_address (CPUMIPSState *env, hwaddr *physical, int *prot,
> -                        target_ulong address, int rw, int access_type)
> +int no_mmu_map_address(CPUMIPSState *env, hwaddr *physical, int *prot,
> +                       target_ulong address, int rw, int access_type)
>  {
>      *physical = address;
>      *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
> @@ -48,26 +48,28 @@ int no_mmu_map_address (CPUMIPSState *env, hwaddr 
> *physical, int *prot,
>  }
>  
>  /* fixed mapping MMU emulation */
> -int fixed_mmu_map_address (CPUMIPSState *env, hwaddr *physical, int *prot,
> -                           target_ulong address, int rw, int access_type)
> +int fixed_mmu_map_address(CPUMIPSState *env, hwaddr *physical, int *prot,
> +                          target_ulong address, int rw, int access_type)
>  {
>      if (address <= (int32_t)0x7FFFFFFFUL) {
> -        if (!(env->CP0_Status & (1 << CP0St_ERL)))
> +        if (!(env->CP0_Status & (1 << CP0St_ERL))) {
>              *physical = address + 0x40000000UL;
> -        else
> +        } else {
>              *physical = address;
> -    } else if (address <= (int32_t)0xBFFFFFFFUL)
> +        }
> +    } else if (address <= (int32_t)0xBFFFFFFFUL) {

While you're at it: That line looks weird. Why is this first marked as
"unsigned long" with the UL prefix and then casted through a signed
int32_t ? I think you should either drop the prefix or the cast here
(but probably rather in a separate patch).

 Thomas