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

[Aleksandar Markovic]

> From: Thomas Huth <address@hidden>
> 
> On 8/19/19 2:07 PM, Aleksandar Markovic wrote:
> > From: Aleksandar Markovic <address@hidden>
> 
> > ...
> 
> > @@ -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).

Agreed, Thomas, this looks totally weird. Thanks for spotting it. I'll think a 
little
more about the best way of fixing it.

Aleksandar

> 
>  Thomas