Re: cleanup: Make no_free_ptr() __must_check

From: Linus Torvalds
Date: Wed Aug 16 2023 - 04:01:25 EST


On Wed, 16 Aug 2023 at 07:23, Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx> wrote:
>
> Ah, ok, I thought the purpose was to ensure the p expression gets
> evaluated. Well, we can still do without the temp var and weird comma or
> statement expressions:

Well, the statement expression version with that types temporary
pointer was much better than yours, which just randomly returns 'void
*' and then lets the user assign it to any random pointer with no
warning.

But I think you can add just a 'typeof(p)' cast to it and that should
work out ok. I do absolutely hate how

But all of those macros seem to be fundamentally buggy because 'p'
gets evaluated twice. It could have side effects, even when just
having its address taken.

At that point the whole "comma or statement expression" discussion is
entirely immaterial.

So I think it needs to be something like

#define __get_and_free_ptr(p) \
({ __auto_type __ptr = &(p); \
__auto_type __val = *__ptr; \
*__ptr = NULL; __val; })

to deal with the "assign NULL and return old value without double evaluation".

And then you can have a wrapper macro to do the __must_check part,
something like

static inline __must_check
const volatile void * __must_check_fn(const volatile void *val)
{ return val; }

#define no_free_ptr(p) \
((typeof(p)) __must_check_fn(__get_and_free_ptr(p)))

the above is entirely untested. Of course.

Linus