IB/core: avoid 32-bit warning

PATCH In review Arnd Bergmann 4 participants 6 messages Oct 7, 2015 – Oct 21, 2015 InfiniBand

Share & tools

Arnd Bergmann

author

2015-10-07 12:29 UTC #0 lore raw

PATCH 1/1 IB/core: avoid 32-bit warning

The INIT_UDATA() macro requires a pointer or unsigned long argument for both input and output buffer, and all callers had a cast from when the code was merged until a recent restructuring, so now we get

core/uverbs_cmd.c: In function 'ib_uverbs_create_cq':
core/uverbs_cmd.c:1481:66: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]

This makes the code behave as before by adding back the cast to unsigned long.

Signed-off-by Arnd Bergmann <[email protected]>

Fixes 565197dd8fb1 ("IB/core: Extend ib_uverbs_create_cq")

drivers/infiniband/core/uverbs_cmd.c +1 -1

index be4cb9f04be3..88b3b78340f2 100644
@@ -1478,7 +1478,7 @@ ssize_t ib_uverbs_create_cq(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof(cmd)))
 		return -EFAULT;
 
-	INIT_UDATA(&ucore, buf, cmd.response, sizeof(cmd), sizeof(resp));
+	INIT_UDATA(&ucore, buf, (unsigned long)cmd.response, sizeof(cmd), sizeof(resp));
 
 	INIT_UDATA(&uhw, buf + sizeof(cmd),
 		   (unsigned long)cmd.response + sizeof(resp),

Yann Droneaud

Reviewed-by on v1 1/1

2015-10-07 13:00 UTC #1 lore raw

Hi,

Le mercredi 07 octobre 2015 ? 14:29 +0200, Arnd Bergmann a ?crit :

Quoted 8 lines

The INIT_UDATA() macro requires a pointer or unsigned long argument for both input and output buffer, and all callers had a cast from when the code was merged until a recent restructuring, so now we get
core/uverbs_cmd.c: In function 'ib_uverbs_create_cq':
core/uverbs_cmd.c:1481:66: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]

Quoted 6 lines

This makes the code behave as before by adding back the cast to unsigned long.
Signed-off-by Arnd Bergmann <[email protected]>
Fixes 565197dd8fb1 ("IB/core: Extend ib_uverbs_create_cq")

Reviewed-by Yann Droneaud <[email protected]>

Quoted 16 lines

drivers/infiniband/core/uverbs_cmd.c +1 -1

index be4cb9f04be3..88b3b78340f2 100644
@@ -1478,7 +1478,7 @@ ssize_t ib_uverbs_create_cq(struct
 ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof(cmd)))
 		return -EFAULT;
 
-	INIT_UDATA(&ucore, buf, cmd.response, sizeof(cmd),
 sizeof(resp));
+	INIT_UDATA(&ucore, buf, (unsigned long)cmd.response,
 sizeof(cmd), sizeof(resp));

drivers/infiniband/core/uverbs_cmd.c ssize_t ib_uverbs_create_cq(struct ib_uverbs_file *file, · line 1484

Quoted 2 lines

drivers/infiniband/core/uverbs_cmd.c +0 -0

 	INIT_UDATA(&uhw, buf + sizeof(cmd),
 		   (unsigned long)cmd.response + sizeof(resp),

Regards.

Signature · 2 lines

Yann Droneaud
OPTEYA

Sagi Grimberg

2015-10-07 13:19 UTC #2 lore raw

In reply to Arnd Bergmann

Quoted 18 lines · Arnd Bergmann

The INIT_UDATA() macro requires a pointer or unsigned long argument for both input and output buffer, and all callers had a cast from when the code was merged until a recent restructuring, so now we get
core/uverbs_cmd.c: In function 'ib_uverbs_create_cq':
core/uverbs_cmd.c:1481:66: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
This makes the code behave as before by adding back the cast to unsigned long.
Signed-off-by Arnd Bergmann <[email protected]>
Fixes 565197dd8fb1 ("IB/core: Extend ib_uverbs_create_cq")
drivers/infiniband/core/uverbs_cmd.c +0 -0
index be4cb9f04be3..88b3b78340f2 100644

drivers/infiniband/core/uverbs_cmd.c ssize_t ib_uverbs_create_cq(struct ib_uverbs_file *file, · line 1481

Quoted 5 lines

drivers/infiniband/core/uverbs_cmd.c +1 -1

@@ -1478,7 +1478,7 @@ ssize_t ib_uverbs_create_cq(struct ib_uverbs_file *file,
  	if (copy_from_user(&cmd, buf, sizeof(cmd)))
  		return -EFAULT;
 
-	INIT_UDATA(&ucore, buf, cmd.response, sizeof(cmd), sizeof(resp));
+	INIT_UDATA(&ucore, buf, (unsigned long)cmd.response, sizeof(cmd), sizeof(resp));

Would it make sense to cast inside INIT_UDATA() and not have callers worry about it?

Arnd Bergmann

author

2015-10-07 13:46 UTC #3 lore raw

Quoted 22 lines · On Wednesday 07 October 2015 16:19:27 Sagi Grimberg wrote:

Quoted 21 lines · Arnd Bergmann
The INIT_UDATA() macro requires a pointer or unsigned long argument for both input and output buffer, and all callers had a cast from when the code was merged until a recent restructuring, so now we get
core/uverbs_cmd.c: In function 'ib_uverbs_create_cq':
core/uverbs_cmd.c:1481:66: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
This makes the code behave as before by adding back the cast to unsigned long.
Signed-off-by Arnd Bergmann <[email protected]>
Fixes 565197dd8fb1 ("IB/core: Extend ib_uverbs_create_cq")
drivers/infiniband/core/uverbs_cmd.c +0 -0
index be4cb9f04be3..88b3b78340f2 100644
@@ -1478,7 +1478,7 @@ ssize_t ib_uverbs_create_cq(struct ib_uverbs_file *file,
      if (copy_from_user(&cmd, buf, sizeof(cmd)))
              return -EFAULT;
 

Quoted 5 lines

drivers/infiniband/core/uverbs_cmd.c ssize_t ib_uverbs_create_cq(struct ib_uverbs_file *file, · line 1481
Quoted 2 lines
drivers/infiniband/core/uverbs_cmd.c +1 -1
-     INIT_UDATA(&ucore, buf, cmd.response, sizeof(cmd), sizeof(resp));
+     INIT_UDATA(&ucore, buf, (unsigned long)cmd.response, sizeof(cmd), sizeof(resp));
Would it make sense to cast inside INIT_UDATA() and not have callers worry about it?

If we want to do this properly, INIT_UDATA should be converted into a function call that has the right types.

My patch doesn't try to do that here, it just restores the code to how it was for the past 10 years.

Arnd

Yann Droneaud

2015-10-07 13:46 UTC #4 lore raw

In reply to Sagi Grimberg

Hi,

Le mercredi 07 octobre 2015 ? 16:19 +0300, Sagi Grimberg a ?crit :

Quoted 26 lines · Arnd Bergmann

Quoted 25 lines
The INIT_UDATA() macro requires a pointer or unsigned long argument for
both input and output buffer, and all callers had a cast from when the code was merged until a recent restructuring, so now we get
core/uverbs_cmd.c: In function 'ib_uverbs_create_cq':
core/uverbs_cmd.c:1481:66: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
This makes the code behave as before by adding back the cast to unsigned long.
Signed-off-by Arnd Bergmann <[email protected]>
Fixes 565197dd8fb1 ("IB/core: Extend ib_uverbs_create_cq")
drivers/infiniband/core/uverbs_cmd.c +0 -0
index be4cb9f04be3..88b3b78340f2 100644
@@ -1478,7 +1478,7 @@ ssize_t ib_uverbs_create_cq(struct
 ib_uverbs_file *file,
  	if (copy_from_user(&cmd, buf, sizeof(cmd)))
  		return -EFAULT;
 

Quoted 7 lines

drivers/infiniband/core/uverbs_cmd.c ssize_t ib_uverbs_create_cq(struct · line 1484
Quoted 4 lines
drivers/infiniband/core/uverbs_cmd.c +1 -1
-	INIT_UDATA(&ucore, buf, cmd.response, sizeof(cmd),
 sizeof(resp));
+	INIT_UDATA(&ucore, buf, (unsigned long)cmd.response,
 sizeof(cmd), sizeof(resp));
Would it make sense to cast inside INIT_UDATA() and not have callers worry about it?

It's ... complicated. See INIT_UDATA_BUF_OR_NULL().

Awayway, I have patch to do the opposite, eg. explicitly cast u64 value to (void __user *)(unsigned long) in the caller function instead, as it allows some safer / new operations on the response buffer.

Regards.

Signature · 2 lines

Yann Droneaud
OPTEYA

Doug Ledford

2015-10-21 20:58 UTC #5 lore raw

Quoted 37 lines · Yann Droneaud

Hi,
Le mercredi 07 octobre 2015 ? 16:19 +0300, Sagi Grimberg a ?crit :
Quoted 26 lines · Arnd Bergmann
Quoted 25 lines
The INIT_UDATA() macro requires a pointer or unsigned long argument for
both input and output buffer, and all callers had a cast from when the code was merged until a recent restructuring, so now we get
core/uverbs_cmd.c: In function 'ib_uverbs_create_cq':
core/uverbs_cmd.c:1481:66: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
This makes the code behave as before by adding back the cast to unsigned long.
Signed-off-by Arnd Bergmann <[email protected]>
Fixes 565197dd8fb1 ("IB/core: Extend ib_uverbs_create_cq")
drivers/infiniband/core/uverbs_cmd.c +0 -0
index be4cb9f04be3..88b3b78340f2 100644
@@ -1478,7 +1478,7 @@ ssize_t ib_uverbs_create_cq(struct
 ib_uverbs_file *file,
  	if (copy_from_user(&cmd, buf, sizeof(cmd)))
  		return -EFAULT;
 
Quoted 7 lines
drivers/infiniband/core/uverbs_cmd.c ssize_t ib_uverbs_create_cq(struct · line 1484
Quoted 4 lines
drivers/infiniband/core/uverbs_cmd.c +1 -1
-	INIT_UDATA(&ucore, buf, cmd.response, sizeof(cmd),
 sizeof(resp));
+	INIT_UDATA(&ucore, buf, (unsigned long)cmd.response,
 sizeof(cmd), sizeof(resp));
Would it make sense to cast inside INIT_UDATA() and not have callers worry about it?

Quoted 8 lines

It's ... complicated. See INIT_UDATA_BUF_OR_NULL().
Awayway, I have patch to do the opposite, eg. explicitly cast u64 value to (void __user *)(unsigned long) in the caller function instead, as it allows some safer / new operations on the response buffer.
Regards.

I haven't seen this oether patch that Yann is talking about, so I've taken this one.

Signature · 11 lines

Doug Ledford <[email protected]>
              GPG KeyID: 0E572FDD


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 884 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151021/71b41919/attachment-0001.sig>