Patchwork fix incorrect bracket in tracetool

login
register
mail settings
Submitter Lee Essen
Date March 16, 2012, 12:29 p.m.
Message ID <541C8B7E-1E25-479E-B69E-A4B58BFA45F3@nowonline.co.uk>
Download mbox | patch
Permalink /patch/147197/
State New
Headers show

Comments

Lee Essen - March 16, 2012, 12:29 p.m.
Signed-off-by: Lee Essen <lee.essen@nowonline.co.uk>

---

 scripts/tracetool |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Andreas Färber - March 16, 2012, 12:44 p.m.
Am 16.03.2012 13:29, schrieb Lee Essen:
> Signed-off-by: Lee Essen <lee.essen@nowonline.co.uk>
>
> ---
>
>  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.

Andreas
Lee Essen - March 16, 2012, 1 p.m.
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 <lee.essen@nowonline.co.uk>
>> 
>> ---
>> 
>> 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.
> 
> Andreas
> 

Actually, I think I need to slow down a bit… 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."

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?

Regards,

Lee.
Paolo Bonzini - March 16, 2012, 1:18 p.m.
Il 16/03/2012 13:29, Lee Essen ha scritto:
> @@ -123,7 +123,7 @@ get_argc()
>  # Get the format string including double quotes for a trace event
>  get_fmt()
>  {
> -    puts "${1#*)}"
> +    puts "${1#*}"
>  }
>  

Eric, can you look at this?  Is it a bashism or a Solaris bug?

I would write it, to be entirely safe, as

   local fmt
   fmt=${1#*\)}
   puts "$fmt"

where I'm using the extra variable to avoid the ambiguity of quoting the
parentheses within quotes; variable assignments are always implicitly
quoted.

Paolo
Paolo Bonzini - March 16, 2012, 1:19 p.m.
Il 16/03/2012 14:00, Lee Essen ha scritto:
> /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."
> 
> 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 think #!/bin/bash is a better solution in the short-term.

Paolo
Andreas Färber - March 16, 2012, 1:21 p.m.
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 <lee.essen@nowonline.co.uk>
>>>
>>> ---
>>>
>>> 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
Andreas Färber - March 16, 2012, 1:28 p.m.
Am 16.03.2012 14:19, schrieb Paolo Bonzini:
> Il 16/03/2012 14:00, Lee Essen ha scritto:
>> /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."
>>
>> 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 think #!/bin/bash is a better solution in the short-term.

Breaks if it's in /usr/gnu/bin or /opt/SUNWfoo/bin. :)

Just like Python scripts can't rely on #!/usr/bin/env python on
BeOS/Haiku in lack of /usr. The world is complicated.

Andreas
Eric Blake - March 16, 2012, 2:56 p.m.
On 03/16/2012 07:18 AM, Paolo Bonzini wrote:
> Il 16/03/2012 13:29, Lee Essen ha scritto:
>> @@ -123,7 +123,7 @@ get_argc()
>>  # Get the format string including double quotes for a trace event
>>  get_fmt()
>>  {
>> -    puts "${1#*)}"

This says to call puts with the first argument of get_fmt, except with
the shortest prefix ending in ) omitted.  Or are you complaining that
there is a shell treating this as a syntax error?

>> +    puts "${1#*}"

This says to omit the shortest prefix that matches the glob '*', but
that is always the empty string, so you might as well write it "$1".

>>  }
>>  
> 
> Eric, can you look at this?  Is it a bashism or a Solaris bug?

Solaris /bin/sh lacks support for ${var#pattern}, but POSIX requires it.
 If this is using #!/bin/sh, you aren't portable.

> 
> I would write it, to be entirely safe, as
> 
>    local fmt

local is not portable.

>    fmt=${1#*\)}
>    puts "$fmt"

This looks reasonable, if you were hitting syntax errors on an unquoted
), and if you are sure that you have a POSIX rather than Solaris /bin/sh.

> 
> where I'm using the extra variable to avoid the ambiguity of quoting the
> parentheses within quotes; variable assignments are always implicitly
> quoted.
> 
> Paolo
>
Stefan Hajnoczi - March 19, 2012, 11:59 a.m.
On Fri, Mar 16, 2012 at 12:29 PM, Lee Essen <lee.essen@nowonline.co.uk> wrote:
> Signed-off-by: Lee Essen <lee.essen@nowonline.co.uk>
>
> ---
>
>  scripts/tracetool |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

I'm going to spend some time today reviewing recent tracing patches.
I'd prefer to move to a Python version of tracetool rather than worry
about the shell quirks across all host platforms.

If the Python rewrite cannot be merged for 1.1 then it makes sense to
go with shell portability fix.

Stefan
Lee Essen - March 19, 2012, 12:05 p.m.
On 19 Mar 2012, at 11:59, Stefan Hajnoczi wrote:

> On Fri, Mar 16, 2012 at 12:29 PM, Lee Essen <lee.essen@nowonline.co.uk> wrote:
>> Signed-off-by: Lee Essen <lee.essen@nowonline.co.uk>
>> 
>> ---
>> 
>>  scripts/tracetool |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> I'm going to spend some time today reviewing recent tracing patches.
> I'd prefer to move to a Python version of tracetool rather than worry
> about the shell quirks across all host platforms.
> 
> If the Python rewrite cannot be merged for 1.1 then it makes sense to
> go with shell portability fix.

Hi Stefan,

While you are looking at this, there are a couple of other related issues worth having in the back of your mind:

1. "self" is a reserved word in Solaris/Illumos trace, and it's used in a few trace calls.
2. "bool" isn't recognised by default, again used in a couple of traces ... could be fixed by typedef, but switching to int is probably better (imho)
3. Some work is needed on the linking stage when using Solaris/Illumos dtrace. You need to provide all the objs to the dtrace -G call in order to get an object generated that includes all of the relevant symbols in it. Happy to provide more detail if needed.

Cheers,

Lee.
Stefan Hajnoczi - March 19, 2012, 12:32 p.m.
On Mon, Mar 19, 2012 at 12:05 PM, Lee Essen <lee.essen@nowonline.co.uk> wrote:
> On 19 Mar 2012, at 11:59, Stefan Hajnoczi wrote:
>
>> On Fri, Mar 16, 2012 at 12:29 PM, Lee Essen <lee.essen@nowonline.co.uk> wrote:
>>> Signed-off-by: Lee Essen <lee.essen@nowonline.co.uk>
>>>
>>> ---
>>>
>>>  scripts/tracetool |    2 +-
>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> I'm going to spend some time today reviewing recent tracing patches.
>> I'd prefer to move to a Python version of tracetool rather than worry
>> about the shell quirks across all host platforms.
>>
>> If the Python rewrite cannot be merged for 1.1 then it makes sense to
>> go with shell portability fix.
>
> Hi Stefan,
>
> While you are looking at this, there are a couple of other related issues worth having in the back of your mind:
>
> 1. "self" is a reserved word in Solaris/Illumos trace, and it's used in a few trace calls.
> 2. "bool" isn't recognised by default, again used in a couple of traces ... could be fixed by typedef, but switching to int is probably better (imho)
> 3. Some work is needed on the linking stage when using Solaris/Illumos dtrace. You need to provide all the objs to the dtrace -G call in order to get an object generated that includes all of the relevant symbols in it. Happy to provide more detail if needed.

If you already have patches for these points then please send them.
You, I, or someone else can port them to Python.

Stefan
Andreas Färber - March 19, 2012, 12:32 p.m.
Am 19.03.2012 13:05, schrieb Lee Essen:
> On 19 Mar 2012, at 11:59, Stefan Hajnoczi wrote:
>
>> I'm going to spend some time today reviewing recent tracing patches.
>> I'd prefer to move to a Python version of tracetool rather than worry
>> about the shell quirks across all host platforms.
>>
>> If the Python rewrite cannot be merged for 1.1 then it makes sense to
>> go with shell portability fix.
> Hi Stefan,
>
> While you are looking at this, there are a couple of other related issues worth having in the back of your mind:
>
> 1. "self" is a reserved word in Solaris/Illumos trace, and it's used in a few trace calls.
> 2. "bool" isn't recognised by default, again used in a couple of traces ... could be fixed by typedef, but switching to int is probably better (imho)
> 3. Some work is needed on the linking stage when using Solaris/Illumos dtrace. You need to provide all the objs to the dtrace -G call in order to get an object generated that includes all of the relevant symbols in it. Happy to provide more detail if needed.
"Reviewing recent tracing patches" does not imply writing patches to fix
everyone's issues, you'll likely need to send patches for those issues
yourself at some point (or me if I find time, or someone else).

Stopping to use bool throughout QEMU is not an option IMO. If it's
limited to the DTrace backend code and doesn't negatively affect
SystemTap then that may be an option.

Andreas
Stefan Weil - March 19, 2012, 12:40 p.m.
Am 19.03.2012 13:05, schrieb Lee Essen:
> On 19 Mar 2012, at 11:59, Stefan Hajnoczi wrote:
>
>> On Fri, Mar 16, 2012 at 12:29 PM, Lee Essen 
>> <lee.essen@nowonline.co.uk> wrote:
>>> Signed-off-by: Lee Essen <lee.essen@nowonline.co.uk>
>>>
>>> ---
>>>
>>> scripts/tracetool | 2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> I'm going to spend some time today reviewing recent tracing patches.
>> I'd prefer to move to a Python version of tracetool rather than worry
>> about the shell quirks across all host platforms.
>>
>> If the Python rewrite cannot be merged for 1.1 then it makes sense to
>> go with shell portability fix.
>
> Hi Stefan,
>
> While you are looking at this, there are a couple of other related 
> issues worth having in the back of your mind:
>
> 1. "self" is a reserved word in Solaris/Illumos trace, and it's used 
> in a few trace calls.
> 2. "bool" isn't recognised by default, again used in a couple of 
> traces ... could be fixed by typedef, but switching to int is probably 
> better (imho)

No. The correct solution is including stdbool.h which is provided by the
compiler (gcc) and which defines bool and its values false and true.

More boolean variables should use bool instead of int, because that improves
readability of the code and saves memory in structures.

Regards,
Stefan W.
Andreas Färber - March 19, 2012, 12:49 p.m.
Am 19.03.2012 13:40, schrieb Stefan Weil:
> Am 19.03.2012 13:05, schrieb Lee Essen:
>> On 19 Mar 2012, at 11:59, Stefan Hajnoczi wrote:
>>
>>> On Fri, Mar 16, 2012 at 12:29 PM, Lee Essen
>>> <lee.essen@nowonline.co.uk> wrote:
>>>> Signed-off-by: Lee Essen <lee.essen@nowonline.co.uk>
>>>>
>>>> ---
>>>>
>>>> scripts/tracetool | 2 +-
>>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> I'm going to spend some time today reviewing recent tracing patches.
>>> I'd prefer to move to a Python version of tracetool rather than worry
>>> about the shell quirks across all host platforms.
>>>
>>> If the Python rewrite cannot be merged for 1.1 then it makes sense to
>>> go with shell portability fix.
>>
>> Hi Stefan,
>>
>> While you are looking at this, there are a couple of other related
>> issues worth having in the back of your mind:
>>
>> 1. "self" is a reserved word in Solaris/Illumos trace, and it's used
>> in a few trace calls.
>> 2. "bool" isn't recognised by default, again used in a couple of
>> traces ... could be fixed by typedef, but switching to int is
>> probably better (imho)
>
> No. The correct solution is including stdbool.h which is provided by the
> compiler (gcc) and which defines bool and its values false and true.
AFAIU the issue is not with C code but with D code:

http://docs.oracle.com/cd/E19253-01/817-6223/chp-typeopexpr-2/index.html

Andreas

>
>
> More boolean variables should use bool instead of int, because that
> improves
> readability of the code and saves memory in structures.
>
> Regards,
> Stefan W.
>
Lee Essen - March 19, 2012, 1:35 p.m.
On 19 Mar 2012, at 12:32, Andreas Färber wrote:

> Am 19.03.2012 13:05, schrieb Lee Essen:
>> On 19 Mar 2012, at 11:59, Stefan Hajnoczi wrote:
>> 
>>> I'm going to spend some time today reviewing recent tracing patches.
>>> I'd prefer to move to a Python version of tracetool rather than worry
>>> about the shell quirks across all host platforms.
>>> 
>>> If the Python rewrite cannot be merged for 1.1 then it makes sense to
>>> go with shell portability fix.
>> Hi Stefan,
>> 
>> While you are looking at this, there are a couple of other related issues worth having in the back of your mind:
>> 
>> 1. "self" is a reserved word in Solaris/Illumos trace, and it's used in a few trace calls.
>> 2. "bool" isn't recognised by default, again used in a couple of traces ... could be fixed by typedef, but switching to int is probably better (imho)
>> 3. Some work is needed on the linking stage when using Solaris/Illumos dtrace. You need to provide all the objs to the dtrace -G call in order to get an object generated that includes all of the relevant symbols in it. Happy to provide more detail if needed.
> "Reviewing recent tracing patches" does not imply writing patches to fix
> everyone's issues, you'll likely need to send patches for those issues
> yourself at some point (or me if I find time, or someone else).
> 
> Stopping to use bool throughout QEMU is not an option IMO. If it's
> limited to the DTrace backend code and doesn't negatively affect
> SystemTap then that may be an option.


In my original (way-too-long) patch for Illumos I included fixes for both of the above, the approach I took was to map "bool" to "int" and "self" to "_self" in the dtrace part of tracetool. So it should have had no impact on any non-dtrace stuff.

If that seems like a sensible approach I will look again, and at the linking issues.

Lee.
Stefan Hajnoczi - March 20, 2012, 4:59 p.m.
On Mon, Mar 19, 2012 at 1:35 PM, Lee Essen <lee.essen@nowonline.co.uk> wrote:
> On 19 Mar 2012, at 12:32, Andreas Färber wrote:
>
>> Am 19.03.2012 13:05, schrieb Lee Essen:
>>> On 19 Mar 2012, at 11:59, Stefan Hajnoczi wrote:
>>>
>>>> I'm going to spend some time today reviewing recent tracing patches.
>>>> I'd prefer to move to a Python version of tracetool rather than worry
>>>> about the shell quirks across all host platforms.
>>>>
>>>> If the Python rewrite cannot be merged for 1.1 then it makes sense to
>>>> go with shell portability fix.
>>> Hi Stefan,
>>>
>>> While you are looking at this, there are a couple of other related issues worth having in the back of your mind:
>>>
>>> 1. "self" is a reserved word in Solaris/Illumos trace, and it's used in a few trace calls.
>>> 2. "bool" isn't recognised by default, again used in a couple of traces ... could be fixed by typedef, but switching to int is probably better (imho)
>>> 3. Some work is needed on the linking stage when using Solaris/Illumos dtrace. You need to provide all the objs to the dtrace -G call in order to get an object generated that includes all of the relevant symbols in it. Happy to provide more detail if needed.
>> "Reviewing recent tracing patches" does not imply writing patches to fix
>> everyone's issues, you'll likely need to send patches for those issues
>> yourself at some point (or me if I find time, or someone else).
>>
>> Stopping to use bool throughout QEMU is not an option IMO. If it's
>> limited to the DTrace backend code and doesn't negatively affect
>> SystemTap then that may be an option.
>
>
> In my original (way-too-long) patch for Illumos I included fixes for both of the above, the approach I took was to map "bool" to "int" and "self" to "_self" in the dtrace part of tracetool. So it should have had no impact on any non-dtrace stuff.
>
> If that seems like a sensible approach I will look again, and at the linking issues.

That sounds like it can work.

Stefan
Lee Essen - March 20, 2012, 5:11 p.m.
On 20/03/2012 16:59, Stefan Hajnoczi wrote:
> On Mon, Mar 19, 2012 at 1:35 PM, Lee Essen<lee.essen@nowonline.co.uk>  wrote:
>> On 19 Mar 2012, at 12:32, Andreas Färber wrote:
>>
>>> Am 19.03.2012 13:05, schrieb Lee Essen:
>>> In my original (way-too-long) patch for Illumos I included fixes for 
>>> both of the above, the approach I took was to map "bool" to "int" 
>>> and "self" to "_self" in the dtrace part of tracetool. So it should 
>>> have had no impact on any non-dtrace stuff. If that seems like a 
>>> sensible approach I will look again, and at the linking issues. 
> That sounds like it can work.
>
> Stefan
There are only 5 bool's and 4 self's in the whole of the trace-events 
file ... is it easier to "fix" in there, or do you think a change to 
tracetool is the most appropriate way?

Lee.
Stefan Hajnoczi - March 21, 2012, 9:52 a.m.
On Tue, Mar 20, 2012 at 5:11 PM, Lee Essen <lee.essen@nowonline.co.uk> wrote:
> On 20/03/2012 16:59, Stefan Hajnoczi wrote:
>>
>> On Mon, Mar 19, 2012 at 1:35 PM, Lee Essen<lee.essen@nowonline.co.uk>
>>  wrote:
>>>
>>> On 19 Mar 2012, at 12:32, Andreas Färber wrote:
>>>
>>>> Am 19.03.2012 13:05, schrieb Lee Essen:
>>>> In my original (way-too-long) patch for Illumos I included fixes for
>>>> both of the above, the approach I took was to map "bool" to "int" and "self"
>>>> to "_self" in the dtrace part of tracetool. So it should have had no impact
>>>> on any non-dtrace stuff. If that seems like a sensible approach I will look
>>>> again, and at the linking issues.
>>
>> That sounds like it can work.
>>
>> Stefan
>
> There are only 5 bool's and 4 self's in the whole of the trace-events file
> ... is it easier to "fix" in there, or do you think a change to tracetool is
> the most appropriate way?

Folks will add more bool and maybe "self" in the future, so updating
./trace-events is not a permanent solution.

tracetool has the ability to do two things:

1. It can reject invalid trace-events input.

2. It can fudge the generated trace backend code so that "self" gets
turned into "self_", for example.

#2 is nice when we can do it without confusing users.

I suggest modifying tracetool.  You may wish to hold off on this for a
week because we're about to merge the Python version of tracetool,
which makes it much more portable.

Stefan

Patch

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()