On 12/1/21 12:01 PM, Arnaldo Carvalho de Melo wrote:
Em Wed, Dec 01, 2021 at 10:56:49AM +0000, Douglas Raillard escreveu:
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:
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).
Yeah, that namespacing can be an optional argument to --inner_anon, then
having what I suggested + the namespace prefix we can get closer to what
we want?
Yes I'll try to make a new version based on that:
* print only the first time it's encountered
* add option to prefix types for namespacing
* add an option to add typedef for original name for the types that were
specified explicitly (or maybe simply do not apply the namespace on them)
This should fix the recursive type issue and allow using explicitly given types directly.
Douglas
- Arnaldo
