On Mon, 2013-05-06 at 18:44 +0300, Dan Carpenter wrote:
> Hello Simo Sorce,
>
> The patch 1d658336b05f: "SUNRPC: Add RPC based upcall mechanism for
> RPCGSS auth" from May 25, 2012, leads to the following warning:
> "net/sunrpc/auth_gss/gss_rpc_xdr.c:30 gssx_check_pointer()
> warn: signedness bug returning '(-28)'"
>
> net/sunrpc/auth_gss/gss_rpc_xdr.c
> 24 static bool gssx_check_pointer(struct xdr_stream *xdr)
> 25 {
> 26 __be32 *p;
> 27
> 28 p = xdr_reserve_space(xdr, 4);
> 29 if (unlikely(p == NULL))
> 30 return -ENOSPC;
> ^^^^^^^
> This is casted implicitly to "true". Functions named "check" are a bad
> idea anyways because it's not clear what the return value will be. It's
> better to use "gssx_pointer_ok()" or "valid" where obviously true means
> that the pointer is ok.
>
> 31 return *p?true:false;
>
> We just reserved "*p" so doesn't this point to uninitialized data? It
> points to a __be32. So we're not really checking a pointer we're
> checking that __be32 is non-zero.
>
> 32 }
>
> regards,
> dan carpenter
Dan,
thanks a lot for pointing this one out.
Bruce,
we should fix it in 3.10 if we can make it.
probably easiest way is to kill lines 29/30 and do:
return (p && *p) ? true : false;
It would be also ok to change the name to gssx_pointer_is_valid() I
guess.
Simo.
--
Simo Sorce * Red Hat, Inc * New York
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
