From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:37685) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RwqwE-0004RK-PK for qemu-devel@nongnu.org; Mon, 13 Feb 2012 03:07:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RwqwC-0001VR-Ts for qemu-devel@nongnu.org; Mon, 13 Feb 2012 03:07:06 -0500 Received: from e28smtp09.in.ibm.com ([122.248.162.9]:44118) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RwqwC-0001Uw-0P for qemu-devel@nongnu.org; Mon, 13 Feb 2012 03:07:04 -0500 Received: from /spool/local by e28smtp09.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 13 Feb 2012 13:36:58 +0530 Received: from d28av04.in.ibm.com (d28av04.in.ibm.com [9.184.220.66]) by d28relay04.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q1D86sg83330154 for ; Mon, 13 Feb 2012 13:36:55 +0530 Received: from d28av04.in.ibm.com (loopback [127.0.0.1]) by d28av04.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q1D86r8J028191 for ; Mon, 13 Feb 2012 19:06:54 +1100 Message-ID: <4F38C49C.90700@linux.vnet.ibm.com> Date: Mon, 13 Feb 2012 13:36:52 +0530 From: Harsh Bora MIME-Version: 1.0 References: <20120210115429.9787.92858.stgit@ginnungagap.bsc.es> <20120210115508.9787.26299.stgit@ginnungagap.bsc.es> In-Reply-To: <20120210115508.9787.26299.stgit@ginnungagap.bsc.es> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v4 07/11] trace: [tracetool] Rewrite event argument parsing List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?TGx1w61zIFZpbGFub3Zh?= Cc: stefanha@gmail.com, qemu-devel@nongnu.org, "Aneesh Kumar K. V" On 02/10/2012 05:25 PM, Lluís Vilanova wrote: > Signed-off-by: Lluís Vilanova > --- > 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.*)\s+)?(?P[^(\s]+)\((?P[^)]*)\)\s*(?P\".*)?") > @@ -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)) >