Re: [PATCH 1/3] hw/mips/jazz: Remove the big_endian variable

[Thomas Huth <thuth@redhat.com>]

On 28/08/2023 14.19, Philippe Mathieu-Daudé wrote:
> Hi Thomas,
> 
> On 25/8/23 19:51, Thomas Huth wrote:
>> There is an easier way to get a value that can be used to decide
>> whether the target is big endian or not: Simply use the
>> target_words_bigendian() function instead.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   hw/mips/jazz.c | 10 ++--------
>>   1 file changed, 2 insertions(+), 8 deletions(-)
> 
> 
>> @@ -157,12 +157,6 @@ static void mips_jazz_init(MachineState *machine,
>>           [JAZZ_PICA61] = {33333333, 4},
>>       };
>> -#if TARGET_BIG_ENDIAN
>> -    big_endian = 1;
>> -#else
>> -    big_endian = 0;
>> -#endif
>> -
>>       if (machine->ram_size > 256 * MiB) {
>>           error_report("RAM size more than 256Mb is not supported");
>>           exit(EXIT_FAILURE);
>> @@ -301,7 +295,7 @@ static void mips_jazz_init(MachineState *machine,
>>               dev = qdev_new("dp8393x");
>>               qdev_set_nic_properties(dev, nd);
>>               qdev_prop_set_uint8(dev, "it_shift", 2);
>> -            qdev_prop_set_bit(dev, "big_endian", big_endian > 0);
>> +            qdev_prop_set_bit(dev, "big_endian", target_words_bigendian());
> 
> IIRC last time I tried that Peter pointed me at the documentation:
> 
> /**
>   * target_words_bigendian:
>   * Returns true if the (default) endianness of the target is big endian,
>   * false otherwise. Note that in target-specific code, you can use
>   * TARGET_BIG_ENDIAN directly instead. On the other hand, common
>   * code should normally never need to know about the endianness of the
>   * target, so please do *not* use this function unless you know very
>   * well what you are doing!
>   */
> 
> (Commit c95ac10340 "cpu: Provide a proper prototype for
>   target_words_bigendian() in a header")
> 
> Should we update the comment?

What would you change? My motivation here was mainly to decrease the size of 
the code - I think it's way more complicated via the #if + extra variable 
compared to simply calling target_words_bigendian(), isn't it? I think the 
diffstat says it all...

  Thomas