Re: [PATCH] err_ptr.h: introduce ERR_PTR_SAFE()
David Laight <[email protected]> Mon, 18 May 2026 13:48:32 +0100
| Newsgroups | org.kernel.vger.linux-unionfs,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <20260518134832.69853809@pumpkin> |
On Mon, 18 May 2026 11:52:06 +0200 Amir Goldstein <[email protected]> wrote: > On Mon, May 18, 2026 at 11:04 AM Rasmus Villemoes <[email protected]> wrote: > > > > On Sat, May 16 2026, Amir Goldstein <[email protected]> wrote: > > > > > 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 > > > > Could we have a dedicated "EBUG" that can be used to indicate "there's a > > bug somewhere in the kernel, we can handle it somewhat gracefully here > > by returning an error instead of BUG(). You can't do anything about it > > but report that you got -EBUG from $this_syscall", instead of > > overloading EIO, EFAULT, EINVAL or whatnot. Internally, EBUG could be a > > macro that did ({ WARN_ONCE(); -__EBUG; }) or something, so there would > > automatically be bread crumbs in dmesg if it is ever hit. > > How exactly would you use this EBUG() to write the graceful handling? > A bit of overengineering if you ask me. > > > > > Userspace code could also benefit from having EBUG to indicate that some > > internal inconsistency has been detected. > > So what about > > #define EBUG MAX_ERRNO > > as per my suggested patch. Your are relying on there being no 'off-by-one' errors. They happen... -- David > > Thanks, > Amir. >