From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:33192)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <bsd@redhat.com>) id 1Yy4M4-0003y1-4r
	for qemu-devel@nongnu.org; Thu, 28 May 2015 16:24:41 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <bsd@redhat.com>) id 1Yy4M0-0005SX-TO
	for qemu-devel@nongnu.org; Thu, 28 May 2015 16:24:40 -0400
Received: from mx1.redhat.com ([209.132.183.28]:59797)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <bsd@redhat.com>) id 1Yy4M0-0005SG-O7
	for qemu-devel@nongnu.org; Thu, 28 May 2015 16:24:36 -0400
Received: from int-mx14.intmail.prod.int.phx2.redhat.com
	(int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27])
	by mx1.redhat.com (Postfix) with ESMTPS id 60F3F19F279
	for <qemu-devel@nongnu.org>; Thu, 28 May 2015 20:24:35 +0000 (UTC)
From: Bandan Das <bsd@redhat.com>
References: <1432768754-28523-1-git-send-email-bsd@redhat.com>
	<1432768754-28523-2-git-send-email-bsd@redhat.com>
	<87h9qxi322.fsf@blackfin.pond.sub.org> <jpgpp5kczc5.fsf@redhat.com>
	<55677108.8090205@redhat.com>
Date: Thu, 28 May 2015 16:24:33 -0400
In-Reply-To: <55677108.8090205@redhat.com> (Eric Blake's message of "Thu, 28
	May 2015 13:48:24 -0600")
Message-ID: <jpgvbfc5u2m.fsf@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [Qemu-devel] [PATCH v3 1/2] monitor: cleanup parsing of cmd
	name and cmd arguments
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Eric Blake <eblake@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>, qemu-devel@nongnu.org

Eric Blake <eblake@redhat.com> writes:

> On 05/28/2015 12:48 PM, Bandan Das wrote:
>> Markus Armbruster <armbru@redhat.com> writes:
>> 
>>> Bandan Das <bsd@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.