| Message ID | 20170302195337.31558-12-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
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 <alex.bennee@linaro.org> > --- > 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? Thanks, Fred > static const uint8_t gic_id_11mpcore[] = { > 0x00, 0x00, 0x00, 0x00, 0x90, 0x13, 0x04, 0x00, 0x0d, 0xf0, 0x05, 0xb1 > }; >
On 3 March 2017 at 17:05, Frederic Konrad <fred.konrad@greensocs.com> 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 <alex.bennee@linaro.org> >> --- >> 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
On 2 March 2017 at 19:53, Alex Bennée <alex.bennee@linaro.org> 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 <alex.bennee@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
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) + static const uint8_t gic_id_11mpcore[] = { 0x00, 0x00, 0x00, 0x00, 0x90, 0x13, 0x04, 0x00, 0x0d, 0xf0, 0x05, 0xb1 };
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 <alex.bennee@linaro.org> --- hw/intc/arm_gic.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)