Re: [PATCH 3/9] s390: Replace weird use of PTR_RET.

From: Heiko Carstens
Date: Mon Jun 17 2013 - 01:03:20 EST


On Sun, Jun 16, 2013 at 02:12:42PM +0930, Rusty Russell wrote:
> Saves repeating "(void __force *)__uptr" but it's less clear. Using
> the output of PTR_RET() to determine the error rather than just
> testing IS_ERR() is odd.

Ok, if it's confusing I won't mind if it gets changed. I intended to
keep the code as short as possible, but.. ;)

> For example, I *assume* __gptr_to_uptr() never returns NULL? Because
> the __ret would be 0 for the old code. The new version is clearer, IMHO:
> it would try to get_user() on that address.

__gptr_to_uptr() could return 0 and it's not an error case. In that case
it should indeed try a to get_user() on that address.

> If you hate this variant, I can just s/PTR_RET/PTR_ERR_OR_ZERO/ instead.

Your patch is fine.

> Cc: Heiko Carstens <heiko.carstens@xxxxxxxxxx>
> Cc: Christian Borntraeger <borntraeger@xxxxxxxxxx>
> Cc: Martin Schwidefsky <schwidefsky@xxxxxxxxxx>
> Signed-off-by: Rusty Russell <rusty@xxxxxxxxxxxxxxx>
> ---
> arch/s390/kvm/gaccess.h | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)

Acked-by: Heiko Carstens <heiko.carstens@xxxxxxxxxx>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/