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?
- Arnaldo
