Hello Rusty,
[added akpm to Cc: who took the patch back then and Julia for the
coccinelle part below]
On Mon, Jun 03, 2013 at 11:59:15AM +0930, Rusty Russell wrote:
>
> Back in 2011, Uwe Kleine-König added the nonsensically-named
> PTR_RET(), providing a means to avoid if() statements in code (commit
> fa9ee9c4b9).
>
> Instead, just make PTR_ERR() return 0 if the pointer isn't an error
> value. This is harmless, since PTR_ERR() should have never been
> passed a non-error value. And GCC is usually smart enough to remove
> the extra test if IS_ERR() has already been called.
I wonder in which situations gcc fails to be smart enough. Did you check
this?
> My vmlinux text increased by 300 bytes:
>
> text data bss dec hex filename
> 6029452 491628 2576384 9097464 8ad0f8 vmlinux
> 6029721 491628 2576384 9097733 8ad205 vmlinux.PTR_ERR
>
> Cc: Uwe Kleine-König <u.kleine-koe...@pengutronix.de>
> Cc: Thomas Meyer <tho...@m3y3r.de>
> Signed-off-by: Rusty Russell <ru...@rustcorp.com.au>
>
> diff --git a/include/linux/err.h b/include/linux/err.h
> index f2edce2..621d859 100644
> --- a/include/linux/err.h
> +++ b/include/linux/err.h
> @@ -24,14 +24,16 @@ static inline void * __must_check ERR_PTR(long error)
> return (void *) error;
> }
>
> -static inline long __must_check PTR_ERR(const void *ptr)
> +static inline long __must_check IS_ERR(const void *ptr)
> {
> - return (long) ptr;
> + return IS_ERR_VALUE((unsigned long)ptr);
> }
>
> -static inline long __must_check IS_ERR(const void *ptr)
> +static inline long __must_check PTR_ERR(const void *ptr)
> {
> - return IS_ERR_VALUE((unsigned long)ptr);
> + if (IS_ERR(ptr))
> + return (long) ptr;
> + return 0;
> }
>
> static inline long __must_check IS_ERR_OR_NULL(const void *ptr)
> @@ -52,14 +54,7 @@ static inline void * __must_check ERR_CAST(const void *ptr)
> return (void *) ptr;
> }
>
> -static inline int __must_check PTR_RET(const void *ptr)
> -{
> - if (IS_ERR(ptr))
> - return PTR_ERR(ptr);
> - else
> - return 0;
> -}
> -
> +#define PTR_RET PTR_ERR
I'd add a comment here that PTR_RET shouldn't be used anymore.
> #endif
>
> #endif /* _LINUX_ERR_H */
>
Is it worth to apply the same change to tools/virtio/linux/err.h to
minimize the chance for later surprises?
Also scripts/coccinelle/api/ptr_ret.cocci starts giving false warnings.
Other than that I think the change is fine.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
