| Message ID | 20240627-work-pidfs-v1-1-7e9ab6cc3bb1@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | pidfs: allow retrieval of namespace descriptors | expand |
On Thu, Jun 27, 2024 at 04:11:39PM +0200, Christian Brauner wrote: > Add a helper that returns the file descriptor and ensures that the old > variable contains a negative value. This makes it easy to rely on > CLASS(get_unused_fd). > Can we get an extra bit of explanation here, because I had to go read a bunch of code to figure out what exactly was happening here. Something like This makes it easy to rely on CLASS(get_unused_fd) for success, as the fd will be returned and the cleanup will not occur. Or something like this. Some of us are dumb and have a hard time with these new cleanup uses. Thanks, Josef
On Thu, Jun 27, 2024 at 01:24:48PM GMT, Josef Bacik wrote: > On Thu, Jun 27, 2024 at 04:11:39PM +0200, Christian Brauner wrote: > > Add a helper that returns the file descriptor and ensures that the old > > variable contains a negative value. This makes it easy to rely on > > CLASS(get_unused_fd). > > > > Can we get an extra bit of explanation here, because I had to go read a bunch of > code to figure out what exactly was happening here. Something like > > This makes it easy to rely on CLASS(get_unused_fd) for success, as the fd will > be returned and the cleanup will not occur. Yes, absolutely.
diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h index c2d09bc4f976..80c4181e194a 100644 --- a/include/linux/cleanup.h +++ b/include/linux/cleanup.h @@ -63,17 +63,20 @@ #define __free(_name) __cleanup(__free_##_name) -#define __get_and_null_ptr(p) \ - ({ __auto_type __ptr = &(p); \ - __auto_type __val = *__ptr; \ - *__ptr = NULL; __val; }) +#define __get_and_null(p, nullvalue) \ + ({ \ + __auto_type __ptr = &(p); \ + __auto_type __val = *__ptr; \ + *__ptr = nullvalue; \ + __val; \ + }) 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_null_ptr(p))) + ((typeof(p)) __must_check_fn(__get_and_null(p, NULL))) #define return_ptr(p) return no_free_ptr(p) diff --git a/include/linux/file.h b/include/linux/file.h index 45d0f4800abd..3ea7f2452f20 100644 --- a/include/linux/file.h +++ b/include/linux/file.h @@ -97,6 +97,8 @@ extern void put_unused_fd(unsigned int fd); DEFINE_CLASS(get_unused_fd, int, if (_T >= 0) put_unused_fd(_T), get_unused_fd_flags(flags), unsigned flags) +#define take_fd(fd) __get_and_null(fd, -EBADF) + extern void fd_install(unsigned int fd, struct file *file); int receive_fd(struct file *file, int __user *ufd, unsigned int o_flags);
Add a helper that returns the file descriptor and ensures that the old variable contains a negative value. This makes it easy to rely on CLASS(get_unused_fd). Signed-off-by: Christian Brauner <brauner@kernel.org> --- include/linux/cleanup.h | 13 ++++++++----- include/linux/file.h | 2 ++ 2 files changed, 10 insertions(+), 5 deletions(-)