simpletrace: improved error handling on struct unpack

A failed call to `read_header` wouldn't be handled the same for the two
different code paths (one path would try to use `None` as a list).
Changed to raise exception to be handled centrally. This also allows for
easier unpacking, as errors has been filtered out.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Mads Ynddal <m.ynddal@samsung.com>
Message-id: 20230926103436.25700-7-mads@ynddal.dk
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
This commit is contained in:
Mads Ynddal 2023-09-26 12:34:28 +02:00 committed by Stefan Hajnoczi
parent ce96eb334d
commit d1f9259014

View File

@ -37,26 +37,24 @@ def read_header(fobj, hfmt):
hlen = struct.calcsize(hfmt) hlen = struct.calcsize(hfmt)
hdr = fobj.read(hlen) hdr = fobj.read(hlen)
if len(hdr) != hlen: if len(hdr) != hlen:
return None raise ValueError('Error reading header. Wrong filetype provided?')
return struct.unpack(hfmt, hdr) return struct.unpack(hfmt, hdr)
def get_record(event_mapping, event_id_to_name, rechdr, fobj): def get_record(event_mapping, event_id_to_name, rechdr, fobj):
"""Deserialize a trace record from a file into a tuple """Deserialize a trace record from a file into a tuple
(name, timestamp, pid, arg1, ..., arg6).""" (name, timestamp, pid, arg1, ..., arg6)."""
if rechdr is None: event_id, timestamp_ns, length, pid = rechdr
return None if event_id != dropped_event_id:
if rechdr[0] != dropped_event_id:
event_id = rechdr[0]
name = event_id_to_name[event_id] name = event_id_to_name[event_id]
rec = (name, rechdr[1], rechdr[3])
try: try:
event = event_mapping[name] event = event_mapping[name]
except KeyError as e: except KeyError as e:
sys.stderr.write('%s event is logged but is not declared ' \ sys.stderr.write(f'{e} event is logged but is not declared ' \
'in the trace events file, try using ' \ 'in the trace events file, try using ' \
'trace-events-all instead.\n' % str(e)) 'trace-events-all instead.\n')
sys.exit(1) sys.exit(1)
rec = (name, timestamp_ns, pid)
for type, name in event.args: for type, name in event.args:
if is_string(type): if is_string(type):
l = fobj.read(4) l = fobj.read(4)
@ -67,9 +65,8 @@ def get_record(event_mapping, event_id_to_name, rechdr, fobj):
(value,) = struct.unpack('=Q', fobj.read(8)) (value,) = struct.unpack('=Q', fobj.read(8))
rec = rec + (value,) rec = rec + (value,)
else: else:
rec = ("dropped", rechdr[1], rechdr[3]) (dropped_count,) = struct.unpack('=Q', fobj.read(8))
(value,) = struct.unpack('=Q', fobj.read(8)) rec = ("dropped", timestamp_ns, pid, dropped_count)
rec = rec + (value,)
return rec return rec
def get_mapping(fobj): def get_mapping(fobj):
@ -86,22 +83,16 @@ def read_record(event_mapping, event_id_to_name, fobj):
def read_trace_header(fobj): def read_trace_header(fobj):
"""Read and verify trace file header""" """Read and verify trace file header"""
header = read_header(fobj, log_header_fmt) _header_event_id, _header_magic, log_version = read_header(fobj, log_header_fmt)
if header is None: if _header_event_id != header_event_id:
raise ValueError('Not a valid trace file!') raise ValueError(f'Not a valid trace file, header id {_header_event_id} != {header_event_id}')
if header[0] != header_event_id: if _header_magic != header_magic:
raise ValueError('Not a valid trace file, header id %d != %d' % raise ValueError(f'Not a valid trace file, header magic {_header_magic} != {header_magic}')
(header[0], header_event_id))
if header[1] != header_magic:
raise ValueError('Not a valid trace file, header magic %d != %d' %
(header[1], header_magic))
log_version = header[2]
if log_version not in [0, 2, 3, 4]: if log_version not in [0, 2, 3, 4]:
raise ValueError('Unknown version of tracelog format!') raise ValueError(f'Unknown version {log_version} of tracelog format!')
if log_version != 4: if log_version != 4:
raise ValueError('Log format %d not supported with this QEMU release!' raise ValueError(f'Log format {log_version} not supported with this QEMU release!')
% log_version)
def read_trace_records(event_mapping, event_id_to_name, fobj): def read_trace_records(event_mapping, event_id_to_name, fobj):
"""Deserialize trace records from a file, yielding record tuples (event_num, timestamp, pid, arg1, ..., arg6). """Deserialize trace records from a file, yielding record tuples (event_num, timestamp, pid, arg1, ..., arg6).