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

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

On Monday, December 11 2017, Pedro Alves wrote:

> On 12/11/2017 09:07 PM, Sergio Durigan Junior wrote:
>
>>>> This is actually on purpose.  If I indent the line, it will have more
>>>> than 80 chars.  I believe this is a well known method for avoiding this
>>>> problem...?
>>>
>>> I am not aware of that.  In this case I would put the parameter list on the next,
>>> I'm not sure if it's 100% GNU-style approved, but nobody complained when I did it
>>> so far :)
>>>
>>> static void
>>> print_spaces_filtered_with_print_options
>>>   (int level, struct ui_file *stream, const struct type_print_options *flags);
>>>
>>> It helps with long function names.  In this case, I would probably just drop the
>>> "struct" to save a few chars, because C++.
>> 
>> Fair enough.  I use this trick for function prototypes, but not for the
>> definitions.
>
> Simon's format is what I've been using for a long while too.

Well, I could post a few examples of the format I chose, but I'm pretty
sure this would be worthless.  As I said, I will change my code.

>>> But I don't mind it, it just stuck out as a little inconsistency.
>> 
>> I don't see the inconsistency.
>> 
>> If a field is inside a struct, it has its offset *and* size printed.  No
>> matter if the field is an int, another struct, or an union.
>> 
>> If a field is inside an union, it has only its size printed.
>> 
>> In the case above, it makes sense to have the offsets printed for the
>> fields inside the two structs (inside the union), because there might be
>> holes to report (well, one can argue that it doesn't matter if there are
>> holes or not in this case, because if the other struct is bigger then
>> the union size will stay the same).  However, it doesn't make sense to
>> print the offsets for the two structs themselves, because they are
>> members of the union.
>> 
>> I hope it makes more sense now.
>
> But why do we need the special case?  Does it help anything?
> So far, it seems it only added confusion.

What do you mean by "special case"?

This is what pahole does, and as I've said a few times, the output of
'ptype /o' has been based on pahole's output.  I don't consider this a
special case; I consider it to be the natural thing to do, because
offsets don't make much sense in unions.

> The option is "/o" for "print offsets".  Why not print offsets always?

I hope I explained it above.

> BTW, shouldn't the documentation in the manual include an example
> of GDB's output?

I can include an example, OK.

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