Re: [Qemu-devel] Re: [PATCH 01/14] trace: Add trace-events file for declaring trace events

[Stefan Hajnoczi]

On Tue, Aug 31, 2010 at 7:00 PM, Blue Swirl <address@hidden> wrote:
> On Tue, Aug 31, 2010 at 8:46 AM, Stefan Hajnoczi <address@hidden> wrote:
>> On Mon, Aug 30, 2010 at 10:01 PM, Blue Swirl <address@hidden> wrote:
>>> On Mon, Aug 30, 2010 at 8:42 PM, Blue Swirl <address@hidden> wrote:
>>>> On Mon, Aug 30, 2010 at 8:10 PM, malc <address@hidden> wrote:
>>>>> On Mon, 30 Aug 2010, Blue Swirl wrote:
>>>>>
>>>>>> On Mon, Aug 30, 2010 at 1:27 PM, Stefan Hajnoczi
>>>>>> <address@hidden> wrote:
>>>>>> > This patch introduces the trace-events file where trace events can be
>>>>>> > declared like so:
>>>>>> >
>>>>>> > qemu_malloc(size_t size) "size %zu"
>>>>>> > qemu_free(void *ptr) "ptr %p"
>>>>>> >
>>>>>> > These trace event declarations are processed by a new tool called
>>>>>> > tracetool to generate code for the trace events.  Trace event
>>>>>> > declarations are independent of the backend tracing system (LTTng User
>>>>>> > Space Tracing, ftrace markers, DTrace).
>>>>>>
>>>>>> I think the tool does not work if 'sh' is not 'bash'. For example, on
>>>>>> OpenBSD I got:
>>>>>
>>>>> Well, it does work with ash.
>>>>>
>>>>>>
>>>>>> config-host.mak is out-of-date, running configure
>>>>>>
>>>>>> Error: invalid trace backend
>>>>>> Please choose a supported trace backend.
>>>>>>
>>>>>>   GEN   trace.h
>>>>>> /src/qemu/tracetool[176]: no closing quote
>>>>>>
>>>>>> This shows the problem:
>>>>>> sh -x ../tracetool --nop --check-backend
>>>>>> + set -f
>>>>>> ../tracetool[176]: no closing quote
>>>>>
>>>>> `set -f' is a valid construct according to:
>>>>> http://www.opengroup.org/onlinepubs/009695399/utilities/set.html
>>>>>
>>>>> The problem is likely elsewhere.
>>>>
>>>> Right, the offending lines are:
>>>>    echo ${1%%(*}
>>>> and
>>>>    args=${1#*(}
>>>>
>>>> If I remove both of those, the errors are gone.
>>>>
>>>
>>> This patch fixes the problem. Double quotes do not help.
>>>
>>> diff --git a/tracetool b/tracetool
>>> index d640100..01de580 100755
>>> --- a/tracetool
>>> +++ b/tracetool
>>> @@ -29,14 +29,14 @@ EOF
>>>  # Get the name of a trace event
>>>  get_name()
>>>  {
>>> -    echo ${1%%(*}
>>> +    echo ${1%%\(*}
>>>  }
>>>
>>>  # Get the argument list of a trace event, including types and names
>>>  get_args()
>>>  {
>>>     local args
>>> -    args=${1#*(}
>>> +    args=${1#*\(}
>>>     args=${args%)*}
>>>     echo "$args"
>>>  }
>>
>> Thanks for finding and fixing this!  I have been testing with dash in
>> addition to bash.
>>
>> I'd like to write tracetool in Python (or Perl, if needed) to
>> eliminate issues like this.  I chose shell though to avoid the
>> dependency on Python.  QEMU currently uses Perl for texi2pod.pl and
>> Python for QMP scripts but both are optional.  Tracetool isn't
>> optional because it runs even when tracing is disabled ("nop" backend
>> generates empty stub functions).  How do you feel about tracetool in
>> Python?
>
> I'd like to avoid additional dependencies. One way to solve this could
> be to ship QEMU with default-trace.[ch] which are used for the nop
> case, then tracetool implementation would not matter much. But these
> files would need to be updated, in synch, whenever new trace points
> are added which is fragile.
>
> Maybe a bit of CPP magic added to trace-events could simplify (or even
> eliminate) the script, something like we do with .hx files.

Okay, let's stick to tracetool in POSIX shell.  I'll resend this patch
with your fix included.

Stefan