Re: [PATCH] zd1211rw: Readd zd_addr_t cast
On Sat, 2007-02-10 at 07:02 +0100, Michael Buesch wrote: On Saturday 10 February 2007 02:27, Daniel Drake wrote: Robert P.J. Day's recent commit (getting rid of all casts of k[cmz]alloc() calls) introduced a sparse warning for zd1211rw, related to our type-checking of addresses. zd_chip.c:116:15: warning: implicit cast to nocast type This patch readds the type cast, it is correct. Signed-off-by: Daniel Drake [EMAIL PROTECTED] Index: linux/drivers/net/wireless/zd1211rw/zd_chip.c === --- linux.orig/drivers/net/wireless/zd1211rw/zd_chip.c +++ linux/drivers/net/wireless/zd1211rw/zd_chip.c @@ -113,7 +113,7 @@ int zd_ioread32v_locked(struct zd_chip * /* Allocate a single memory block for values and addresses. */ count16 = 2*count; - a16 = kmalloc(count16 * (sizeof(zd_addr_t) + sizeof(u16)), + a16 = (zd_addr_t *) kmalloc(count16 * (sizeof(zd_addr_t) + sizeof(u16)), GFP_NOFS); I think it better to remove this __nocast instead. This is the only use of __nocast outside of kmem in the kernel tree. Why not just remove __nocast and use u16 instead of zd_addr_t? If not the removal of the __nocast, would it be better to have a void pointer there? tmp = kcalloc(count16, sizeof(zd_addr_t) + sizeof(u16), GFP_KERNEL); [] a16 = (zd_addr_t *)tmp; v16 = a16 + count16; [] kfree(tmp); - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] zd1211rw: Readd zd_addr_t cast
On 07-02-10 10:51 Joe Perches wrote: On Sat, 2007-02-10 at 07:02 +0100, Michael Buesch wrote: On Saturday 10 February 2007 02:27, Daniel Drake wrote: Robert P.J. Day's recent commit (getting rid of all casts of k[cmz]alloc() calls) introduced a sparse warning for zd1211rw, related to our type-checking of addresses. zd_chip.c:116:15: warning: implicit cast to nocast type This patch readds the type cast, it is correct. Signed-off-by: Daniel Drake [EMAIL PROTECTED] Index: linux/drivers/net/wireless/zd1211rw/zd_chip.c === --- linux.orig/drivers/net/wireless/zd1211rw/zd_chip.c +++ linux/drivers/net/wireless/zd1211rw/zd_chip.c @@ -113,7 +113,7 @@ int zd_ioread32v_locked(struct zd_chip * /* Allocate a single memory block for values and addresses. */ count16 = 2*count; - a16 = kmalloc(count16 * (sizeof(zd_addr_t) + sizeof(u16)), + a16 = (zd_addr_t *) kmalloc(count16 * (sizeof(zd_addr_t) + sizeof(u16)), GFP_NOFS); I think it better to remove this __nocast instead. This is the only use of __nocast outside of kmem in the kernel tree. Why not just remove __nocast and use u16 instead of zd_addr_t? Using the __nocast type sparse catches errors, when address and value arguments are used in the wrong order for calls of the zd_ioread* and zd_iowrite* functions. I'm a lazy guy, I like tools do the hard work for me. If not the removal of the __nocast, would it be better to have a void pointer there? I don't think that an explicit cast requires the introduction of a temporary variable. -- Uli Kunitz - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] zd1211rw: Readd zd_addr_t cast
Robert P.J. Day's recent commit (getting rid of all casts of k[cmz]alloc()
calls) introduced a sparse warning for zd1211rw, related to our type-checking
of addresses.
zd_chip.c:116:15: warning: implicit cast to nocast type
This patch readds the type cast, it is correct.
Signed-off-by: Daniel Drake [EMAIL PROTECTED]
Index: linux/drivers/net/wireless/zd1211rw/zd_chip.c
===
--- linux.orig/drivers/net/wireless/zd1211rw/zd_chip.c
+++ linux/drivers/net/wireless/zd1211rw/zd_chip.c
@@ -113,7 +113,7 @@ int zd_ioread32v_locked(struct zd_chip *
/* Allocate a single memory block for values and addresses. */
count16 = 2*count;
- a16 = kmalloc(count16 * (sizeof(zd_addr_t) + sizeof(u16)),
+ a16 = (zd_addr_t *) kmalloc(count16 * (sizeof(zd_addr_t) + sizeof(u16)),
GFP_NOFS);
if (!a16) {
dev_dbg_f(zd_chip_dev(chip),
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] zd1211rw: Readd zd_addr_t cast
On Saturday 10 February 2007 02:27, Daniel Drake wrote: Robert P.J. Day's recent commit (getting rid of all casts of k[cmz]alloc() calls) introduced a sparse warning for zd1211rw, related to our type-checking of addresses. zd_chip.c:116:15: warning: implicit cast to nocast type This patch readds the type cast, it is correct. Signed-off-by: Daniel Drake [EMAIL PROTECTED] Index: linux/drivers/net/wireless/zd1211rw/zd_chip.c === --- linux.orig/drivers/net/wireless/zd1211rw/zd_chip.c +++ linux/drivers/net/wireless/zd1211rw/zd_chip.c @@ -113,7 +113,7 @@ int zd_ioread32v_locked(struct zd_chip * /* Allocate a single memory block for values and addresses. */ count16 = 2*count; - a16 = kmalloc(count16 * (sizeof(zd_addr_t) + sizeof(u16)), + a16 = (zd_addr_t *) kmalloc(count16 * (sizeof(zd_addr_t) + sizeof(u16)), GFP_NOFS); Unrelated, but I am wondering since quite some time why you pass GFP_NOFS here. Any special reason? I think in general there is no good reason for code outside of the VFS to use this flag. IMHO you should pass either GFP_ATOMIC or GFP_KERNEL. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] zd1211rw: Readd zd_addr_t cast
On 07-02-10 07:02 Michael Buesch wrote: On Saturday 10 February 2007 02:27, Daniel Drake wrote: Robert P.J. Day's recent commit (getting rid of all casts of k[cmz]alloc() calls) introduced a sparse warning for zd1211rw, related to our type-checking of addresses. zd_chip.c:116:15: warning: implicit cast to nocast type This patch readds the type cast, it is correct. Signed-off-by: Daniel Drake [EMAIL PROTECTED] Index: linux/drivers/net/wireless/zd1211rw/zd_chip.c === --- linux.orig/drivers/net/wireless/zd1211rw/zd_chip.c +++ linux/drivers/net/wireless/zd1211rw/zd_chip.c @@ -113,7 +113,7 @@ int zd_ioread32v_locked(struct zd_chip * /* Allocate a single memory block for values and addresses. */ count16 = 2*count; - a16 = kmalloc(count16 * (sizeof(zd_addr_t) + sizeof(u16)), + a16 = (zd_addr_t *) kmalloc(count16 * (sizeof(zd_addr_t) + sizeof(u16)), GFP_NOFS); Unrelated, but I am wondering since quite some time why you pass GFP_NOFS here. Any special reason? I think in general there is no good reason for code outside of the VFS to use this flag. IMHO you should pass either GFP_ATOMIC or GFP_KERNEL. Michael, you are right. The theory has been to prevent the NFS operations while being in the kernel. But no driver in drivers/net is using it. I will exchange it by GFP_KERNEL, this code is protected by a mutex and is not atomic. Ciao, Uli -- Uli Kunitz - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
