| Message ID | 20190115195241.GB1074@kadam (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | XArray: Fix a math problem in xa_is_err() | expand |
On Tue, Jan 15, 2019 at 10:52:41PM +0300, Dan Carpenter wrote: > @@ -177,7 +177,8 @@ static inline bool xa_is_internal(const void *entry) > static inline bool xa_is_err(const void *entry) > { > return unlikely(xa_is_internal(entry) && > - (unsigned long)entry >= -((MAX_ERRNO << 2) + 2)); > + (unsigned long)entry >= > + (((unsigned long)(-MAX_ERRNO << 2) + 2))); > } Ugh all the brackets, I'm not surprised I got it wrong. How about this instead; does it make your static checker happy? diff --git a/include/linux/xarray.h b/include/linux/xarray.h index 7da665f5cb20..5d9d318bcf7a 100644 --- a/include/linux/xarray.h +++ b/include/linux/xarray.h @@ -177,7 +177,7 @@ static inline bool xa_is_internal(const void *entry) static inline bool xa_is_err(const void *entry) { return unlikely(xa_is_internal(entry) && - (unsigned long)entry >= -((MAX_ERRNO << 2) + 2)); + entry >= xa_mk_internal(-MAX_ERRNO)); } /** (passes sparse & gcc, but ...)
On Tue, Jan 15, 2019 at 01:07:26PM -0800, Matthew Wilcox wrote: > On Tue, Jan 15, 2019 at 10:52:41PM +0300, Dan Carpenter wrote: > > @@ -177,7 +177,8 @@ static inline bool xa_is_internal(const void *entry) > > static inline bool xa_is_err(const void *entry) > > { > > return unlikely(xa_is_internal(entry) && > > - (unsigned long)entry >= -((MAX_ERRNO << 2) + 2)); > > + (unsigned long)entry >= > > + (((unsigned long)(-MAX_ERRNO << 2) + 2))); > > } > > Ugh all the brackets, I'm not surprised I got it wrong. How about this > instead; does it make your static checker happy? > I wasn't a super fan of that expression either... I think your code will work, and if not then it's the checker which needs fixing. Thanks! regards, dan carpenter
diff --git a/include/linux/xarray.h b/include/linux/xarray.h index 12244aa98a69..4208042f939a 100644 --- a/include/linux/xarray.h +++ b/include/linux/xarray.h @@ -177,7 +177,8 @@ static inline bool xa_is_internal(const void *entry) static inline bool xa_is_err(const void *entry) { return unlikely(xa_is_internal(entry) && - (unsigned long)entry >= -((MAX_ERRNO << 2) + 2)); + (unsigned long)entry >= + (((unsigned long)(-MAX_ERRNO << 2) + 2))); } /**
There is a math problem here which leads to a lot of static checker warnings for me: net/sunrpc/clnt.c:451 rpc_new_client() error: (-4096) too low for ERR_PTR Error values are from -1 to -4095 or from 0xffffffff to 0xfffff001 in hexadecimal. (I am assuming a 32 bit system for simplicity). We are using the lowest two bits to hold some internal XArray data so the error is shifted two spaces to the left. 0xfffff001 << 2 is 0xffffc004. And finally we want to check that BIT(1) is set so we add 2 which gives us 0xffffc006. In other words, we should be checking that "entry >= 0xffffc006", but the check is actually testing if "entry >= 0xffffc002". Fixes: 76b4e5299565 ("XArray: Permit storing 2-byte-aligned pointers") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- include/linux/xarray.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)