Messages in this thread | | | Subject | Re: [PATCH v2] scripts/gdb: fix lx-dmesg when CONFIG_PRINTK_CALLER is set | From | Jan Kiszka <> | Date | Fri, 11 Oct 2019 14:38:47 +0200 |
| |
On 11.10.19 14:24, Joel Colledge wrote: > When CONFIG_PRINTK_CALLER is set, struct printk_log contains an > additional member caller_id. This affects the offset of the log text. > Account for this by using the type information from gdb to determine all > the offsets instead of using hardcoded values. > > This fixes following error: > > (gdb) lx-dmesg > Python Exception <class 'ValueError'> embedded null character: > Error occurred in Python command: embedded null character > > Signed-off-by: Joel Colledge <joel.colledge@linbit.com> > --- > Changes in v2: > - use type information from gdb instead of hardcoded offsets > > Thanks for the idea about using the struct layout info from gdb, Leonard. I can't see any reason we shouldn't use that here, since most of the other commands use it. LxDmesg has used hardcoded offsets since scripts/gdb was introduced, so I assume it just ended up like that during the initial development of the tool. Here is a version of the fix using offsets from gdb.
That's not unlikely, indeed. lx-dmesg was one of the very first features I've implemented back then, and it definitely predated things like CachedType.
> > scripts/gdb/linux/dmesg.py | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/scripts/gdb/linux/dmesg.py b/scripts/gdb/linux/dmesg.py > index 6d2e09a2ad2f..8f5d899029b7 100644 > --- a/scripts/gdb/linux/dmesg.py > +++ b/scripts/gdb/linux/dmesg.py > @@ -16,6 +16,8 @@ import sys > > from linux import utils > > +printk_log_type = utils.CachedType("struct printk_log") > + > > class LxDmesg(gdb.Command): > """Print Linux kernel log buffer.""" > @@ -42,9 +44,14 @@ class LxDmesg(gdb.Command): > b = utils.read_memoryview(inf, log_buf_addr, log_next_idx) > log_buf = a.tobytes() + b.tobytes() > > + length_offset = printk_log_type.get_type()['len'].bitpos // 8
Does bitpos really use a non-int type? Otherwise, plain '/' suffices.
> + text_len_offset = printk_log_type.get_type()['text_len'].bitpos // 8 > + time_stamp_offset = printk_log_type.get_type()['ts_nsec'].bitpos // 8 > + text_offset = printk_log_type.get_type().sizeof > + > pos = 0 > while pos < log_buf.__len__(): > - length = utils.read_u16(log_buf[pos + 8:pos + 10]) > + length = utils.read_u16(log_buf[pos + length_offset:pos + length_offset + 2])
Overlong line.
> if length == 0: > if log_buf_2nd_half == -1: > gdb.write("Corrupted log buffer!\n") > @@ -52,10 +59,11 @@ class LxDmesg(gdb.Command): > pos = log_buf_2nd_half > continue > > - text_len = utils.read_u16(log_buf[pos + 10:pos + 12]) > - text = log_buf[pos + 16:pos + 16 + text_len].decode( > + text_len = utils.read_u16(log_buf[pos + text_len_offset:pos + text_len_offset + 2])
Here as well. Better use some temp vars to break up the expressions. Helps with readability.
> + text = log_buf[pos + text_offset:pos + text_offset + text_len].decode( > encoding='utf8', errors='replace') > - time_stamp = utils.read_u64(log_buf[pos:pos + 8]) > + time_stamp = utils.read_u64( > + log_buf[pos + time_stamp_offset:pos + time_stamp_offset + 8]) > > for line in text.splitlines(): > msg = u"[{time:12.6f}] {line}\n".format( >
Looks good otherwise.
Jan
-- Siemens AG, Corporate Technology, CT RDA IOT SES-DE Corporate Competence Center Embedded Linux
| |