On Thu, May 24, 2012 at 10:50 AM, Harsh Prateek Bora <ha...@linux.vnet.ibm.com> wrote: > - fields = [event[0], '%0.3f' % (delta_ns / 1000.0)] > - for i in xrange(1, len(event)): > - fields.append('%s=0x%x' % (event[i], rec[i + 1])) > + if rec[0] == dropped_event_id: > + fields = ['Dropped_Event', '%0.3f' % (delta_ns / 1000.0)] > + fields.append('%s=0x%x' % ("dropped_events", rec[2])) > + else:
Why the special case? Can the dropped event be an event so the normal code path pretty-prints it? > + fields = [event.name, '%0.3f' % (delta_ns / 1000.0)] > + if log_version == 0: The global log_version variable splits the code. A nice solution is to encapsulate the format-specific behavior in a SimpleTraceV1 and SimpleTraceV2 class - both provide the same interface and the code that calls it doesn't care which exact version is used. That said I'm happy for us to drop simpletrace v1 pretty-printing entirely. Users can only generate v2 files with this QEMU binary. If they want to view old files they also need the old trace-events file - they might as well use the old simpletrace.py too! Feel free to focus on v2 only and drop the v1 support, you can add a nice error message explaining that this file uses the v1 format and should be used with the script that came with that QEMU binary. > + for type, name in event.args: > + fields.append('%s=0x%x' % (name, rec[i + 1])) > + i += 1 > + elif log_version == 2: > + for type, name in event.args: > + if is_string(type): > + fields.append('%s=%s' % (name, rec[i + 1])) > + else: > + fields.append('%s=0x%x' % (name, rec[i + 1])) > + i += 1 > print ' '.join(fields) > > run(Formatter()) > -- > 1.7.1.1 > >