Bandan Das <b...@redhat.com> writes: > Eric Blake <ebl...@redhat.com> writes: > >> On 05/28/2015 12:48 PM, Bandan Das wrote: >>> Markus Armbruster <arm...@redhat.com> writes: >>> >>>> Bandan Das <b...@redhat.com> writes: >>>> >>>>> There's too much going on in monitor_parse_command(). >>>>> Split up the arguments parsing bits into a separate function >>>>> monitor_parse_arguments(). Let the original function check for >>>>> command validity and sub-commands if any and return data (*cmd) >>>>> that the newly introduced function can process and return a >>>>> QDict. Also, pass a pointer to the cmdline to track current >>>>> parser location. >> >>>>> >>>>> #ifdef DEBUG >>>>> - monitor_printf(mon, "command='%s', start='%d'\n", cmdline, start); >>>>> + monitor_printf(mon, "command='%s', start='%d'\n", cmdline, *start); >>>> >>>> Would this compile if we defined DEBUG? >>> >>> No, it won't :) Sorry, will fix. >> >> That's why I like solutions that can't bitrot; something like this >> framework (needs a bit more to actually compile, but you get the picture): >> >> #ifdef DEBUG >> # define DEBUG_MONITOR 1 >> #else >> # define DEBUG_MONITOR 0 >> #endif >> #define DEBUG_MONITOR_PRINTF(stuff...) do { \ >> if (DEBUG_MONITOR) { \ >> monitor_printf(stuff...); \ >> } \ >> } while (0) >> >> then you can avoid the #ifdef in the function body, and just do >> DEBUG_MONITOR_PRINTF(mon, "command='%s'....) and the compiler will >> always check for correct format vs. arguments, even when debugging is off. >> >> Of course, adding such a framework in this file would be a separate >> patch, and does not have to be done as a prerequisite of this series. > > Thanks, Eric. Ok, I will post patch(es) separately.
The preferred solution in QEMU is tracepoints. In this case, I wouldn't mind ditching the debugging prints instead. All three of them.