Re: [RFC][PATCH] rcu: Use typeof(p) instead of typeof(*p) *

[Steven Rostedt <rostedt@goodmis.org>]

On Tue, 5 Oct 2021 23:09:08 +0200 (CEST)
Jan Engelhardt <jengelh@inai.de> wrote:

> On Tuesday 2021-10-05 22:37, Steven Rostedt wrote:
> >
> >Really, thinking about abstraction, I don't believe there's anything wrong
> >with returning a pointer of one type, and then typecasting it to a pointer
> >of another type. Is there? As long as whoever uses the returned type does
> >nothing with it.  
> 
> Illegal.
> https://en.cppreference.com/w/c/language/conversion
> subsection "Pointer conversion"
> "No other guarantees are offered"

Basically (one alternative I was looking at) was simply passing around a
void pointer. Not sure how the RCU macros would handle that. But to
completely abstract it out, I was thinking of just returning void * and
accepting void *, but I didn't want to do that because now we just lost any
kind of type checking done by the compiler. The tricks I was playing was to
keep some kind of type checking.

> 
> >struct trace_pid_list *trace_pid_list_alloc(void)
> >{
> >	struct pid_list *pid_list;
> >
> >	pid_list = kmalloc(sizeof(*pid_list), GFP_KERNEL);
> >	[..]
> >
> >	return (struct trace_pid_list *)pid_list;
> >}  
> 
> struct trace_pid_list { void *pid_list; };
> struct trace_pid_list trace_pid_list_alloc(void)
> {
> 	struct trace_pid_list t;
> 	t.pid_list = kmalloc(sizeof(t.orig), GFP_KERNEL);
> 	return t;
> }
> void freethat(struct strace_pid_list x)
> {
> 	kfree(x.pid_list);
> }
> 
> Might run afoul of -Waggregate-return in C.

The above isn't exactly what I was suggesting.

And really, not that I'm going to do this, I could have followed the rest
of the kernel with:

struct trace_pid_list {
	int max;
	[..]
};

int *trace_pid_list_alloc(void)
{
	struct trace_pid_list *pid_list;

	pid_list = kmalloc(sizeof(*pid_list), GFP_KERNEL);

	[..]
	return &pid_list->max;
}

void trace_pid_list_free(int *p)
{
	struct trace_pid_list *pid_list = container_of(p, struct pid_list, max);

	[..]
	free(pid_list);
}


Because we do this all over the kernel. Talk about lying to the compiler ;-)

-- Steve