[PATCH 0/2] fix some sparse warnings in lustre

[PATCH 0/2] fix some sparse warnings in lustre

[Zahari Doychev]

The two patches fix several sparse warning in the lustre module.

Zahari Doychev (2):
  [drivers] staging/lustre: fix sparse warnings
  [drivers] staging/lustre: fix sparse warnings

 drivers/staging/lustre/include/linux/lnet/lib-lnet.h   | 14 +++++++++-----
 drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c | 15 ++++++++++-----
 2 files changed, 19 insertions(+), 10 deletions(-)

-- 
2.1.0

[PATCH 1/2] [drivers] staging/lustre: fix sparse warnings

[Zahari Doychev]

This patch fixes the following sparse warnings:

lib-lnet.h:787:47: warning: incorrect type in initializer (different address spaces)
lib-lnet.h:787:47:    expected void [noderef] <asn:1>*iov_base
lib-lnet.h:787:47:    got void *dest

Signed-off-by: Zahari Doychev <zahari.doychev@linux.com>
---
 drivers/staging/lustre/include/linux/lnet/lib-lnet.h | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/lustre/include/linux/lnet/lib-lnet.h b/drivers/staging/lustre/include/linux/lnet/lib-lnet.h
index 7e89b3b..ec70f40 100644
--- a/drivers/staging/lustre/include/linux/lnet/lib-lnet.h
+++ b/drivers/staging/lustre/include/linux/lnet/lib-lnet.h
@@ -784,7 +784,8 @@ lnet_copy_iov2flat(int dlen, void *dest, unsigned int doffset,
 		   unsigned int nsiov, struct iovec *siov, unsigned int soffset,
 		   unsigned int nob)
 {
-	struct iovec diov = {/*.iov_base = */ dest, /*.iov_len = */ dlen};
+	struct iovec diov = {/*.iov_base = */ (void __user *)dest,
+			     /*.iov_len = */ dlen};
 
 	lnet_copy_iov2iov(1, &diov, doffset,
 			  nsiov, siov, soffset, nob);
@@ -795,7 +796,8 @@ lnet_copy_kiov2flat(int dlen, void *dest, unsigned int doffset,
 		    unsigned int nsiov, lnet_kiov_t *skiov,
 		    unsigned int soffset, unsigned int nob)
 {
-	struct iovec diov = {/* .iov_base = */ dest, /* .iov_len = */ dlen};
+	struct iovec diov = {/* .iov_base = */ (void __user *)dest,
+			     /* .iov_len = */ dlen};
 
 	lnet_copy_kiov2iov(1, &diov, doffset,
 			   nsiov, skiov, soffset, nob);
@@ -803,9 +805,10 @@ lnet_copy_kiov2flat(int dlen, void *dest, unsigned int doffset,
 
 static inline void
 lnet_copy_flat2iov(unsigned int ndiov, struct iovec *diov, unsigned int doffset,
-		   int slen, void *src, unsigned int soffset, unsigned int nob)
+	   int slen, void *src, unsigned int soffset, unsigned int nob)
 {
-	struct iovec siov = {/*.iov_base = */ src, /*.iov_len = */slen};
+	struct iovec siov = {/*.iov_base = */ (void __user *)src,
+			     /*.iov_len = */ slen};
 
 	lnet_copy_iov2iov(ndiov, diov, doffset,
 			  1, &siov, soffset, nob);
@@ -816,7 +819,8 @@ lnet_copy_flat2kiov(unsigned int ndiov, lnet_kiov_t *dkiov,
 		    unsigned int doffset, int slen, void *src,
 		    unsigned int soffset, unsigned int nob)
 {
-	struct iovec siov = {/* .iov_base = */ src, /* .iov_len = */ slen};
+	struct iovec siov = {/* .iov_base = */ (void __user *)src,
+			     /* .iov_len = */ slen};
 
 	lnet_copy_iov2kiov(ndiov, dkiov, doffset,
 			   1, &siov, soffset, nob);
-- 
2.1.0

Re: [PATCH 1/2] [drivers] staging/lustre: fix sparse warnings

[Dan Carpenter]

On Mon, Nov 24, 2014 at 07:55:41PM +0100, Zahari Doychev wrote:
> --- a/drivers/staging/lustre/include/linux/lnet/lib-lnet.h
> +++ b/drivers/staging/lustre/include/linux/lnet/lib-lnet.h
> @@ -784,7 +784,8 @@ lnet_copy_iov2flat(int dlen, void *dest, unsigned int doffset,
>  		   unsigned int nsiov, struct iovec *siov, unsigned int soffset,
>  		   unsigned int nob)
>  {
> -	struct iovec diov = {/*.iov_base = */ dest, /*.iov_len = */ dlen};
> +	struct iovec diov = {/*.iov_base = */ (void __user *)dest,
> +			     /*.iov_len = */ dlen};
>  

Why can't we just make the comments into code by removing the /*
characters?  Remove the cast by declaring the data as __user data to
begin with instead of declaring it incorrectly and then casting to the
correct type later.

Also it's not allowed to send two patches with the exact same subject.
Also the subject was sucky and too vague anyway.  :)

regards,
dan carpenter

Re: [PATCH 1/2] [drivers] staging/lustre: fix sparse warnings

[Dilger, Andreas]

The non-posix initializers are a holdover from Windows, where the compiler doesn't have C99 initializers. We don't need that anymore, so the initializer can indeed be uncommented. 

Cheers, Andreas

> On Nov 24, 2014, at 16:41, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> 
>> On Mon, Nov 24, 2014 at 07:55:41PM +0100, Zahari Doychev wrote:
>> --- a/drivers/staging/lustre/include/linux/lnet/lib-lnet.h
>> +++ b/drivers/staging/lustre/include/linux/lnet/lib-lnet.h
>> @@ -784,7 +784,8 @@ lnet_copy_iov2flat(int dlen, void *dest, unsigned int doffset,
>>           unsigned int nsiov, struct iovec *siov, unsigned int soffset,
>>           unsigned int nob)
>> {
>> -    struct iovec diov = {/*.iov_base = */ dest, /*.iov_len = */ dlen};
>> +    struct iovec diov = {/*.iov_base = */ (void __user *)dest,
>> +                 /*.iov_len = */ dlen};
>> 
> 
> Why can't we just make the comments into code by removing the /*
> characters?  Remove the cast by declaring the data as __user data to
> begin with instead of declaring it incorrectly and then casting to the
> correct type later.
> 
> Also it's not allowed to send two patches with the exact same subject.
> Also the subject was sucky and too vague anyway.  :)
> 
> regards,
> dan carpenter
> 

Re: [PATCH 1/2] [drivers] staging/lustre: fix sparse warnings

[Zahari Doychev]

On Tue, Nov 25, 2014 at 02:40:36AM +0300, Dan Carpenter wrote:
> On Mon, Nov 24, 2014 at 07:55:41PM +0100, Zahari Doychev wrote:
> > --- a/drivers/staging/lustre/include/linux/lnet/lib-lnet.h
> > +++ b/drivers/staging/lustre/include/linux/lnet/lib-lnet.h
> > @@ -784,7 +784,8 @@ lnet_copy_iov2flat(int dlen, void *dest, unsigned int doffset,
> >  		   unsigned int nsiov, struct iovec *siov, unsigned int soffset,
> >  		   unsigned int nob)
> >  {
> > -	struct iovec diov = {/*.iov_base = */ dest, /*.iov_len = */ dlen};
> > +	struct iovec diov = {/*.iov_base = */ (void __user *)dest,
> > +			     /*.iov_len = */ dlen};
> >  
> 
> Why can't we just make the comments into code by removing the /*
> characters?  Remove the cast by declaring the data as __user data to
> begin with instead of declaring it incorrectly and then casting to the
> correct type later.

If I make the declarations right then I am moving the casts to other functions.
I was not sure which one is better but it is not a problem I will fix this.

> 
> Also it's not allowed to send two patches with the exact same subject.
> Also the subject was sucky and too vague anyway.  :)

Sorry for that.

regards,
Zahari

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

Re: [PATCH 1/2] [drivers] staging/lustre: fix sparse warnings

[Dan Carpenter]

On Tue, Nov 25, 2014 at 08:35:42AM +0100, Zahari Doychev wrote:
> On Tue, Nov 25, 2014 at 02:40:36AM +0300, Dan Carpenter wrote:
> > On Mon, Nov 24, 2014 at 07:55:41PM +0100, Zahari Doychev wrote:
> > > --- a/drivers/staging/lustre/include/linux/lnet/lib-lnet.h
> > > +++ b/drivers/staging/lustre/include/linux/lnet/lib-lnet.h
> > > @@ -784,7 +784,8 @@ lnet_copy_iov2flat(int dlen, void *dest, unsigned int doffset,
> > >  		   unsigned int nsiov, struct iovec *siov, unsigned int soffset,
> > >  		   unsigned int nob)
> > >  {
> > > -	struct iovec diov = {/*.iov_base = */ dest, /*.iov_len = */ dlen};
> > > +	struct iovec diov = {/*.iov_base = */ (void __user *)dest,
> > > +			     /*.iov_len = */ dlen};
> > >  
> > 
> > Why can't we just make the comments into code by removing the /*
> > characters?  Remove the cast by declaring the data as __user data to
> > begin with instead of declaring it incorrectly and then casting to the
> > correct type later.
> 
> If I make the declarations right then I am moving the casts to other functions.

Yeah.  It might introduce new warnings.  Don't stress about that.  Don't
add casts, we'll just fix it all later.  It's better to have warnings
instead of hiding them with casts.

regards,
dan carpenter

[PATCH 2/2] [drivers] staging/lustre: fix sparse warnings

[Zahari Doychev]

This patch fixes the following warnings:

socklnd_cb.c:134:39: warning: incorrect type in assignment (different address spaces)
socklnd_cb.c:134:39:    expected void [noderef] <asn:1>*iov_base
socklnd_cb.c:134:39:    got void *<noident>

Signed-off-by: Zahari Doychev <zahari.doychev@linux.com>
---
 drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
index d29f5f1..971b877 100644
--- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
+++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
@@ -131,7 +131,8 @@ ksocknal_send_iov (ksock_conn_t *conn, ksock_tx_t *tx)
 		LASSERT (tx->tx_niov > 0);
 
 		if (nob < (int) iov->iov_len) {
-			iov->iov_base = (void *)((char *)iov->iov_base + nob);
+			iov->iov_base =
+				(void __user *)((char *)iov->iov_base + nob);
 			iov->iov_len -= nob;
 			return rc;
 		}
@@ -1052,7 +1053,8 @@ ksocknal_new_packet (ksock_conn_t *conn, int nob_to_skip)
 		case  KSOCK_PROTO_V3:
 			conn->ksnc_rx_state = SOCKNAL_RX_KSM_HEADER;
 			conn->ksnc_rx_iov = (struct iovec *)&conn->ksnc_rx_iov_space;
-			conn->ksnc_rx_iov[0].iov_base = (char *)&conn->ksnc_msg;
+			conn->ksnc_rx_iov[0].iov_base =
+				(void __user *)&conn->ksnc_msg;
 
 			conn->ksnc_rx_nob_wanted = offsetof(ksock_msg_t, ksm_u);
 			conn->ksnc_rx_nob_left = offsetof(ksock_msg_t, ksm_u);
@@ -1066,7 +1068,8 @@ ksocknal_new_packet (ksock_conn_t *conn, int nob_to_skip)
 			conn->ksnc_rx_nob_left = sizeof(lnet_hdr_t);
 
 			conn->ksnc_rx_iov = (struct iovec *)&conn->ksnc_rx_iov_space;
-			conn->ksnc_rx_iov[0].iov_base = (char *)&conn->ksnc_msg.ksm_u.lnetmsg;
+			conn->ksnc_rx_iov[0].iov_base =
+				(void __user *)&conn->ksnc_msg.ksm_u.lnetmsg;
 			conn->ksnc_rx_iov[0].iov_len  = sizeof (lnet_hdr_t);
 			break;
 
@@ -1093,7 +1096,8 @@ ksocknal_new_packet (ksock_conn_t *conn, int nob_to_skip)
 	do {
 		nob = MIN (nob_to_skip, sizeof (ksocknal_slop_buffer));
 
-		conn->ksnc_rx_iov[niov].iov_base = ksocknal_slop_buffer;
+		conn->ksnc_rx_iov[niov].iov_base =
+			(void __user *)ksocknal_slop_buffer;
 		conn->ksnc_rx_iov[niov].iov_len  = nob;
 		niov++;
 		skipped += nob;
@@ -1218,7 +1222,8 @@ ksocknal_process_receive (ksock_conn_t *conn)
 		conn->ksnc_rx_nob_left = sizeof(ksock_lnet_msg_t);
 
 		conn->ksnc_rx_iov = (struct iovec *)&conn->ksnc_rx_iov_space;
-		conn->ksnc_rx_iov[0].iov_base = (char *)&conn->ksnc_msg.ksm_u.lnetmsg;
+		conn->ksnc_rx_iov[0].iov_base =
+				(void __user *)&conn->ksnc_msg.ksm_u.lnetmsg;
 		conn->ksnc_rx_iov[0].iov_len  = sizeof(ksock_lnet_msg_t);
 
 		conn->ksnc_rx_niov = 1;
-- 
2.1.0

Re: [PATCH 0/2] fix some sparse warnings in lustre

[Greg KH]

On Mon, Nov 24, 2014 at 07:55:40PM +0100, Zahari Doychev wrote:
> The two patches fix several sparse warning in the lustre module.
> 
> Zahari Doychev (2):
>   [drivers] staging/lustre: fix sparse warnings
>   [drivers] staging/lustre: fix sparse warnings
> 
>  drivers/staging/lustre/include/linux/lnet/lib-lnet.h   | 14 +++++++++-----
>  drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c | 15 ++++++++++-----
>  2 files changed, 19 insertions(+), 10 deletions(-)

Please fix up and resend anything still pending, I've purged your
patches from my queue now as they all seem to not be correct :(