On 11/30/21 6:49 PM, Arnaldo Carvalho de Melo wrote:
Em Tue, Nov 30, 2021 at 04:42:55PM +0000, Douglas Raillard escreveu:
On 11/26/21 6:27 PM, Arnaldo Carvalho de Melo wrote:
Em Tue, Oct 19, 2021 at 11:07:23AM +0100, Douglas RAILLARD escreveu:
From: Douglas Raillard <douglas.raillard@xxxxxxx>
Allow making inner struct enums and union anonymous.
So I had to apply it by hand due to changes in the areas it touches, see
below, and I expanded a bit the commit message, please review:
Thanks for the review, it seems to be working as well as before. That said I found a couple of issues:
1. CLI options seem to abbreviate "anonymous" into "anon" so maybe we should do the same for this one ?
Ok, I can shorten the option name here.
2. I just found a case that generates a broken header:
struct /* hrtimer */ {
struct /* timerqueue_node */ {
struct /* rb_node */ {
long unsigned int __rb_parent_color; /* 3328 8 */
struct rb_node * rb_right; /* 3336 8 */
struct rb_node * rb_left; /* 3344 8 */
}node; /* 3328 24 */
/* typedef ktime_t -> s64 -> __s64 */ long long int expires; /* 3352 8 */
}node; /* 3328 32 */
/* typedef ktime_t -> s64 -> __s64 */ long long int _softexpires; /* 3360 8 */
enum hrtimer_restart (*function)(struct hrtimer *); /* 3368 8 */
struct hrtimer_clock_base * base; /* 3376 8 */
/* typedef u8 -> __u8 */ unsigned char state; /* 3384 1 */
/* typedef u8 -> __u8 */ unsigned char is_rel; /* 3385 1 */
/* typedef u8 -> __u8 */ unsigned char is_soft; /* 3386 1 */
/* typedef u8 -> __u8 */ unsigned char is_hard; /* 3387 1 */
}hrtick_timer; /* 3328 64 */
Here we can see that "struct rb_node" is a recursive type, so since the type
definition is now anonymous it will not compile. Detecting recursive types and
printing the name would avoid that but defeats the original purpose
of --inner_anonymous.
I can see two solutions:
1. Detecting recursive types and appending a user-defined prefix to create a
unique name.
2. Detecting recursive types and replacing the recursive references by "void *".
Solution #2 is the least invasive but will require a bit more work for the
end-user of the header:
struct hrtimer foo = *ptr;
typeof(foo.node.node) node = foo.node.node;
// Extra cast using typeof in order to change the type of tb_right from void*
// to "struct rb_node*"
((typeof(node))node.rb_right)->rb_left
A third solution and probably the easiest to implement. If you don't
punch a hole on it:
Track if the struct is being printed the first time and then flip a bit
not to print it again?
$ cat inner_anon.c
struct foo {
struct baz {
struct rb_node {
struct rb_node *prev, *next;
} yy;
} y;
struct /* rb_node */ {
struct rb_node *prev, *next;
} x;
} i;
$ cc -c inner_anon.c -o inner_anon.o
$ vim inner_anon.c
$ cat inner_anon.c
struct foo {
struct rb_node {
struct rb_node *prev, *next;
} x;
struct baz {
struct /* rb_node */ {
struct rb_node *prev, *next;
} yy;
} y;
} i;
$ cc -c inner_anon.c -o inner_anon.o
$
"struct baz" and "struct foo" could very well be private and needing pahole to be exposed,
but "struct rb_node" could be already exposed in a public header necessary for e.g. macros,
leading to a name clash. It's quite frustrating that the language leaves you with nothing
to deal with that sort of issues, C++ at least gives the options of namespaces and does
proper scoping for nested type definitions, but it's unfortunately not an option for kernel modules ...
Another alternative is to just manually "namespace" all the definitions with a user-given prefix
(not just for the recursive types).
