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.
