| Message ID | 7A168ABF-EB8A-43E5-9821-F4D8AD9B6E53@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Programmingkid <programmingkidx@gmail.com> writes: > Add debug macros to the code for easier debugging. > > Signed-off-by: John Arbuckle <programmingkidx@gmail.com> > --- > hw/input/hid.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/hw/input/hid.c b/hw/input/hid.c > index 329a27b..42ca592 100644 > --- a/hw/input/hid.c > +++ b/hw/input/hid.c > @@ -37,6 +37,13 @@ > #define RELEASED -1 > #define PUSHED -2 > > +/* #define DEBUG_HID_CODE */ > +#ifdef DEBUG_HID_CODE > + #define DEBUG_HID(fmt, ...) printf(fmt, __VA_ARGS__) > +#else > + #define DEBUG_HID(fmt, ...) (void)0 > +#endif > + This style of debug setup is discouraged these days as its prone to bitrot. It's better to define like this: #define DEBUG_HID(fmt, ...) \ if (DEBUG_HID_CODE) { \ printf(fmt, __VA_ARGS);\ } This means you get the benefit of the compiler checking your format strings even if the code gets optimised away when DEBUG_HID_CODE isn't defined. > /* Translates a QKeyCode to USB HID value */ > static const uint8_t qcode_to_usb_hid[] = { > [Q_KEY_CODE_SHIFT] = USB_HID_LEFT_SHIFT, > @@ -331,6 +338,7 @@ static void hid_keyboard_event(DeviceState *dev, QemuConsole *src, > return; > } > keycode = qcode_to_usb_hid[qcode]; > + DEBUG_HID("keycode = 0x%x qcode:%d\n", keycode, qcode); > > count = 2; > if (evt->u.key.data->down == false) { /* if key up event */ > @@ -381,6 +389,9 @@ static void hid_keyboard_process_keycode(HIDState *hs) > slot = hs->head & QUEUE_MASK; QUEUE_INCR(hs->head); hs->n--; > keycode = hs->kbd.keycodes[slot]; > > + DEBUG_HID("keycode:0x%x status:%s\n", keycode, (status == PUSHED ? "Pushed" > + : "Released")); > + > /* handle Control, Option, GUI/Windows/Command, and Shift keys */ > if (keycode >= 0xe0) { > process_modifier_key(status, keycode, &(hs->kbd.modifiers)); -- Alex Bennée
On Mar 26, 2016, at 2:48 AM, Alex Bennée wrote: > > Programmingkid <programmingkidx@gmail.com> writes: > >> Add debug macros to the code for easier debugging. >> >> Signed-off-by: John Arbuckle <programmingkidx@gmail.com> >> --- >> hw/input/hid.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/hw/input/hid.c b/hw/input/hid.c >> index 329a27b..42ca592 100644 >> --- a/hw/input/hid.c >> +++ b/hw/input/hid.c >> @@ -37,6 +37,13 @@ >> #define RELEASED -1 >> #define PUSHED -2 >> >> +/* #define DEBUG_HID_CODE */ >> +#ifdef DEBUG_HID_CODE >> + #define DEBUG_HID(fmt, ...) printf(fmt, __VA_ARGS__) >> +#else >> + #define DEBUG_HID(fmt, ...) (void)0 >> +#endif >> + > > This style of debug setup is discouraged these days as its prone to > bitrot. It's better to define like this: > > #define DEBUG_HID(fmt, ...) \ > if (DEBUG_HID_CODE) { \ > printf(fmt, __VA_ARGS);\ > } > > This means you get the benefit of the compiler checking your format > strings even if the code gets optimised away when DEBUG_HID_CODE isn't > defined. I don't like if conditions in the debug macro because they do take up cpu time. The (void)0 is just zero. I don't think it would take any cpu time away from the program.
It won't. The compiler will dead code away any branches where the test evaluates to a constant. On 27 Mar 2016 3:30 p.m., "Programmingkid" <programmingkidx@gmail.com> wrote: > > On Mar 26, 2016, at 2:48 AM, Alex Bennée wrote: > > > > > Programmingkid <programmingkidx@gmail.com> writes: > > > >> Add debug macros to the code for easier debugging. > >> > >> Signed-off-by: John Arbuckle <programmingkidx@gmail.com> > >> --- > >> hw/input/hid.c | 11 +++++++++++ > >> 1 file changed, 11 insertions(+) > >> > >> diff --git a/hw/input/hid.c b/hw/input/hid.c > >> index 329a27b..42ca592 100644 > >> --- a/hw/input/hid.c > >> +++ b/hw/input/hid.c > >> @@ -37,6 +37,13 @@ > >> #define RELEASED -1 > >> #define PUSHED -2 > >> > >> +/* #define DEBUG_HID_CODE */ > >> +#ifdef DEBUG_HID_CODE > >> + #define DEBUG_HID(fmt, ...) printf(fmt, __VA_ARGS__) > >> +#else > >> + #define DEBUG_HID(fmt, ...) (void)0 > >> +#endif > >> + > > > > This style of debug setup is discouraged these days as its prone to > > bitrot. It's better to define like this: > > > > #define DEBUG_HID(fmt, ...) \ > > if (DEBUG_HID_CODE) { \ > > printf(fmt, __VA_ARGS);\ > > } > > > > This means you get the benefit of the compiler checking your format > > strings even if the code gets optimised away when DEBUG_HID_CODE isn't > > defined. > > I don't like if conditions in the debug macro because they do take up cpu > time. The (void)0 is just zero. I don't think it would take any cpu time > away from the program.
diff --git a/hw/input/hid.c b/hw/input/hid.c index 329a27b..42ca592 100644 --- a/hw/input/hid.c +++ b/hw/input/hid.c @@ -37,6 +37,13 @@ #define RELEASED -1 #define PUSHED -2 +/* #define DEBUG_HID_CODE */ +#ifdef DEBUG_HID_CODE + #define DEBUG_HID(fmt, ...) printf(fmt, __VA_ARGS__) +#else + #define DEBUG_HID(fmt, ...) (void)0 +#endif + /* Translates a QKeyCode to USB HID value */ static const uint8_t qcode_to_usb_hid[] = { [Q_KEY_CODE_SHIFT] = USB_HID_LEFT_SHIFT, @@ -331,6 +338,7 @@ static void hid_keyboard_event(DeviceState *dev, QemuConsole *src, return; } keycode = qcode_to_usb_hid[qcode]; + DEBUG_HID("keycode = 0x%x qcode:%d\n", keycode, qcode); count = 2; if (evt->u.key.data->down == false) { /* if key up event */ @@ -381,6 +389,9 @@ static void hid_keyboard_process_keycode(HIDState *hs) slot = hs->head & QUEUE_MASK; QUEUE_INCR(hs->head); hs->n--; keycode = hs->kbd.keycodes[slot]; + DEBUG_HID("keycode:0x%x status:%s\n", keycode, (status == PUSHED ? "Pushed" + : "Released")); + /* handle Control, Option, GUI/Windows/Command, and Shift keys */ if (keycode >= 0xe0) { process_modifier_key(status, keycode, &(hs->kbd.modifiers));
Add debug macros to the code for easier debugging. Signed-off-by: John Arbuckle <programmingkidx@gmail.com> --- hw/input/hid.c | 11 +++++++++++ 1 file changed, 11 insertions(+)