Re: [Qemu-devel] [PATCH] fix incorrect bracket in tracetool

[Andreas Färber]

Am 16.03.2012 14:00, schrieb Lee Essen:
> On 16 Mar 2012, at 12:44, Andreas Färber wrote:
> 
>> Am 16.03.2012 13:29, schrieb Lee Essen:
>>> Signed-off-by: Lee Essen <address@hidden>
>>>
>>> ---
>>>
>>> scripts/tracetool |    2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/scripts/tracetool b/scripts/tracetool
>>> index 65bd0a1..2e43d05 100755
>>> --- a/scripts/tracetool
>>> +++ b/scripts/tracetool
>>> @@ -123,7 +123,7 @@ get_argc()
>>> # Get the format string including double quotes for a trace event
>>> get_fmt()
>>> {
>>> -    puts "${1#*)}"
>>> +    puts "${1#*}"
>>> }
>>>
>>> linetoh_begin_nop()
>>>
>> Cc'ing the trace maintainer. I assume Lee forgot to look up the
>> maintainer, but Stefan, my checking MAINTAINERS indicates tracetool is
>> missing in the Tracing section too. Could you add it please?
>>
>> Not being a shell expert I can't judge what this is actually trying to
>> do. Note that there is also an effort underway to rewrite tracetool as
>> tracetool.py.
> 
> Actually, I think I need to slow down a bit…

:) No need to rush, it's been broken for a while.

> there are more problems than just that bracket…
> 
> # make
>   GEN   trace.h
> /tmp/patch/qemu/scripts/tracetool[520]: local: not found [No such file or 
> directory]
> /tmp/patch/qemu/scripts/tracetool[66]: local: not found [No such file or 
> directory]
> /tmp/patch/qemu/scripts/tracetool[136]: local: not found [No such file or 
> directory]
> /tmp/patch/qemu/scripts/tracetool[55]: local: not found [No such file or 
> directory]
> /tmp/patch/qemu/scripts/tracetool[80]: local: not found [No such file or 
> directory]
> /tmp/patch/qemu/scripts/tracetool[55]: local: not found [No such file or 
> directory]
> 
> From what I can see "local" isn't supported in posix ...
> "The POSIX standard supports functions, as shown above, but the semantics are 
> weaker: functions do not have local traps or options, it is not possible to 
> define local variables, and functions can't be exported."

Hm, Blue's patch in the bug I referenced earlier still had local in it
and according to my comments worked with Solaris 10's /usr/xpg4/bin/sh,
and I don't have that issue on OpenIndiana (bash). What shell did you
test with on SmartOS?

> So I could do with some advice now on how to proceed … is the goal to keep 
> posix shell compliance? Wait for a tracetool.py version? Or should I go back 
> to messing with SHELL?

I'd recommend to evaluate what needs to be done to make the script(s)
POSIX-compliant. If the resulting patch is reasonable then IMO we should
apply it even if it gets replaced by a Python version later (it was
still feature-incomplete last time posted and has been floating around a
while already). Was just pointing it out for you in case it's easier to
get running on your system.

Andreas