[PATCH 1/1 linux-next] zd1201: replace kmalloc/memset by kzalloc

[PATCH 1/1 linux-next] zd1201: replace kmalloc/memset by kzalloc

[Fabian Frederick]

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 drivers/net/wireless/zd1201.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/wireless/zd1201.c b/drivers/net/wireless/zd1201.c
index 6f5c793..4630b26 100644
--- a/drivers/net/wireless/zd1201.c
+++ b/drivers/net/wireless/zd1201.c
@@ -525,7 +525,7 @@ static int zd1201_setconfig(struct zd1201 *zd, int rid, void *buf, int len, int
 	zd->rxdatas = 0;
 	zd->rxlen = 0;
 	for (seq=0; len > 0; seq++) {
-		request = kmalloc(16, gfp_mask);
+		request = kzalloc(16, gfp_mask);
 		if (!request)
 			return -ENOMEM;
 		urb = usb_alloc_urb(0, gfp_mask);
@@ -533,7 +533,6 @@ static int zd1201_setconfig(struct zd1201 *zd, int rid, void *buf, int len, int
 			kfree(request);
 			return -ENOMEM;
 		}
-		memset(request, 0, 16);
 		reqlen = len>12 ? 12 : len;
 		request[0] = ZD1201_USB_RESREQ;
 		request[1] = seq;
-- 
1.9.3

Re: [PATCH 1/1 linux-next] zd1201: replace kmalloc/memset by kzalloc

[Joe Perches]

On Tue, 2014-10-14 at 18:40 +0200, Fabian Frederick wrote:
> Signed-off-by: Fabian Frederick <fabf@skynet.be>

It might be clearer to use a structure for this 16 byte thing.

There's a comment bit in the code:

/* cmdreq message: 
	u32 type
	u16 cmd
	u16 parm0
	u16 parm1
	u16 parm2
	u8  pad[4]

	total: 4 + 2 + 2 + 2 + 2 + 4 = 16
*/

It seems thought that this first u32 is actually
a series of u8s.

Maybe:

struct zd1201_cmdreq {
	u8	type;
	u8	seq;
	u8	a;
	u8	b;
	__le16	cmd;
	__le16	parm0;
	__le16	parm1;
	__le16	parm2;
	u8	pad[4];
};

And does this really need to be alloc'd?

maybe

struct zd1202_cmdreq request = {};

etc...

Re: [PATCH 1/1 linux-next] zd1201: replace kmalloc/memset by kzalloc

[Bjørn Mork]

Joe Perches <joe@perches.com> writes:

> And does this really need to be alloc'd?

Yes, it does. It is used as a transfer_buffer in usb_fill_bulk_urb() and
must be "suitable for DMA".  From include/linux/usb.h:

/**
 * struct urb - USB Request Block
..
 * @transfer_buffer:  This identifies the buffer to (or from) which the I/O
 *      request will be performed unless URB_NO_TRANSFER_DMA_MAP is set
 *      (however, do not leave garbage in transfer_buffer even then).
 *      This buffer must be suitable for DMA; allocate it with
 *      kmalloc() or equivalent.  For transfers to "in" endpoints, contents
 *      of this buffer will be modified.  This buffer is used for the data
 *      stage of control transfers.



Bjørn

Re: [PATCH 1/1 linux-next] zd1201: replace kmalloc/memset by kzalloc

[Fabian Frederick]

> On 14 October 2014 at 20:08 Bjørn Mork <bjorn@mork.no> wrote:
>
>
> Joe Perches <joe@perches.com> writes:
>
> > And does this really need to be alloc'd?
>
> Yes, it does. It is used as a transfer_buffer in usb_fill_bulk_urb() and
> must be "suitable for DMA".  From include/linux/usb.h:
>
> /**
>  * struct urb - USB Request Block
> ..
>  * @transfer_buffer:  This identifies the buffer to (or from) which the I/O
>  *      request will be performed unless URB_NO_TRANSFER_DMA_MAP is set
>  *      (however, do not leave garbage in transfer_buffer even then).
>  *      This buffer must be suitable for DMA; allocate it with
>  *      kmalloc() or equivalent.  For transfers to "in" endpoints, contents
>  *      of this buffer will be modified.  This buffer is used for the data
>  *      stage of control transfers.
>
>

Adding a structure would be a nice design update indeed but this would require
some testing...Separate patch maybe ?
 
Fabian

>
> Bjørn

Re: [PATCH 1/1 linux-next] zd1201: replace kmalloc/memset by kzalloc

[Joe Perches]

On Thu, 2014-10-16 at 10:08 +0200, Fabian Frederick wrote:
> Adding a structure would be a nice design update indeed but this would require
> some testing...Separate patch maybe ?

Of course that'd be fine.