diff mbox

[v4,07/11] trace: [tracetool] Rewrite event argument parsing

Message ID 20120210115508.9787.26299.stgit@ginnungagap.bsc.es
State New
Headers show

Commit Message

Lluís Vilanova Feb. 10, 2012, 11:55 a.m. UTC
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 scripts/tracetool.py |  190 ++++++++++++++++++++++++--------------------------
 1 files changed, 91 insertions(+), 99 deletions(-)

Comments

Harsh Prateek Bora Feb. 13, 2012, 8:06 a.m. UTC | #1
On 02/10/2012 05:25 PM, Lluís Vilanova wrote:
> Signed-off-by: Lluís Vilanova<vilanova@ac.upc.edu>
> ---
>   scripts/tracetool.py |  190 ++++++++++++++++++++++++--------------------------
>   1 files changed, 91 insertions(+), 99 deletions(-)
>
> diff --git a/scripts/tracetool.py b/scripts/tracetool.py
> index f2bcb65..cd1c29d 100755
> --- a/scripts/tracetool.py
> +++ b/scripts/tracetool.py
> @@ -38,49 +38,6 @@ Options:
>   '''
>       sys.exit(1)
>
> -def get_argnames(args):
> -    nfields = 0
> -    str = []
> -    for field in args.split():
> -      nfields = nfields + 1
> -      # Drop pointer star
> -      type, ptr, tail = field.partition('*')
> -      if type != field:
> -        field = tail
> -
> -      name, sep, tail = field.partition(',')
> -
> -      if name == field:
> -        continue
> -      str.append(name)
> -      str.append(", ")
> -
> -    if nfields>  1:
> -      str.append(name)
> -      return ''.join(str)
> -    else:
> -      return ''
> -
> -def calc_sizeofargs(args, argc):
> -    strtype = ('const char*', 'char*', 'const char *', 'char *')
> -    str = []
> -    newstr = ""
> -    if argc>  0:
> -      str = args.split(',')
> -      for elem in str:
> -        if elem.lstrip().startswith(strtype): #strings
> -          type, sep, var = elem.rpartition('*')
> -          newstr = newstr+"4 + strlen("+var.lstrip()+") + "
> -        #elif '*' in elem:
> -        #  newstr = newstr + "4 + " # pointer vars
> -        else:
> -          #type, sep, var = elem.rpartition(' ')
> -          #newstr = newstr+"sizeof("+type.lstrip()+") + "
> -          newstr = newstr + '8 + '
> -    newstr = newstr + '0' # for last +
> -    return newstr
> -
> -
>   def trace_h_begin():
>       print '''#ifndef TRACE_H
>   #define TRACE_H
> @@ -133,13 +90,6 @@ def simple_h(events):
>
>       return
>
> -def is_string(arg):
> -    strtype = ('const char*', 'char*', 'const char *', 'char *')
> -    if arg.lstrip().startswith(strtype):
> -        return True
> -    else:
> -        return False
> -
>   def simple_c(events):
>       rec_off = 0
>       print '#include "trace.h"'
> @@ -154,8 +104,16 @@ def simple_c(events):
>           print
>       print '};'
>       print
> +
>       for num, event in enumerate(events):
> -        argc = event.argc
> +        sizes = []
> +        for type_, name in event.args:
> +            if type_is_string(type_):
> +                sizes.append("4 + strlen(%s)" % name)



> +            else:
> +                sizes.append("8 + sizeof(%s)" % type_)
> +        sizestr = " + ".join(sizes)
> +

Applied with a small change as reqd:

+            else:
+                sizes.append("8")
+        sizestr = " + ".join(sizes)
+        if len(event.args) == 0:
+            sizestr = '0'
+

BTW, I am manually applying your changes on top of my patches as there 
were significant changes in my patches also. I will include your patches 
in my next series.

- Harsh


>           print '''void trace_%(name)s(%(args)s)
>   {
>       unsigned int tbuf_idx, rec_off __attribute__((unused));
> @@ -166,52 +124,52 @@ def simple_c(events):
>       if (!trace_list[%(event_id)s].state) {
>           return;
>       }
> +
> +    tbuf_idx = trace_alloc_record(%(event_id)s, %(sizestr)s);
> +    rec_off = (tbuf_idx + ST_V2_REC_HDR_LEN) %% TRACE_BUF_LEN; /* seek record header */
>   ''' % {
>       'name': event.name,
>       'args': event.args,
>       'event_id': num,
> +    'sizestr' : sizestr,
>   }
> -        print '''
> -    tbuf_idx = trace_alloc_record(%(event_id)s, %(sizestr)s);
> -    rec_off = (tbuf_idx + ST_V2_REC_HDR_LEN) %% TRACE_BUF_LEN; /* seek record header */
> -''' % {'event_id': num, 'sizestr': event.sizestr,}
>
> -        if argc>  0:
> -            str = event.arglist
> -            for elem in str:
> -                if is_string(elem): # if string
> -                    type, sep, var = elem.rpartition('*')
> +        if len(event.args)>  0:
> +            for type_, name in event.args:
> +                # string
> +                if type_is_string(type_):
>                       print '''
> -    slen = strlen(%(var)s);
> +    slen = strlen(%(name)s);
>       write_to_buffer(rec_off, (uint8_t*)&slen, sizeof(slen));
>       rec_off += sizeof(slen);''' % {
> -    'var': var.lstrip()
> +    'name': name
>   }
>                       print '''
> -    write_to_buffer(rec_off, (uint8_t*)%(var)s, slen);
> +    write_to_buffer(rec_off, (uint8_t*)%(name)s, slen);
>       rec_off += slen;''' % {
> -    'var': var.lstrip()
> +    'name': name
>   }
> -                elif '*' in elem: # pointer var (not string)
> -                    type, sep, var = elem.rpartition('*')
> +                # pointer var (not string)
> +                elif type_.endswith('*'):
>                       print '''
> -    pvar64 = (uint64_t)(uint64_t*)%(var)s;
> +    pvar64 = (uint64_t)(uint64_t*)%(name)s;
>       write_to_buffer(rec_off, (uint8_t*)&pvar64, sizeof(uint64_t));
>       rec_off += sizeof(uint64_t);''' % {
> -    'var': var.lstrip()
> +    'name': name
>   }
> -                else: # primitive data type
> -                    type, sep, var = elem.rpartition(' ')
> +                # primitive data type
> +                else:
>                       print '''
> -    var64 = (uint64_t)%(var)s;
> +    var64 = (uint64_t)%(name)s;
>       write_to_buffer(rec_off, (uint8_t*)&var64, sizeof(uint64_t));
>       rec_off += sizeof(uint64_t);''' % {
> -    'var': var.lstrip()
> +    'name': name
>   }
>           print '''
> -    trace_mark_record_complete(tbuf_idx);'''
> -        print '}'
> -        print
> +    trace_mark_record_complete(tbuf_idx);
> +}
> +
> +'''
>
>       return
>
> @@ -220,12 +178,11 @@ def stderr_h(events):
>   #include "trace/stderr.h"
>
>   extern TraceEvent trace_list[];'''
> +
>       for num, event in enumerate(events):
> -        argnames = event.argnames
> -        if event.argc>  0:
> -            argnames = ', ' + event.argnames
> -        else:
> -            argnames = ''
> +        argnames = ", ".join(event.args.names())
> +        if len(event.args)>  0:
> +            argnames = ", "+argnames
>           print '''
>   static inline void trace_%(name)s(%(args)s)
>   {
> @@ -262,13 +219,13 @@ def ust_h(events):
>   #undef wmb'''
>
>       for event in events:
> -        if event.argc>  0:
> +        if len(event.args)>  0:
>               print '''
>   DECLARE_TRACE(ust_%(name)s, TP_PROTO(%(args)s), TP_ARGS(%(argnames)s));
>   #define trace_%(name)s trace_ust_%(name)s''' % {
>       'name': event.name,
>       'args': event.args,
> -    'argnames': event.argnames
> +    'argnames': ", ".join(event.args.names())
>   }
>           else:
>               print '''
> @@ -287,9 +244,9 @@ def ust_c(events):
>   #undef wmb
>   #include "trace.h"'''
>       for event in events:
> -        argnames = event.argnames
> -        if event.argc>  0:
> -            argnames = ', ' + event.argnames
> +        argnames = ", ".join(event.args.names())
> +        if len(event.args)>  0:
> +            argnames = ', ' + argnames
>               print '''
>   DEFINE_TRACE(ust_%(name)s);
>
> @@ -339,7 +296,7 @@ def dtrace_h(events):
>       'name': event.name,
>       'args': event.args,
>       'uppername': event.name.upper(),
> -    'argnames': event.argnames,
> +    'argnames': ", ".join(event.args.names()),
>   }
>
>   def dtrace_c(events):
> @@ -379,12 +336,12 @@ probe %(probeprefix)s.%(name)s = process("%(binary)s").mark("%(name)s")
>       'binary': binary
>   }
>           i = 1
> -        if event.argc>  0:
> -            for arg in event.argnames.split(','):
> +        if len(event.args)>  0:
> +            for name in event.args.names():
>                   # 'limit' is a reserved keyword
> -                if arg == 'limit':
> -                    arg = '_limit'
> -                print '  %s = $arg%d;' % (arg.lstrip(), i)
> +                if name == 'limit':
> +                    name = '_limit'
> +                print '  %s = $arg%d;' % (name.lstrip(), i)
>                   i += 1
>           print '}'
>       print
> @@ -478,6 +435,47 @@ trace_gen = {
>       },
>   }
>
> +# Event arguments
> +def type_is_string(type_):
> +    strtype = ('const char*', 'char*', 'const char *', 'char *')
> +    return type_.startswith(strtype)
> +
> +class Arguments:
> +    def __init__ (self, arg_str):
> +        self._args = []
> +        for arg in arg_str.split(","):
> +            arg = arg.strip()
> +            parts = arg.split()
> +            head, sep, tail = parts[-1].rpartition("*")
> +            parts = parts[:-1]
> +            if tail == "void":
> +                assert len(parts) == 0 and sep == ""
> +                continue
> +            arg_type = " ".join(parts + [ " ".join([head, sep]).strip() ]).strip()
> +            self._args.append((arg_type, tail))
> +
> +    def __iter__(self):
> +        return iter(self._args)
> +
> +    def __len__(self):
> +        return len(self._args)
> +
> +    def __str__(self):
> +        if len(self._args) == 0:
> +            return "void"
> +        else:
> +            return ", ".join([ " ".join([t, n]) for t,n in self._args ])
> +
> +    def names(self):
> +        return [ name for _, name in self._args ]
> +
> +    def types(self):
> +        return [ type_ for type_, _ in self._args ]
> +
> +    def size_str(self):
> +        res = ""
> +        return res
> +
>   # A trace event
>   import re
>   cre = re.compile("((?P<props>.*)\s+)?(?P<name>[^(\s]+)\((?P<args>[^)]*)\)\s*(?P<fmt>\".*)?")
> @@ -489,17 +487,11 @@ class Event(object):
>           m = cre.match(line)
>           assert m is not None
>           groups = m.groupdict('')
> -        self.args = groups["args"]
> -        self.arglist = self.args.split(',')
>           self.name = groups["name"]
> -        if len(self.arglist) == 1 and self.arglist[0] == "void":
> -            self.argc = 0
> -        else:
> -            self.argc = len(self.arglist)
> -        self.argnames = get_argnames(self.args)
> -        self.sizestr = calc_sizeofargs(self.args, self.argc)
>           self.fmt = groups["fmt"]
>           self.properties = groups["props"].split()
> +        self.args = Arguments(groups["args"])
> +
>           unknown_props = set(self.properties) - VALID_PROPS
>           if len(unknown_props)>  0:
>               raise ValueError("Unknown properties: %s" % ", ".join(unknown_props))
>
Lluís Vilanova Feb. 13, 2012, 3:17 p.m. UTC | #2
Harsh Bora writes:

> BTW, I am manually applying your changes on top of my patches as there were
> significant changes in my patches also. I will include your patches in my next
> series.

Excellent.


Lluis
diff mbox

Patch

diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index f2bcb65..cd1c29d 100755
--- a/scripts/tracetool.py
+++ b/scripts/tracetool.py
@@ -38,49 +38,6 @@  Options:
 '''
     sys.exit(1)
 
-def get_argnames(args):
-    nfields = 0
-    str = []
-    for field in args.split():
-      nfields = nfields + 1
-      # Drop pointer star
-      type, ptr, tail = field.partition('*')
-      if type != field:
-        field = tail
-
-      name, sep, tail = field.partition(',')
-
-      if name == field:
-        continue
-      str.append(name)
-      str.append(", ")
-
-    if nfields > 1:
-      str.append(name)
-      return ''.join(str)
-    else:
-      return ''
-
-def calc_sizeofargs(args, argc):
-    strtype = ('const char*', 'char*', 'const char *', 'char *')
-    str = []
-    newstr = ""
-    if argc > 0:
-      str = args.split(',')
-      for elem in str:
-        if elem.lstrip().startswith(strtype): #strings
-          type, sep, var = elem.rpartition('*')
-          newstr = newstr+"4 + strlen("+var.lstrip()+") + "
-        #elif '*' in elem:
-        #  newstr = newstr + "4 + " # pointer vars
-        else:
-          #type, sep, var = elem.rpartition(' ')
-          #newstr = newstr+"sizeof("+type.lstrip()+") + "
-          newstr = newstr + '8 + '
-    newstr = newstr + '0' # for last +
-    return newstr
-
-
 def trace_h_begin():
     print '''#ifndef TRACE_H
 #define TRACE_H
@@ -133,13 +90,6 @@  def simple_h(events):
 
     return
 
-def is_string(arg):
-    strtype = ('const char*', 'char*', 'const char *', 'char *')
-    if arg.lstrip().startswith(strtype):
-        return True
-    else:
-        return False
-
 def simple_c(events):
     rec_off = 0
     print '#include "trace.h"'
@@ -154,8 +104,16 @@  def simple_c(events):
         print
     print '};'
     print
+
     for num, event in enumerate(events):
-        argc = event.argc
+        sizes = []
+        for type_, name in event.args:
+            if type_is_string(type_):
+                sizes.append("4 + strlen(%s)" % name)
+            else:
+                sizes.append("8 + sizeof(%s)" % type_)
+        sizestr = " + ".join(sizes)
+
         print '''void trace_%(name)s(%(args)s)
 {
     unsigned int tbuf_idx, rec_off __attribute__((unused));
@@ -166,52 +124,52 @@  def simple_c(events):
     if (!trace_list[%(event_id)s].state) {
         return;
     }
+
+    tbuf_idx = trace_alloc_record(%(event_id)s, %(sizestr)s);
+    rec_off = (tbuf_idx + ST_V2_REC_HDR_LEN) %% TRACE_BUF_LEN; /* seek record header */
 ''' % {
     'name': event.name,
     'args': event.args,
     'event_id': num,
+    'sizestr' : sizestr,
 }
-        print '''
-    tbuf_idx = trace_alloc_record(%(event_id)s, %(sizestr)s);
-    rec_off = (tbuf_idx + ST_V2_REC_HDR_LEN) %% TRACE_BUF_LEN; /* seek record header */
-''' % {'event_id': num, 'sizestr': event.sizestr,}
 
-        if argc > 0:
-            str = event.arglist
-            for elem in str:
-                if is_string(elem): # if string
-                    type, sep, var = elem.rpartition('*')
+        if len(event.args) > 0:
+            for type_, name in event.args:
+                # string
+                if type_is_string(type_):
                     print '''
-    slen = strlen(%(var)s);
+    slen = strlen(%(name)s);
     write_to_buffer(rec_off, (uint8_t*)&slen, sizeof(slen));
     rec_off += sizeof(slen);''' % {
-    'var': var.lstrip()
+    'name': name
 }
                     print '''
-    write_to_buffer(rec_off, (uint8_t*)%(var)s, slen);
+    write_to_buffer(rec_off, (uint8_t*)%(name)s, slen);
     rec_off += slen;''' % {
-    'var': var.lstrip()
+    'name': name
 }
-                elif '*' in elem: # pointer var (not string)
-                    type, sep, var = elem.rpartition('*')
+                # pointer var (not string)
+                elif type_.endswith('*'):
                     print '''
-    pvar64 = (uint64_t)(uint64_t*)%(var)s;
+    pvar64 = (uint64_t)(uint64_t*)%(name)s;
     write_to_buffer(rec_off, (uint8_t*)&pvar64, sizeof(uint64_t));
     rec_off += sizeof(uint64_t);''' % {
-    'var': var.lstrip()
+    'name': name
 }
-                else: # primitive data type
-                    type, sep, var = elem.rpartition(' ')
+                # primitive data type
+                else:
                     print '''
-    var64 = (uint64_t)%(var)s;
+    var64 = (uint64_t)%(name)s;
     write_to_buffer(rec_off, (uint8_t*)&var64, sizeof(uint64_t));
     rec_off += sizeof(uint64_t);''' % {
-    'var': var.lstrip()
+    'name': name
 }
         print '''
-    trace_mark_record_complete(tbuf_idx);'''
-        print '}'
-        print
+    trace_mark_record_complete(tbuf_idx);
+}
+
+'''
 
     return
 
@@ -220,12 +178,11 @@  def stderr_h(events):
 #include "trace/stderr.h"
 
 extern TraceEvent trace_list[];'''
+
     for num, event in enumerate(events):
-        argnames = event.argnames
-        if event.argc > 0:
-            argnames = ', ' + event.argnames
-        else:
-            argnames = ''
+        argnames = ", ".join(event.args.names())
+        if len(event.args) > 0:
+            argnames = ", "+argnames
         print '''
 static inline void trace_%(name)s(%(args)s)
 {
@@ -262,13 +219,13 @@  def ust_h(events):
 #undef wmb'''
 
     for event in events:
-        if event.argc > 0:
+        if len(event.args) > 0:
             print '''
 DECLARE_TRACE(ust_%(name)s, TP_PROTO(%(args)s), TP_ARGS(%(argnames)s));
 #define trace_%(name)s trace_ust_%(name)s''' % {
     'name': event.name,
     'args': event.args,
-    'argnames': event.argnames
+    'argnames': ", ".join(event.args.names())
 }
         else:
             print '''
@@ -287,9 +244,9 @@  def ust_c(events):
 #undef wmb
 #include "trace.h"'''
     for event in events:
-        argnames = event.argnames
-        if event.argc > 0:
-            argnames = ', ' + event.argnames
+        argnames = ", ".join(event.args.names())
+        if len(event.args) > 0:
+            argnames = ', ' + argnames
             print '''
 DEFINE_TRACE(ust_%(name)s);
 
@@ -339,7 +296,7 @@  def dtrace_h(events):
     'name': event.name,
     'args': event.args,
     'uppername': event.name.upper(),
-    'argnames': event.argnames,
+    'argnames': ", ".join(event.args.names()),
 }
 
 def dtrace_c(events):
@@ -379,12 +336,12 @@  probe %(probeprefix)s.%(name)s = process("%(binary)s").mark("%(name)s")
     'binary': binary
 }
         i = 1
-        if event.argc > 0:
-            for arg in event.argnames.split(','):
+        if len(event.args) > 0:
+            for name in event.args.names():
                 # 'limit' is a reserved keyword
-                if arg == 'limit':
-                    arg = '_limit'
-                print '  %s = $arg%d;' % (arg.lstrip(), i)
+                if name == 'limit':
+                    name = '_limit'
+                print '  %s = $arg%d;' % (name.lstrip(), i)
                 i += 1
         print '}'
     print
@@ -478,6 +435,47 @@  trace_gen = {
     },
 }
 
+# Event arguments
+def type_is_string(type_):
+    strtype = ('const char*', 'char*', 'const char *', 'char *')
+    return type_.startswith(strtype)
+
+class Arguments:
+    def __init__ (self, arg_str):
+        self._args = []
+        for arg in arg_str.split(","):
+            arg = arg.strip()
+            parts = arg.split()
+            head, sep, tail = parts[-1].rpartition("*")
+            parts = parts[:-1]
+            if tail == "void":
+                assert len(parts) == 0 and sep == ""
+                continue
+            arg_type = " ".join(parts + [ " ".join([head, sep]).strip() ]).strip()
+            self._args.append((arg_type, tail))
+
+    def __iter__(self):
+        return iter(self._args)
+
+    def __len__(self):
+        return len(self._args)
+
+    def __str__(self):
+        if len(self._args) == 0:
+            return "void"
+        else:
+            return ", ".join([ " ".join([t, n]) for t,n in self._args ])
+
+    def names(self):
+        return [ name for _, name in self._args ]
+
+    def types(self):
+        return [ type_ for type_, _ in self._args ]
+
+    def size_str(self):
+        res = ""
+        return res
+
 # A trace event
 import re
 cre = re.compile("((?P<props>.*)\s+)?(?P<name>[^(\s]+)\((?P<args>[^)]*)\)\s*(?P<fmt>\".*)?")
@@ -489,17 +487,11 @@  class Event(object):
         m = cre.match(line)
         assert m is not None
         groups = m.groupdict('')
-        self.args = groups["args"]
-        self.arglist = self.args.split(',')
         self.name = groups["name"]
-        if len(self.arglist) == 1 and self.arglist[0] == "void":
-            self.argc = 0
-        else:
-            self.argc = len(self.arglist)
-        self.argnames = get_argnames(self.args)
-        self.sizestr = calc_sizeofargs(self.args, self.argc)
         self.fmt = groups["fmt"]
         self.properties = groups["props"].split()
+        self.args = Arguments(groups["args"])
+
         unknown_props = set(self.properties) - VALID_PROPS
         if len(unknown_props) > 0:
             raise ValueError("Unknown properties: %s" % ", ".join(unknown_props))