| Message ID | AANLkTi=veEuNzENFGVBRDMSRBrv4Ks9w9MF62uB7K-XO@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Aug 30, 2010 at 10:01 PM, Blue Swirl <blauwirbel@gmail.com> wrote: > On Mon, Aug 30, 2010 at 8:42 PM, Blue Swirl <blauwirbel@gmail.com> wrote: >> On Mon, Aug 30, 2010 at 8:10 PM, malc <av1474@comtv.ru> wrote: >>> On Mon, 30 Aug 2010, Blue Swirl wrote: >>> >>>> On Mon, Aug 30, 2010 at 1:27 PM, Stefan Hajnoczi >>>> <stefanha@linux.vnet.ibm.com> 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? Stefan
On Tue, Aug 31, 2010 at 8:46 AM, Stefan Hajnoczi <stefanha@gmail.com> wrote: > On Mon, Aug 30, 2010 at 10:01 PM, Blue Swirl <blauwirbel@gmail.com> wrote: >> On Mon, Aug 30, 2010 at 8:42 PM, Blue Swirl <blauwirbel@gmail.com> wrote: >>> On Mon, Aug 30, 2010 at 8:10 PM, malc <av1474@comtv.ru> wrote: >>>> On Mon, 30 Aug 2010, Blue Swirl wrote: >>>> >>>>> On Mon, Aug 30, 2010 at 1:27 PM, Stefan Hajnoczi >>>>> <stefanha@linux.vnet.ibm.com> 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.
On Tue, Aug 31, 2010 at 7:00 PM, Blue Swirl <blauwirbel@gmail.com> wrote: > On Tue, Aug 31, 2010 at 8:46 AM, Stefan Hajnoczi <stefanha@gmail.com> wrote: >> On Mon, Aug 30, 2010 at 10:01 PM, Blue Swirl <blauwirbel@gmail.com> wrote: >>> On Mon, Aug 30, 2010 at 8:42 PM, Blue Swirl <blauwirbel@gmail.com> wrote: >>>> On Mon, Aug 30, 2010 at 8:10 PM, malc <av1474@comtv.ru> wrote: >>>>> On Mon, 30 Aug 2010, Blue Swirl wrote: >>>>> >>>>>> On Mon, Aug 30, 2010 at 1:27 PM, Stefan Hajnoczi >>>>>> <stefanha@linux.vnet.ibm.com> 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
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" }