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.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
