[RFA 0/2] Support ptype/o in Rust
Sergio Durigan Junior
sergiodj@redhat.com
Sat Jun 23 23:44:00 GMT 2018
On Saturday, June 23 2018, Tom Tromey wrote:
> This adds support for ptype/o to the Rust code.
>
> The first patch slightly refactors the existing ptype/o code. The
> utility functions are now public methods on struct print_offset_data.
Thanks for the patches, Tom.
> The second patch changes the Rust language code. I would self-approve
> this one but it required a change outside of Rust. Perhaps this check
> ought to have been a flag on the language_defn.
>
> I noticed that ptype/o generates somewhat funny output:
>
> /* offset | size */ type = union simple::Union {
> /* 1 */ f1: i8,
> /* 1 */ f2: u8,
>
> /* total size (bytes): 1 */
> }
>
> Here, I think it might be cleaner to put the "total size" information
> on the same line as the trailing "}" (and of course not indent it),
> like:
>
> /* offset | size */ type = union simple::Union {
> /* 1 */ f1: i8,
> /* 1 */ f2: u8,
> /* total size 1 */ }
>
> If you agree I can at least file a bug or maybe implement it.
Your version does look better (and IIRC, one of the early versions of
the patch printed "total size" at column 0), but I think it may be a bit
confusing when we're dealing with inner structures. For example:
$ cat 2.c
struct a
{
int a1;
char a2;
int a3;
};
struct b
{
struct a b1;
int b2;
char b3;
};
struct c
{
struct a c1;
struct b c2;
char c3;
int c4;
};
int main ()
{
struct c foo;
return 0;
}
We have:
(gdb) ptype /o struct c
/* offset | size */ type = struct c {
/* 0 | 12 */ struct a {
/* 0 | 4 */ int a1;
/* 4 | 1 */ char a2;
/* XXX 3-byte hole */
/* 8 | 4 */ int a3;
/* total size (bytes): 12 */
} c1;
/* 12 | 20 */ struct b {
/* 12 | 12 */ struct a {
/* 12 | 4 */ int a1;
/* 16 | 1 */ char a2;
/* XXX 3-byte hole */
/* 20 | 4 */ int a3;
/* total size (bytes): 12 */
} b1;
/* 24 | 4 */ int b2;
/* 28 | 1 */ char b3;
/* XXX 3-byte padding */
/* total size (bytes): 20 */
} c2;
/* 32 | 1 */ char c3;
/* XXX 3-byte hole */
/* 36 | 4 */ int c4;
/* total size (bytes): 40 */
}
Even though it's a bit uglier than your solution, IMO it's easier to
understand and correlate the sizes with their respective structs.
Compare this to:
(gdb) ptype /o struct c
/* offset | size */ type = struct c {
/* 0 | 12 */ struct a {
/* 0 | 4 */ int a1;
/* 4 | 1 */ char a2;
/* XXX 3-byte hole */
/* 8 | 4 */ int a3;
/* total size (bytes): 12 */
} c1;
/* 12 | 20 */ struct b {
/* 12 | 12 */ struct a {
/* 12 | 4 */ int a1;
/* 16 | 1 */ char a2;
/* XXX 3-byte hole */
/* 20 | 4 */ int a3;
/* total size (bytes): 12 */
} b1;
/* 24 | 4 */ int b2;
/* 28 | 1 */ char b3;
/* XXX 3-byte padding */
/* total size (bytes): 20 */
} c2;
/* 32 | 1 */ char c3;
/* XXX 3-byte hole */
/* 36 | 4 */ int c4;
/* total size (bytes): 40 */
}
Of course, it's still possible to read the output and interpret it
correctly, but it demands a bit more effort, I think.
Maybe a solution would be to be a bit more verbose, like:
/* total size of struct a (bytes):... */
> Additionally I noticed that in C, in most cases fields are indented 4
> spaces, but with ptype/o the outermost fields are only indented 2
> spaces (relative to the "type =" text). I think this is probably
> unintended as well, but I thought I'd ask... ?
I don't really remember why I made this decision. I guess it had to do
with the fact that using 4 whitespaces to indent would consume a lot of
unnecessary space, and since ptype/o prints more information than the
regular ptype, every whitespace counts. I vaguely remember having this
thought, so that may be the reason, after all. Or maybe it also had
something to do with increasing the readability?
Anyway, TBH I don't have a strong opinion here. If you want to indent
the outermost fields by 4 spaces, I won't oppose.
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/
More information about the Gdb-patches
mailing list