Messages in this thread | | | Date | Mon, 12 May 2008 16:38:30 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH] let ERR_PTR BUILD_BUG_ON when we know its argument is not a valid errno |
| |
On Sun, 11 May 2008 22:12:14 +0200 Marcin Slusarz <marcin.slusarz@gmail.com> wrote:
> Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com> > Cc: Andrew Morton <akpm@osdl.org> > --- > allmodconfig compile tested (on x86_64) > > should be applied after: > net/sunrpc/xprtrdma: fix svc_rdma_create out of memory error path > jfs: 0 is not valid errno value > --- > include/linux/err.h | 4 +++- > 1 files changed, 3 insertions(+), 1 deletions(-) > > diff --git a/include/linux/err.h b/include/linux/err.h > --- a/include/linux/err.h > +++ b/include/linux/err.h > @@ -19,11 +19,13 @@ > > #define IS_ERR_VALUE(x) unlikely((x) >= (unsigned long)-MAX_ERRNO) > > -static inline void *ERR_PTR(long error) > +static inline void *__ERR_PTR(long error) > { > return (void *) error; > } > > +#define ERR_PTR(error) (BUILD_BUG_ON(!IS_ERR_VALUE(error)), __ERR_PTR(error)) > + > static inline long PTR_ERR(const void *ptr) > { > return (long) ptr;
Not sure about this one. BUILD_BUG_ON only makes sense if the value is a compile-time constant. I think the code as you have it will take this:
int e = foo();
p = ERR_PTR(e);
and will attempt to evaluate sizeof() on a negative-sized array at runtime. The conmpile will laugh and throw that all away, but it's a bit weird.
Plus I'd have thought that the amount of code which does ERR_PTR(-EFOO) is fairly small, but perhaps that's wrong.
If I _am_ wrong then I do think it'd be saner to only do the BUILD_BUG_ON() if __builtin_constant_p(error) evaluates true. And even then I do think we'd like to see a more lengthy justification of why the kernel needs this check. More lengthy than zero, anyway...
(If a compile-time check is needed then why not a runtime one also?)
Thanks.
| |