[Qemu-devel] [PATCH trivial] trace-events: don't use dtrace reserved words in and next

[Qemu-devel] [PATCH trivial] trace-events: don't use dtrace reserved words in and next

[Alon Levy]

---
 trace-events |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/trace-events b/trace-events
index 70f059d..0924666 100644
--- a/trace-events
+++ b/trace-events
@@ -313,7 +313,7 @@ usb_host_set_interface(int bus, int addr, int interface, int alt) "dev %d:%d, in
 usb_host_claim_interfaces(int bus, int addr, int config, int nif) "dev %d:%d, config %d, nif %d"
 usb_host_release_interfaces(int bus, int addr) "dev %d:%d"
 usb_host_req_control(int bus, int addr, int req, int value, int index) "dev %d:%d, req 0x%x, value %d, index %d"
-usb_host_req_data(int bus, int addr, int in, int ep, int size) "dev %d:%d, in %d, ep %d, size %d"
+usb_host_req_data(int bus, int addr, int _in, int ep, int size) "dev %d:%d, in %d, ep %d, size %d"
 usb_host_req_complete(int bus, int addr, int status) "dev %d:%d, status %d"
 usb_host_urb_submit(int bus, int addr, void *aurb, int length, int more) "dev %d:%d, aurb %p, length %d, more %d"
 usb_host_urb_complete(int bus, int addr, void *aurb, int status, int length, int more) "dev %d:%d, aurb %p, status %d, length %d, more %d"
@@ -562,7 +562,7 @@ qemu_coroutine_terminate(void *co) "self %p"
 
 # qemu-coroutine-lock.c
 qemu_co_queue_next_bh(void) ""
-qemu_co_queue_next(void *next) "next %p"
+qemu_co_queue_next(void *_next) "next %p"
 qemu_co_mutex_lock_entry(void *mutex, void *self) "mutex %p self %p"
 qemu_co_mutex_lock_return(void *mutex, void *self) "mutex %p self %p"
 qemu_co_mutex_unlock_entry(void *mutex, void *self) "mutex %p self %p"
-- 
1.7.9.3

Re: [Qemu-devel] [PATCH trivial] trace-events: don't use dtrace reserved words in and next

[Stefan Hajnoczi]

On Wed, Mar 28, 2012 at 8:55 AM, Alon Levy <alevy@redhat.com> wrote:
> ---
>  trace-events |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

We're always going to struggle with new trace events that add reserved
keywords.  Fixing up trace-events is going to be an ongoing task so
I'd like to also see tracetool automatically modify keywords 'next' ->
'next_'.  Do you want to send a patch to do that instead?

Stefan

Re: [Qemu-devel] [PATCH trivial] trace-events: don't use dtrace reserved words in and next

[Alon Levy]

On Wed, Mar 28, 2012 at 01:55:49PM +0100, Stefan Hajnoczi wrote:
> On Wed, Mar 28, 2012 at 8:55 AM, Alon Levy <alevy@redhat.com> wrote:
> > ---
> >  trace-events |    4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> We're always going to struggle with new trace events that add reserved
> keywords.  Fixing up trace-events is going to be an ongoing task so
> I'd like to also see tracetool automatically modify keywords 'next' ->
> 'next_'.  Do you want to send a patch to do that instead?

sure. For the new tracetool.py too ? :)

> 
> Stefan
> 

[Qemu-trivial] [PATCH 1/2] tracetool: dtrace: handle in and next reserved words

[Alon Levy]

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 scripts/tracetool |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/scripts/tracetool b/scripts/tracetool
index 65bd0a1..e7cebf3 100755
--- a/scripts/tracetool
+++ b/scripts/tracetool
@@ -494,9 +494,9 @@ EOF
     i=1
     for arg in $arglist
     do
-        # 'limit' is a reserved keyword
-        if [ "$arg" = "limit" ]; then
-          arg="_limit"
+        # 'limit', 'in' and 'next' are reserved keywords
+        if [ "$arg" = "limit" -o "$arg" = "in" -o "$arg" = "next" ]; then
+          arg="_$arg"
         fi
         cat <<EOF
   $arg = \$arg$i;
-- 
1.7.9.3

[Qemu-trivial] [PATCH 2/2] tracetool: dtrace: warn on reserved word usage

[Alon Levy]

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 scripts/tracetool |    1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/tracetool b/scripts/tracetool
index e7cebf3..d011bb7 100755
--- a/scripts/tracetool
+++ b/scripts/tracetool
@@ -496,6 +496,7 @@ EOF
     do
         # 'limit', 'in' and 'next' are reserved keywords
         if [ "$arg" = "limit" -o "$arg" = "in" -o "$arg" = "next" ]; then
+          echo "reserved word used in line: $1" 1>&2
           arg="_$arg"
         fi
         cat <<EOF
-- 
1.7.9.3

Re: [Qemu-devel] [PATCH trivial] trace-events: don't use dtrace reserved words in and next

[Alon Levy]

On Wed, Mar 28, 2012 at 03:16:34PM +0200, Alon Levy wrote:
> On Wed, Mar 28, 2012 at 01:55:49PM +0100, Stefan Hajnoczi wrote:
> > On Wed, Mar 28, 2012 at 8:55 AM, Alon Levy <alevy@redhat.com> wrote:
> > > ---
> > >  trace-events |    4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > We're always going to struggle with new trace events that add reserved
> > keywords.  Fixing up trace-events is going to be an ongoing task so
> > I'd like to also see tracetool automatically modify keywords 'next' ->
> > 'next_'.  Do you want to send a patch to do that instead?
> 

hmm, I changed to _next, not next_, if you'd like to change that and
commit I don't mind.

> sure. For the new tracetool.py too ? :)
> 
> > 
> > Stefan
> > 
> 

Re: [Qemu-devel] [PATCH trivial] trace-events: don't use dtrace reserved words in and next

[Stefan Hajnoczi]

On Wed, Mar 28, 2012 at 2:33 PM, Alon Levy <alevy@redhat.com> wrote:
> On Wed, Mar 28, 2012 at 03:16:34PM +0200, Alon Levy wrote:
>> On Wed, Mar 28, 2012 at 01:55:49PM +0100, Stefan Hajnoczi wrote:
>> > On Wed, Mar 28, 2012 at 8:55 AM, Alon Levy <alevy@redhat.com> wrote:
>> > > ---
>> > >  trace-events |    4 ++--
>> > >  1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > We're always going to struggle with new trace events that add reserved
>> > keywords.  Fixing up trace-events is going to be an ongoing task so
>> > I'd like to also see tracetool automatically modify keywords 'next' ->
>> > 'next_'.  Do you want to send a patch to do that instead?
>>
>
> hmm, I changed to _next, not next_, if you'd like to change that and
> commit I don't mind.

Thanks.  next_ can be a little safer when languages reserve
identifiers starting with '_'.  I can tweak it.

Stefan

[Qemu-devel] [PATCH 1/2] tracetool: dtrace: handle in and next reserved words

[Alon Levy]

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 scripts/tracetool |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/scripts/tracetool b/scripts/tracetool
index 65bd0a1..e7cebf3 100755
--- a/scripts/tracetool
+++ b/scripts/tracetool
@@ -494,9 +494,9 @@ EOF
     i=1
     for arg in $arglist
     do
-        # 'limit' is a reserved keyword
-        if [ "$arg" = "limit" ]; then
-          arg="_limit"
+        # 'limit', 'in' and 'next' are reserved keywords
+        if [ "$arg" = "limit" -o "$arg" = "in" -o "$arg" = "next" ]; then
+          arg="_$arg"
         fi
         cat <<EOF
   $arg = \$arg$i;
-- 
1.7.9.3

[Qemu-devel] [PATCH 2/2] tracetool: dtrace: warn on reserved word usage

[Alon Levy]

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 scripts/tracetool |    1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/tracetool b/scripts/tracetool
index e7cebf3..d011bb7 100755
--- a/scripts/tracetool
+++ b/scripts/tracetool
@@ -496,6 +496,7 @@ EOF
     do
         # 'limit', 'in' and 'next' are reserved keywords
         if [ "$arg" = "limit" -o "$arg" = "in" -o "$arg" = "next" ]; then
+          echo "reserved word used in line: $1" 1>&2
           arg="_$arg"
         fi
         cat <<EOF
-- 
1.7.9.3

Re: [Qemu-devel] [PATCH 1/2] tracetool: dtrace: handle in and next reserved words

[Lee Essen]

On 28 Mar 2012, at 15:26, Alon Levy <alevy@redhat.com> wrote:

> Signed-off-by: Alon Levy <alevy@redhat.com>
> ---
> scripts/tracetool |    6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/tracetool b/scripts/tracetool
> index 65bd0a1..e7cebf3 100755
> --- a/scripts/tracetool
> +++ b/scripts/tracetool
> @@ -494,9 +494,9 @@ EOF
>     i=1
>     for arg in $arglist
>     do
> -        # 'limit' is a reserved keyword
> -        if [ "$arg" = "limit" ]; then
> -          arg="_limit"
> +        # 'limit', 'in' and 'next' are reserved keywords
> +        if [ "$arg" = "limit" -o "$arg" = "in" -o "$arg" = "next" ]; then
> +          arg="_$arg"
>         fi
>         cat <<EOF
>   $arg = \$arg$i;
> -- 
> 1.7.9.3

Could we add 'self' to this list?

Lee.

Re: [Qemu-devel] [PATCH 1/2] tracetool: dtrace: handle in and next reserved words

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 711 bytes --]

On 03/28/2012 10:26 AM, Lee Essen wrote:

>> +        # 'limit', 'in' and 'next' are reserved keywords
>> +        if [ "$arg" = "limit" -o "$arg" = "in" -o "$arg" = "next" ]; then

[ ... -o ... ] is not portable.  POSIX says you must break it into:
[ ... ] || [ ... ]

or, more efficiently for this particular filtering, rewrite this as:

# munge reserved words
case $arg in
  limit | in | next ) arg=_$arg ;;
esac

> 
> Could we add 'self' to this list?

and using case makes it easier to add more reserved words:

case $arg in
  limit | in | next | self ) arg=_$arg ;;
esac

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

Re: [Qemu-devel] [PATCH 1/2] tracetool: dtrace: handle in and next reserved words

[Peter Maydell]

2012/3/28 Alon Levy <alevy@redhat.com>:
> -        # 'limit' is a reserved keyword
> -        if [ "$arg" = "limit" ]; then
> -          arg="_limit"
> +        # 'limit', 'in' and 'next' are reserved keywords
> +        if [ "$arg" = "limit" -o "$arg" = "in" -o "$arg" = "next" ]; then
> +          arg="_$arg"
>         fi

The test -a and -o options are obsolete in POSIX shell scripts
(use "if [ foo ] && [ bar ]" instead).

In this case it would be neater to use 'case' anyway:
    case "$arg" in
        limit|in|next)
            arg="_$arg"
            ;;
    esac

-- PMM

[Qemu-trivial] [PATCH v3 1/2] tracetool: dtrace: handle in and next reserved words

[Alon Levy]

Signed-off-by: Alon Levy <alevy@redhat.com>
---

I think I need a qemu-trivial-review too ;)

v3:
 added self    (Lee Essen)
 use post '_'  (Stefan Hajnoczi)
 use case/esca (Eric Blake & Peter Maydell)

 scripts/tracetool |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/scripts/tracetool b/scripts/tracetool
index 65bd0a1..ab788f2 100755
--- a/scripts/tracetool
+++ b/scripts/tracetool
@@ -494,10 +494,12 @@ EOF
     i=1
     for arg in $arglist
     do
-        # 'limit' is a reserved keyword
-        if [ "$arg" = "limit" ]; then
-          arg="_limit"
-        fi
+        # postfix reserved words with '_'
+        case "$arg" in
+            limit|in|next|self)
+                arg="${arg}_"
+                ;;
+        esac
         cat <<EOF
   $arg = \$arg$i;
 EOF
-- 
1.7.9.3

[Qemu-trivial] [PATCH 2/2] tracetool: dtrace: warn on reserved word usage

[Alon Levy]

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 scripts/tracetool |    1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/tracetool b/scripts/tracetool
index ab788f2..c2aed8f 100755
--- a/scripts/tracetool
+++ b/scripts/tracetool
@@ -497,6 +497,7 @@ EOF
         # postfix reserved words with '_'
         case "$arg" in
             limit|in|next|self)
+                echo "reserved word used in line: $1" 1>&2
                 arg="${arg}_"
                 ;;
         esac
-- 
1.7.9.3

Re: [Qemu-trivial] [PATCH v3 1/2] tracetool: dtrace: handle in and next reserved words

[Stefan Hajnoczi]

On Thu, Mar 29, 2012 at 10:35 PM, Alon Levy <alevy@redhat.com> wrote:
> Signed-off-by: Alon Levy <alevy@redhat.com>
> ---
>
> I think I need a qemu-trivial-review too ;)
>
> v3:
>  added self    (Lee Essen)
>  use post '_'  (Stefan Hajnoczi)
>  use case/esca (Eric Blake & Peter Maydell)
>
>  scripts/tracetool |   10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)

Thanks, I have merged this patch.

Stefan

Re: [Qemu-trivial] [PATCH 2/2] tracetool: dtrace: warn on reserved word usage

[Stefan Hajnoczi]

On Thu, Mar 29, 2012 at 10:35 PM, Alon Levy <alevy@redhat.com> wrote:
> Signed-off-by: Alon Levy <alevy@redhat.com>
> ---
>  scripts/tracetool |    1 +
>  1 file changed, 1 insertion(+)

I have not merged this patch because I don't want to avoid adding
warnings that are only visible when using a specific backend.

The problem is that offending trace-events lines will be added and
later on someone who uses dtrace will find a whole list of new
warnings.

Stefan