Re: [PATCH] err_ptr.h: introduce ERR_PTR_SAFE()
Amir Goldstein <[email protected]> Sat, 16 May 2026 13:39:11 +0200
| Newsgroups | org.kernel.vger.linux-unionfs,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CAOQ4uxjX=C6WZvW1DdJ13tJj-XBWVxgE2Q9vVvHoYqiOhqW0wQ@mail.gmail.com> |
On Sat, May 16, 2026 at 10:42 AM David Laight <[email protected]> wrote: > > On Fri, 15 May 2026 21:26:04 +0200 > Amir Goldstein <[email protected]> wrote: > > > On Fri, May 15, 2026 at 8:30 PM David Laight > > <[email protected]> wrote: > > > > > > On Thu, 14 May 2026 22:01:29 +0200 > > > Amir Goldstein <[email protected]> wrote: > > > > ... > > > > > > The object code bloat would be noticeable if this were used everywhere. > > > But you could make it a bit simpler: > > > if (__builtin_constant_p(__e)) > > > BUILD_BUG_ON(__e && !IS_ERR_VALUE(__e)); > > > else if WARN_ON(__e && !IS_ERR_VALUE(__e)) > > > __e = -MAX_ERRNO; // Or maybe -EINVAL to stop and other boundary errors > > > (void *)__e; > > > > Yeh that's nicer thanks. > > Actually this might be better still (or just more succinct): > void *__e = (void *)error; > BUILD_BUG_ON(!statically_true(IS_ERR_OR_NULL(__e)); This condition is wrong but also my compiler does not evaluate __builtin_constant_p(IS_ERR_OR_NULL(__e)) as true. This works BUILD_BUG_ON(statically_true(!IS_ERR_VALUE(__e))); I think it is enough to statically assert on ERR_PTR(EINVAL) and no need to bother with ERR_PTR(0) > if (WARN_ON(!IS_ERR_OR_NULL(__e)) > __e = (void *)-EINVAL; Oh, anything but EINVAL please - the most overloaded error value My choice of meaningful error value would be EFAULT because without the safe helper we would be returning an address which is in most likelihood bad, so better be explicit about it. > __e; > > The WARN_ON() will be optimised away (valid) constants. > Yeh this looks nice I'll use this: #define ERR_PTR_SAFE(error) ({ \ void *__e = (void *)(long)(error); \ BUILD_BUG_ON(statically_true(!IS_ERR_VALUE(__e))); \ if (WARN_ON(!IS_ERR_OR_NULL(__e))) \ __e = (void *)(long)-EFAULT; \ __e; \ }) Thanks! Amir.