Re: [PATCH] Implement pahole-like 'ptype /o' option

[Sergio Durigan Junior <sergiodj@redhat.com>]

On Tuesday, November 21 2017, Eli Zaretskii wrote:

>> From: Sergio Durigan Junior <sergiodj@redhat.com>
>> Cc: Sergio Durigan Junior <sergiodj@redhat.com>
>> Date: Tue, 21 Nov 2017 11:07:09 -0500
>> 
>>  gdb/NEWS                                 |   3 +
>>  gdb/c-typeprint.c                        | 144 ++++++++++++++++++++++++++++---
>>  gdb/doc/gdb.texinfo                      |   4 +
>>  gdb/testsuite/gdb.base/ptype-offsets.cc  |  77 +++++++++++++++++
>>  gdb/testsuite/gdb.base/ptype-offsets.exp |  52 +++++++++++
>>  gdb/typeprint.c                          |   8 +-
>>  gdb/typeprint.h                          |   3 +
>>  7 files changed, 276 insertions(+), 15 deletions(-)
>>  create mode 100644 gdb/testsuite/gdb.base/ptype-offsets.cc
>>  create mode 100644 gdb/testsuite/gdb.base/ptype-offsets.exp
>
> The documentation parts are okay, but ...
>
>> +@item o
>> +Print the offsets and sizes of fields in a struct, similar to what the
>> +@command{pahole} tool does.
>
> ... how about an example showing the output of this?  I, for one, have
> never heard of 'pahole', so the reference to it won't help the likes
> of myself to understand what will be produced.

Good point.  I will include an example output, then.

> And here:
>
>> +  /o    print offsets and sizes of fields in a struct (like pahole)\n"));
>
> I wonder whether we should mention 'pahole' at all.

I thought that it was good to mention 'pahole' here because that was the
main motivation for writing this patch.  My feeling is that it's
benefitial to mention 'pahole' because even if the user doesn't know
what it is, this will be a good opportunity for them to learn :-).

But that's my personal opinion; I won't fight if you think the reference
should be removed.

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/