Re: [Qemu-devel] [PATCH for-3.1 0/2] Fix disas/nanomips

[Stefan Weil]

Am 27.11.2018 um 13:36 schrieb Peter Maydell:
> On Tue, 27 Nov 2018 at 12:17, Stefan Weil <address@hidden> wrote:
>> These two patches fix wrong format strings used in disas/nanomips.
>>
>> The first patch replaces proprietary data types by POSIX data types,
>> because otherwise the PRI... macros cannot be used in the second patch.
> I think this is a good idea, but that first patch is a huge patch for
> this point in the 3.1 release cycle. Can we fix the warnings
> by just casting the arguments to the img::format() function ?


Casting would be possible, but I try to avoid it if there are other
solutions.

The patch is huge, but the changes are purely mechanical. I used
commands like "perl -pi -e s/int64/int64_t/g nanomips.*" to make them.


>> Those patches are only relevant for 3.1 if full nanomips support is 
>> considered
>> important enought and if QEMU is used on big endian machines (I think
>> the old code will work on little endian hosts even with wrong format
>> strings).
> It only affects the disassembler, which is a debug tool.
> A week ago this would have been a fairly definite "yes, fix it"
> issue; at this point, with rc3 due to go out today, it's a bit
> trickier to justify. (rc3 will be the last rc unless any showstopper
> issues turn up.)


Yes.


>> I use those patches for QEMU for Windows because that is compiled with
>> more compiler warnings, so compilation would fail without the fix.
> My Windows cross-compiler builds work OK, FWIW, and -Werror is
> only enabled by default for Linux builds, so warnings shouldn't
> result in build failures.


I forgot to mention another potential portability issue: the proprietary
data types which I removed in the first patch work on all relevant hosts
/ compilers which I know, but there might also be platforms where the
definitions are wrong:

-typedef unsigned short uint16;
-typedef unsigned int uint32;
-typedef long long int64;
-typedef unsigned long long uint64;

Nevertheless I think that 3.1 can also be released without changing 
disas/nanomips.

Stefan