[Qemu-devel] [RFC] Configure option to turn on all debug printerfy

[Qemu-devel] [RFC] Configure option to turn on all debug printerfy

[Peter Crosthwaite]

So in a couple of cases now I have done work on mature device models
that include #ifdef'd debug printfery that is broken, and have
submitted (trivial) patches to fix. Whats happening is tree wide or
automated changes (changing types of variables etc) is causing the
debug printfery to break, usually werror issues on %x in printfs due
to types. Issue is, it never gets detected until someone tries to use
the debug mode: ./configure --extra-cflags="-DFOO_DEBUG". Can we have
some sort of alternate symbol "ALL_DEBUG" or something, such that
every debug printf is compiled? Then when developers make tree wide
changes they can at least compile test for breakages in debug printfs
using ./configure --extra-cflags="-DALL_DEBUG"? It would be fairly
trivial to implement.

Regards,
Peter

Re: [Qemu-devel] [RFC] Configure option to turn on all debug printerfy

[Andreas Färber]

Am 31.10.2012 04:10, schrieb Peter Crosthwaite:
> So in a couple of cases now I have done work on mature device models
> that include #ifdef'd debug printfery that is broken, and have
> submitted (trivial) patches to fix. Whats happening is tree wide or
> automated changes (changing types of variables etc) is causing the
> debug printfery to break, usually werror issues on %x in printfs due
> to types. Issue is, it never gets detected until someone tries to use
> the debug mode: ./configure --extra-cflags="-DFOO_DEBUG". Can we have
> some sort of alternate symbol "ALL_DEBUG" or something, such that
> every debug printf is compiled? Then when developers make tree wide
> changes they can at least compile test for breakages in debug printfs
> using ./configure --extra-cflags="-DALL_DEBUG"? It would be fairly
> trivial to implement.

The original idea I thought was to replace all the DPRINTFs by tracing...

An alternative might be to change the definition from empty DPRINTF(...)
to DPRINTF(...) if (0) { ... }. Then no ALL_DEBUG symbol would be
needed. I don't think anyone would like to get flooded with ALL_DEBUG!

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [RFC] Configure option to turn on all debug printerfy

[Peter Crosthwaite]

On Wed, Oct 31, 2012 at 1:22 PM, Andreas Färber <afaerber@suse.de> wrote:
> Am 31.10.2012 04:10, schrieb Peter Crosthwaite:
>> So in a couple of cases now I have done work on mature device models
>> that include #ifdef'd debug printfery that is broken, and have
>> submitted (trivial) patches to fix. Whats happening is tree wide or
>> automated changes (changing types of variables etc) is causing the
>> debug printfery to break, usually werror issues on %x in printfs due
>> to types. Issue is, it never gets detected until someone tries to use
>> the debug mode: ./configure --extra-cflags="-DFOO_DEBUG". Can we have
>> some sort of alternate symbol "ALL_DEBUG" or something, such that
>> every debug printf is compiled? Then when developers make tree wide
>> changes they can at least compile test for breakages in debug printfs
>> using ./configure --extra-cflags="-DALL_DEBUG"? It would be fairly
>> trivial to implement.
>
> The original idea I thought was to replace all the DPRINTFs by tracing...
>

Thats a much longer term goal isn't it? Also wont tracing have the
same problem, given that trace functionality is compiled on a per
module basis? Would need an ALL_TRACE.

> An alternative might be to change the definition from empty DPRINTF(...)
> to DPRINTF(...) if (0) { ... }.

Should work. Proof of concept diff below (untested).

Then no ALL_DEBUG symbol would be
> needed. I don't think anyone would like to get flooded with ALL_DEBUG!
>

So the idea is no one actually needs to run a QEMU built with
ALL_DEBUG. Just maintainers can compile test it for werror breakage.

Regards,
Peter

> Regards,
> Andreas
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>

--- a/hw/pflash_cfi01.c
+++ b/hw/pflash_cfi01.c
@@ -50,13 +50,16 @@ do { \
 } while(0)

 /* #define PFLASH_DEBUG */
-#ifdef PFLASH_DEBUG
-#define DPRINTF(fmt, ...)                          \
+#define DO_DPRINTF(fmt, ...)                       \
 do {                                               \
     printf("PFLASH: " fmt , ## __VA_ARGS__);       \
 } while (0)
+#ifdef PFLASH_DEBUG
+#define DPRINTF(fmt, ...) DO_PRINTF(fmt,  ## __VA_ARGS__)
 #else
-#define DPRINTF(fmt, ...) do { } while (0)
+#define DPRINTF(fmt, ...) if(0) {                   \
+    DO_PRINTF(fmt,  ## __VA_ARGS__)                 \
+}
 #endif

Re: [Qemu-devel] [RFC] Configure option to turn on all debug printerfy

[Igor Mitsyanko]

On 10/31/2012 07:10 AM, Peter Crosthwaite wrote:
> So in a couple of cases now I have done work on mature device models
> that include #ifdef'd debug printfery that is broken, and have
> submitted (trivial) patches to fix. Whats happening is tree wide or
> automated changes (changing types of variables etc) is causing the
> debug printfery to break, usually werror issues on %x in printfs due
> to types. Issue is, it never gets detected until someone tries to use
> the debug mode: ./configure --extra-cflags="-DFOO_DEBUG". Can we have
> some sort of alternate symbol "ALL_DEBUG" or something, such that
> every debug printf is compiled? Then when developers make tree wide
> changes they can at least compile test for breakages in debug printfs
> using ./configure --extra-cflags="-DALL_DEBUG"? It would be fairly
> trivial to implement.
>
> Regards,
> Peter
>

Agree, I think majority of device-specific debugging was broken after 
ARM physical address length was changed to
64 bit. Of course, no one would want to run QEMU with ALL_DEBUG, but I 
think no one wants to configure QEMU with --target-list=all either, but
sometimes we have to)

But touching every device could be too invasive, maybe add new target to 
QEMU makefile "make compile-test", and ask for every new contributor to
manually add his debug define in some kind of list of defines for "make 
compile-test".

-- 
Mitsyanko Igor
ASWG, Moscow R&D center, Samsung Electronics
email: i.mitsyanko@samsung.com

Re: [Qemu-devel] [RFC] Configure option to turn on all debug printerfy

[Igor Mitsyanko]

On 10/31/2012 01:29 PM, Igor Mitsyanko wrote:
>
> On 10/31/2012 07:10 AM, Peter Crosthwaite wrote:
>> So in a couple of cases now I have done work on mature device models
>> that include #ifdef'd debug printfery that is broken, and have
>> submitted (trivial) patches to fix. Whats happening is tree wide or
>> automated changes (changing types of variables etc) is causing the
>> debug printfery to break, usually werror issues on %x in printfs due
>> to types. Issue is, it never gets detected until someone tries to use
>> the debug mode: ./configure --extra-cflags="-DFOO_DEBUG". Can we have
>> some sort of alternate symbol "ALL_DEBUG" or something, such that
>> every debug printf is compiled? Then when developers make tree wide
>> changes they can at least compile test for breakages in debug printfs
>> using ./configure --extra-cflags="-DALL_DEBUG"? It would be fairly
>> trivial to implement.
>>
>> Regards,
>> Peter
>>
>
> Agree, I think majority of device-specific debugging was broken after 
> ARM physical address length was changed to
> 64 bit. Of course, no one would want to run QEMU with ALL_DEBUG, but I 
> think no one wants to configure QEMU with --target-list=all either, but
> sometimes we have to)
>
> But touching every device could be too invasive, maybe add new target 
> to QEMU makefile "make compile-test", and ask for every new 
> contributor to
> manually add his debug define in some kind of list of defines for 
> "make compile-test".
>

Or maybe just using TARGET_FMT_plx & co should be enough.

-- 
Mitsyanko Igor
ASWG, Moscow R&D center, Samsung Electronics
email: i.mitsyanko@samsung.com