Re: [Qemu-devel] [PATCH v2 11/11] hw/intc/arm_gic: modernise the DPRINTF

[Peter Maydell]

On 3 March 2017 at 17:05, Frederic Konrad <address@hidden> wrote:
> Hi Alex,
>
> On 03/02/2017 08:53 PM, Alex Bennée wrote:
>> While I was debugging the icount issues I realised a bunch of the
>> messages look quite similar. I've fixed this by including __func__ in
>> the debug print. At the same time I move the a modern if (GATE) style
>> printf which ensures the compiler can check for format string errors
>> even if the code gets optimised away in the non-DEBUG_GIC case.
>>
>> Signed-off-by: Alex Bennée <address@hidden>
>> ---
>>  hw/intc/arm_gic.c | 13 +++++++++----
>>  1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
>> index 8e5a9d8a3e..b305d9032a 100644
>> --- a/hw/intc/arm_gic.c
>> +++ b/hw/intc/arm_gic.c
>> @@ -26,15 +26,20 @@
>>  #include "qemu/log.h"
>>  #include "trace.h"
>>
>> -//#define DEBUG_GIC
>> +/* #define DEBUG_GIC */
>>
>>  #ifdef DEBUG_GIC
>> -#define DPRINTF(fmt, ...) \
>> -do { fprintf(stderr, "arm_gic: " fmt , ## __VA_ARGS__); } while (0)
>> +#define DEBUG_GIC_GATE 1
>>  #else
>> -#define DPRINTF(fmt, ...) do {} while(0)
>> +#define DEBUG_GIC_GATE 0
>>  #endif
>>
>> +#define DPRINTF(fmt, ...) do {                                          \
>> +        if (DEBUG_GIC_GATE) {                                           \
>> +            fprintf(stderr, "%s: " fmt, __func__, ## __VA_ARGS__);      \
>> +        }                                                               \
>> +    } while (0)
>> +
>
> Seems a prefered way is using qemu_log instead of fprintf?

This is debug print. Our "preferred" approach is the trace-events
framework (look at how gicv3 does it for example), which has the
nice property of being user-enableable at runtime, but there's
no absolute requirement to update old code to use that instead of
debug printfs. (It's not completely trivial to update, and among
other things you need to consider what trace might be useful to
the user rather than merely whatever random fprintfs you found
helpful during development.)

qemu_log is for certain specific kinds of user-enableable tracing
(notably "guest did something wrong" and "we don't implement this").

thanks
-- PMM