From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=41501 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1Q7ZNT-0007U2-Vw
	for qemu-devel@nongnu.org; Wed, 06 Apr 2011 16:31:01 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1Q7ZNS-0006vq-Ql
	for qemu-devel@nongnu.org; Wed, 06 Apr 2011 16:30:59 -0400
Received: from mail-vw0-f45.google.com ([209.85.212.45]:60613)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1Q7ZNS-0006vT-OP
	for qemu-devel@nongnu.org; Wed, 06 Apr 2011 16:30:58 -0400
Received: by vws17 with SMTP id 17so1658883vws.4
	for <qemu-devel@nongnu.org>; Wed, 06 Apr 2011 13:30:58 -0700 (PDT)
MIME-Version: 1.0
In-Reply-To: <874o6bbh4j.fsf@ginnungagap.bsc.es>
References: <20110404214900.9761.49418.stgit@ginnungagap.bsc.es>
	<20110404214933.9761.62154.stgit@ginnungagap.bsc.es>
	<BANLkTimfcQbK-H99GvyUvd=qScs=vWTOcg@mail.gmail.com>
	<874o6bbh4j.fsf@ginnungagap.bsc.es>
Date: Wed, 6 Apr 2011 21:30:58 +0100
Message-ID: <BANLkTimsbG+rRrKgnLsJJvkVkTnc+xm3Fw@mail.gmail.com>
Subject: Re: [Qemu-devel] Re: [PATCH 5/6] trace-state: [simple] add "-trace
	events" argument to control initial state
From: Stefan Hajnoczi <stefanha@gmail.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/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: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: =?ISO-8859-1?Q?Llu=EDs?= <xscript@gmx.net>
Cc: Stefan Hajnoczi <stefanha@gmail.com>, qemu-devel@nongnu.org

On Wed, Apr 6, 2011 at 3:15 PM, Llu=EDs <xscript@gmx.net> wrote:
> Stefan Hajnoczi writes:
>
>>> + =A0 =A0 =A0 =A0 =A0 =A0if (len > 1) { =A0 =A0 =A0 =A0 =A0 =A0 =A0/* s=
kip empty lines */
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0line[len - 1] =3D '\0';
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (!st_change_trace_event_state(line,=
 true)) {
>
>> The build breaks when --enable-trace-backend !=3D simple because this
>> code is outside an #ifdef CONFIG_SIMPLE_TRACE. =A0Please add this:
>
>> diff --git a/simpletrace.h b/simpletrace.h
>> index 8d893bd..5d9d2ec 100644
>> --- a/simpletrace.h
>> +++ b/simpletrace.h
>> @@ -43,6 +43,11 @@ static inline bool st_init(const char *file)
>> =A0{
>> =A0 =A0 =A0return true;
>> =A0}
>> +
>> +static bool st_change_trace_event_state(const char *tname, bool tstate)
>> +{
>> + =A0 =A0return true;
>> +}
>> =A0#endif /* !CONFIG_SIMPLE_TRACE */
>
>> =A0#endif /* SIMPLETRACE_H */
>
> Hmmm... why don't simply conditionally call st_init (put it into an
> #ifdef) and remove the "#else" in simpletrace.h.
>
> I've looked at it and it's not called from anywhere else.

The benefit to stubbing out these functions is that callers don't have
#ifdefs.  And caller code is always built (i.e. syntax checked by the
parser) so it helps avoid bitrot.  vl.c:main() is already a long and
ugly function so it would be nice to avoid #ifdefs there.

> This also reminds me that I didn't see any "-trace" option parsing in
> the OS-specific frontends (at least in linux-user).

User emulation does not have any way to control simpletrace today.

Stefan