On Wed Jan 31, 2024 at 7:36 AM AEST, John Snow wrote:
> On Thu, Jan 25, 2024 at 11:09 AM Nicholas Piggin <npig...@gmail.com> wrote:
> >
> > The v12 format support for replay-dump has a few issues still. This
> > fixes async decoding; adds event, shutdown, and end decoding; fixes
> > audio in / out events, fixes checkpoint checking of following async
> > events.
> >
> > Signed-off-by: Nicholas Piggin <npig...@gmail.com>
> > ---
> > scripts/replay-dump.py | 132 ++++++++++++++++++++++++++++++-----------
> > 1 file changed, 98 insertions(+), 34 deletions(-)
> >
> > diff --git a/scripts/replay-dump.py b/scripts/replay-dump.py
> > index d668193e79..35732da08f 100755
> > --- a/scripts/replay-dump.py
> > +++ b/scripts/replay-dump.py
> > @@ -20,6 +20,7 @@
> >
> > import argparse
> > import struct
> > +import os
> > from collections import namedtuple
> > from os import path
> >
> > @@ -63,6 +64,10 @@ def read_byte(fin):
> > "Read a single byte"
> > return struct.unpack('>B', fin.read(1))[0]
> >
> > +def read_bytes(fin, nr):
> > + "Read a nr bytes"
>
> Existing problem in this file, but please use """triple quotes""" for
> docstrings.
Just coming back to this, sorry, was struggling a bit with ppc merge :/
> > + return fin.read(nr)
> > +
>
> Does it really save a lot of typing to alias fin.read(1) to
> read_bytes(fin, 1) ...?
Not really, I'll squash it.
>
> > def read_event(fin):
> > "Read a single byte event, but save some state"
> > if replay_state.already_read:
> > @@ -134,6 +139,18 @@ def swallow_async_qword(eid, name, dumpfile):
> > print(" %s(%d) @ %d" % (name, eid, step_id))
> > return True
> >
> > +def swallow_bytes(eid, name, dumpfile, nr):
> > + "Swallow nr bytes of data without looking at it"
> > + dumpfile.seek(nr, os.SEEK_CUR)
> > + return True
> > +
>
> Why bother returning a bool if it's not based on any condition? Add an
> error check or just drop the return value.
>
> > +def decode_exception(eid, name, dumpfile):
> > + print_event(eid, name)
> > + return True
> > +
>
> I suppose in this case, the return is to fit a common signature.
Yes that's why I did it, but it actually can't fit in the normal
decoder pattern because a nr has to be supplied as well. I'll
change it as you say.
Thanks,
Nick
