RE: [PATCH v2 12/12] btrace: Extend ptwrite event decoding.

Willgerodt, Felix via Gdb-patches
2021-06-15T13:41:38Z

>> * The 'set disassembler-options' command now supports specifying options
>> for the ARC target.

Eli> This part is OK.

Thanks!

>> +This module provides additional functionality for recording programs
>> +that make use of the PTWRITE instruction. PTWRITE is a x86
>> +instruction that allows to write values into the Intel Processor Trace (@pxref{Process Record and Replay}).

Eli> Should PTWRITE be in @code or in @sc?

I don't know, I couldn't find any convention for instructions in text
paragraphs when searching the docs. I changed it locally to @code,
as that looked cleaner to me.

>> +The @code{gdb.ptwrite} module allows customizing the default output
>> +of ptwrite auxiliary information. A custom Python function can be
>> +registered via @code{gdb.ptwrite.register_listener()} as the ptwrite listener function.

Eli> ^^ Please remove the parentheses there. Otherwise, this looks like a
Eli> call to the function with no arguments, which is not what you want.

Fixed it locally for the next revision.

>> +@smallexample
>> +@group
>> +(gdb) python-interactive
>> +>>> class my_listener(object):
>> +... def __init__(self):
>> +... self.variable = 0
>> +... def __call__(self, payload, ip):
>> +... if gdb.selected_thread().global_num == 1:
>> +... self.variable += 1
>> +... return "payload: @{@}, ip: @{:#x@}".format(self.variable, ip)

Eli> This line is too long for @smallexample, it will cause overful hbox in the
Eli> printed version of the manual.

Are you sure? I can't find any overfull hbox message for this line in the output
of 'make pdf' or the resulting gdb.log. When I shorten the line significantly
and diff the output of 'make pdf' (and gdb.log) with the long version,
I see no real difference. There are only underful messages for python.texi for me.
I have locally changed the variable name to "self.var" to shorten this anyway.

>> +This GDB feature is dependent on hardware and operating system
>> +support and

Eli> @value{GDBN}

Fixed it locally for the next revision.

Thanks!
Felix