Re: [Qemu-devel] [PATCH v7 2/3] Simpletrace v2: Support multiple arguments, strings.

[Stefan Hajnoczi]

On Tue, Jul 17, 2012 at 8:01 PM, Harsh Bora <address@hidden> wrote:
> On 07/17/2012 08:51 PM, Stefan Hajnoczi wrote:
>>
>> On Tue, Jul 3, 2012 at 10:20 AM, Harsh Prateek Bora
>> <address@hidden> wrote:
>>>
>>> Existing simpletrace backend allows to trace at max 6 args and does not
>>> support strings. This newer tracelog format gets rid of fixed size
>>> records
>>> and therefore allows to trace variable number of args including strings.
>>>
>>> Sample trace with strings:
>>> v9fs_version 0.000 tag=0xffff id=0x64 msize=0x2000 version=9P2000.L
>>> v9fs_version_return 6.705 tag=0xffff id=0x64 msize=0x2000
>>> version=9P2000.L
>>>
>>> Signed-off-by: Harsh Prateek Bora <address@hidden>
>>> ---
>>>   scripts/tracetool/backend/simple.py |   84 +++++++++---
>>>   trace/simple.c                      |  256
>>> ++++++++++++++++++++++-------------
>>>   trace/simple.h                      |   39 +++++-
>>>   3 files changed, 260 insertions(+), 119 deletions(-)
>>>
>>> diff --git a/scripts/tracetool/backend/simple.py
>>> b/scripts/tracetool/backend/simple.py
>>> index fbb5717..d3cf4da 100644
>>> --- a/scripts/tracetool/backend/simple.py
>>> +++ b/scripts/tracetool/backend/simple.py
>>> @@ -15,9 +15,16 @@ __email__      = "address@hidden"
>>>
>>>   from tracetool import out
>>>
>>> +def is_string(arg):
>>> +    strtype = ('const char*', 'char*', 'const char *', 'char *')
>>> +    if arg.lstrip().startswith(strtype):
>>> +        return True
>>> +    else:
>>> +        return False
>>>
>>>   def c(events):
>>>       out('#include "trace.h"',
>>> +        '#include "trace/simple.h"',
>>>           '',
>>>           'TraceEvent trace_list[] = {')
>>>
>>> @@ -26,30 +33,69 @@ def c(events):
>>>               name = e.name,
>>>               )
>>>
>>> -    out('};')
>>> -
>>> -def h(events):
>>> -    out('#include "trace/simple.h"',
>>> +    out('};',
>>>           '')
>>>
>>> -    for num, e in enumerate(events):
>>> -        if len(e.args):
>>> -            argstr = e.args.names()
>>> -            arg_prefix = ', (uint64_t)(uintptr_t)'
>>> -            cast_args = arg_prefix + arg_prefix.join(argstr)
>>> -            simple_args = (str(num) + cast_args)
>>> -        else:
>>> -            simple_args = str(num)
>>> +    for num, event in enumerate(events):
>>> +        sizes = []
>>> +        for type_, name in event.args:
>>> +            if is_string(type_):
>>> +                sizes.append("4 + ((" + name + " ? strlen(" + name + ")
>>> : 0) % MAX_TRACE_STRLEN)")
>>
>>
>> trace_record_write_str() and this code both use % to truncate the
>> string.  If the string is 512 characters long you get an empty string.
>>   That's weird and not normally how truncation works.
>>
>> Perhaps it's better to change this Python code to emit something like:
>> size_t arg%(num)d_len = %(name)s ? MAX(strlen(%(name)s, MAX_TRACE_STRLEN))
>> : 0;
>>
>
> I think we need to use MIN instead of MAX, right ?

Yes, my bad :).

Stefan