[Qemu-devel] Re: [PULL] More format checking

[Blue Swirl <blauwirbel@gmail.com>]

On Sat, Dec 4, 2010 at 7:24 PM, Stefan Weil <weil@mail.berlios.de> wrote:
> Am 04.12.2010 19:35, schrieb Blue Swirl:
>>
>> On Wed, Dec 1, 2010 at 9:41 PM, Stefan Weil <weil@mail.berlios.de> wrote:
>>>
>>> Am 24.11.2010 22:46, schrieb Stefan Weil:
>>>>
>>>> Hello,
>>>>
>>>> with these patches, hopefully all functions with printf like arguments
>>>> use gcc's format checking, so the TODO comment in HACKING can be
>>>> removed.
>>>>
>>>> The patch series was tested with a default build configuration on linux
>>>> and windows hosts (including some cross compilations for arm, mips and
>>>> powerpc),
>>>> so chances are good that there remain few (if any) functions without
>>>> format checking.
>>>>
>>>> If there remain such functions (and maybe also some related format
>>>> bugs),
>>>> the compiler will raise a warning now, so it should be easy to fix them.
>>>>
>>>> All patches were published on qemu-devel. There were two replies:
>>>>
>>>> * 1st patch: small typo in commit message, fixed now
>>>>
>>>> * last patch: breaks build
>>>>
>>>> It's normal that the last patch will break builds with warning = error
>>>> as long as the other four patches (and maybe more format related fixes)
>>>> are not applied. This is the intention of the last patch!
>>>>
>>>> Regards,
>>>> Stefan Weil
>>>>
>>>> The following changes since commit
>>>> f711df67d611e4762966a249742a5f7499e19f99:
>>>>
>>>>  microblaze: target-ify target_ucontext (2010-11-23 10:04:30 +0100)
>>>>
>>>> are available in the git repository at:
>>>>  git://git.weilnetz.de/git/qemu for-blueswirl
>>>>
>>>> Stefan Weil (5):
>>>>     *-dis: Replace fprintf_ftype by fprintf_function (format checking)
>>>>     target-sparc: Use fprintf_function (format checking)
>>>>     audio: Use GCC_FMT_ATTR (format checking)
>>>>     darwin-user: Use GCC_FMT_ATTR (format checking)
>>>>     configure: Add compiler option -Wmissing-format-attribute
>>>>
>>>>  HACKING                |    3 ---
>>>>  alpha-dis.c            |    3 +++
>>>>  arm-dis.c              |   14 +++++++-------
>>>>  audio/audio_pt_int.c   |    3 ++-
>>>>  configure              |    1 +
>>>>  darwin-user/machload.c |    2 +-
>>>>  darwin-user/qemu.h     |    2 +-
>>>>  dis-asm.h              |   10 ++--------
>>>>  m68k-dis.c             |    2 +-
>>>>  microblaze-dis.c       |    2 +-
>>>>  mips-dis.c             |    2 --
>>>>  sh4-dis.c              |   16 +++++-----------
>>>>  target-sparc/cpu.h     |    4 ++--
>>>>  13 files changed, 26 insertions(+), 38 deletions(-)
>>>>
>>>
>>>
>>> Is there anything missing, or can the patch series be applied finally?
>>
>> The last patch is not OK yet:
>> CC qemu-img.o
>> /src/qemu/qemu-img.c: In function 'error':
>> /src/qemu/qemu-img.c:48:5: error: function might be possible candidate
>> for 'ms_printf' format attribute [-Werror=missing-format-attribute]
>> CC qemu-error.o
>> /src/qemu/qemu-error.c: In function 'error_vprintf':
>> /src/qemu/qemu-error.c:27:9: error: function might be possible
>> candidate for 'ms_printf' format attribute
>> [-Werror=missing-format-attribute]
>> CC block/blkverify.o
>> /src/qemu/block/blkverify.c: In function 'blkverify_err':
>> /src/qemu/block/blkverify.c:65:5: error: function might be possible
>> candidate for 'ms_printf' format attribute
>> [-Werror=missing-format-attribute]
>> CC json-parser.o
>> /src/qemu/json-parser.c: In function 'parse_error':
>> /src/qemu/json-parser.c:100:5: error: function might be possible
>> candidate for 'ms_printf' format attribute
>> [-Werror=missing-format-attribute]
>> CC qerror.o
>> /src/qemu/qerror.c: In function 'qerror_abort':
>> /src/qemu/qerror.c:230:5: error: function might be possible candidate
>> for 'ms_printf' format attribute [-Werror=missing-format-attribute]
>> CC blockdev.o
>> /src/qemu/blockdev.c: In function 'drive_add':
>> /src/qemu/blockdev.c:52:5: error: function might be possible candidate
>> for 'ms_printf' format attribute [-Werror=missing-format-attribute]
>> CC qemu-char.o
>> /src/qemu/qemu-char.c: In function 'qemu_chr_printf':
>> /src/qemu/qemu-char.c:182:5: error: function might be possible
>> candidate for 'ms_printf' format attribute
>> [-Werror=missing-format-attribute]
>> CC audio/audio.o
>> /src/qemu/audio/audio.c: In function 'AUD_vlog':
>> /src/qemu/audio/audio.c:346:9: error: function might be possible
>> candidate for 'ms_printf' format attribute
>> [-Werror=missing-format-attribute]
>>
>> This is with mingw32 compiler, it should be happy with gnu_printf
>> instead of ms_printf in case you wonder that.
>
> As far as I know all those warnings are false warnings.

No, for example qemu-img.c and blockdev.c cases are real. I don't
understand blkverify.c and qerror cases, GCC_FMT_ATTR() attributes are
in place.

> The mingw32 includes are broken because they always
> use ms format specifiers for the PRI* macros, even
> when the compiler was told to use ansi/posix format.
> Sending a patch to mingw32 is on my todo list.

I think that is unrelated.

> qemu-common.h uses the same kind of wrong definitions
> for PRI*64. I already have a patch for this and will send
> it to qemu-devel soon.
>
> Until mingw32 is fixed, I suggest to disable -Werror for
> windows builds.
>
> Is it possible to commit the previous patches (if you
> hesitate committing the last one)? That would help a lot.

OK.