fix some sparse warnings in lustre

PATCH In review Zahari Doychev 4 participants 8 messages Nov 24, 2014 – Nov 26, 2014 Staging

lore

Share & tools

lore

Zahari Doychev

author

2014-11-24 18:55 UTC #0 lore raw

cover fix some sparse warnings in lustre

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(-)

Signature · 1 line

2.1.0

Zahari Doychev

author

2014-11-24 18:55 UTC #1 lore raw

PATCH 1/2 staging/lustre: fix sparse warnings

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 <[email protected]>

---
 drivers/staging/lustre/include/linux/lnet/lib-lnet.h | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

drivers/staging/lustre/include/linux/lnet/lib-lnet.h +9 -5

index 7e89b3b..ec70f40 100644
@@ -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);

Signature · 1 line

2.1.0

Dan Carpenter

2014-11-24 23:40 UTC #2 lore raw

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

drivers/staging/lustre/include/linux/lnet/lib-lnet.h lnet_copy_iov2flat(int dlen, void *dest, unsigned int doffset, · line 789

Quoted 10 lines · Zahari Doychev

drivers/staging/lustre/include/linux/lnet/lib-lnet.h +2 -1

@@ -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

Dilger, Andreas

2014-11-25 00:49 UTC #3 lore raw

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

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

Quoted 25 lines · Dan Carpenter

drivers/staging/lustre/include/linux/lnet/lib-lnet.h lnet_copy_iov2flat(int dlen, void *dest, unsigned int doffset, · line 789
Quoted 12 lines · Zahari Doychev
drivers/staging/lustre/include/linux/lnet/lib-lnet.h +2 -1
@@ -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

Zahari Doychev

author

2014-11-25 07:34 UTC #4 lore raw

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

In reply to Dan Carpenter

Quoted 16 lines · Dan Carpenter

drivers/staging/lustre/include/linux/lnet/lib-lnet.h lnet_copy_iov2flat(int dlen, void *dest, unsigned int doffset, · line 789
Quoted 10 lines · Zahari Doychev
drivers/staging/lustre/include/linux/lnet/lib-lnet.h +2 -1
@@ -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.

Quoted 3 lines

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

Quoted 9 lines

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

Dan Carpenter

2014-11-25 07:52 UTC #5 lore raw

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

Quoted 18 lines · Zahari Doychev

Quoted 16 lines · Dan Carpenter
drivers/staging/lustre/include/linux/lnet/lib-lnet.h lnet_copy_iov2flat(int dlen, void *dest, unsigned int doffset, · line 789
Quoted 10 lines · Zahari Doychev
drivers/staging/lustre/include/linux/lnet/lib-lnet.h +2 -1
@@ -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.

Quoted 1 line

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

Zahari Doychev

author

2014-11-24 18:56 UTC #6 lore raw

PATCH 2/2 staging/lustre: fix sparse warnings

In reply to 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 <[email protected]>

---
 drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c +10 -5

index d29f5f1..971b877 100644
@@ -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;

Signature · 1 line

2.1.0

Greg KH

maintainer

2014-11-26 20:53 UTC #7 lore raw

In reply to Zahari Doychev

Quoted 9 lines · 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(-)

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 :(