[PATCH v2] mwifiex: replace kmalloc & memcpy sequence with kmemdup

[PATCH v2] mwifiex: replace kmalloc & memcpy sequence with kmemdup

[Bing Zhao]

From: Yogesh Ashok Powar <yogeshp@marvell.com>

Sequence of kmalloc/kzalloc and memcpy is replaced with
kmemdup.

Cc: Walter Harms <wharms@bfs.de>
Signed-off-by: Yogesh Ashok Powar <yogeshp@marvell.com>
Signed-off-by: Kiran Divekar <dkiran@marvell.com>
Signed-off-by: Bing Zhao <bzhao@marvell.com>
---
v2: Fixing OOMs as alloc failures instead of dup failures
    as suggested by Joe Perches <joe@perches.com>

 drivers/net/wireless/mwifiex/join.c      |    3 +--
 drivers/net/wireless/mwifiex/scan.c      |   13 ++++++++-----
 drivers/net/wireless/mwifiex/sta_ioctl.c |    9 +++++----
 3 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/mwifiex/join.c b/drivers/net/wireless/mwifiex/join.c
index 5cdad92..62b4c29 100644
--- a/drivers/net/wireless/mwifiex/join.c
+++ b/drivers/net/wireless/mwifiex/join.c
@@ -147,13 +147,12 @@ static int mwifiex_get_common_rates(struct mwifiex_private *priv, u8 *rate1,
 	u8 *ptr = rate1, *tmp;
 	u32 i, j;
 
-	tmp = kmalloc(rate1_size, GFP_KERNEL);
+	tmp = kmemdup(rate1, rate1_size, GFP_KERNEL);
 	if (!tmp) {
 		dev_err(priv->adapter->dev, "failed to alloc tmp buf\n");
 		return -ENOMEM;
 	}
 
-	memcpy(tmp, rate1, rate1_size);
 	memset(rate1, 0, rate1_size);
 
 	for (i = 0; rate2[i] && i < rate2_size; i++) {
diff --git a/drivers/net/wireless/mwifiex/scan.c b/drivers/net/wireless/mwifiex/scan.c
index 37ca2f9..f3b4a61 100644
--- a/drivers/net/wireless/mwifiex/scan.c
+++ b/drivers/net/wireless/mwifiex/scan.c
@@ -1479,12 +1479,12 @@ mwifiex_update_curr_bss_params(struct mwifiex_private *priv,
 		dev_err(priv->adapter->dev, " failed to alloc bss_desc\n");
 		return -ENOMEM;
 	}
-	beacon_ie = kzalloc(ie_len, GFP_KERNEL);
+
+	beacon_ie = kmemdup(ie_buf, ie_len, GFP_KERNEL);
 	if (!beacon_ie) {
 		dev_err(priv->adapter->dev, " failed to alloc beacon_ie\n");
 		return -ENOMEM;
 	}
-	memcpy(beacon_ie, ie_buf, ie_len);
 
 	ret = mwifiex_fill_new_bss_desc(priv, bssid, rssi, beacon_ie,
 					ie_len, beacon_period,
@@ -1986,17 +1986,20 @@ mwifiex_save_curr_bcn(struct mwifiex_private *priv)
 		priv->curr_bcn_size = curr_bss->beacon_buf_size;
 
 		kfree(priv->curr_bcn_buf);
-		priv->curr_bcn_buf = kzalloc(curr_bss->beacon_buf_size,
+
+		priv->curr_bcn_buf = kmemdup(curr_bss->beacon_buf,
+						curr_bss->beacon_buf_size,
 						GFP_KERNEL);
 		if (!priv->curr_bcn_buf) {
 			dev_err(priv->adapter->dev,
 					"failed to alloc curr_bcn_buf\n");
 			return;
 		}
+	} else {
+		memcpy(priv->curr_bcn_buf, curr_bss->beacon_buf,
+			curr_bss->beacon_buf_size);
 	}
 
-	memcpy(priv->curr_bcn_buf, curr_bss->beacon_buf,
-		curr_bss->beacon_buf_size);
 	dev_dbg(priv->adapter->dev, "info: current beacon saved %d\n",
 		priv->curr_bcn_size);
 
diff --git a/drivers/net/wireless/mwifiex/sta_ioctl.c b/drivers/net/wireless/mwifiex/sta_ioctl.c
index 3fca219..eb569fa 100644
--- a/drivers/net/wireless/mwifiex/sta_ioctl.c
+++ b/drivers/net/wireless/mwifiex/sta_ioctl.c
@@ -199,13 +199,14 @@ int mwifiex_bss_start(struct mwifiex_private *priv, struct cfg80211_bss *bss,
 			dev_err(priv->adapter->dev, " failed to alloc bss_desc\n");
 			return -ENOMEM;
 		}
-		beacon_ie = kzalloc(bss->len_beacon_ies, GFP_KERNEL);
+
+		beacon_ie = kmemdup(bss->information_elements,
+					bss->len_beacon_ies, GFP_KERNEL);
 		if (!beacon_ie) {
-			dev_err(priv->adapter->dev, " failed to alloc bss_desc\n");
+			dev_err(priv->adapter->dev, " failed to alloc beacon_ie\n");
 			return -ENOMEM;
 		}
-		memcpy(beacon_ie, bss->information_elements,
-		       bss->len_beacon_ies);
+
 		ret = mwifiex_fill_new_bss_desc(priv, bss->bssid, bss->signal,
 						beacon_ie, bss->len_beacon_ies,
 						bss->beacon_interval,
-- 
1.7.0.2

Re: [PATCH v2] mwifiex: replace kmalloc & memcpy sequence with kmemdup

[Joe Perches]

On Thu, 2011-08-25 at 12:37 -0700, Bing Zhao wrote:
> Sequence of kmalloc/kzalloc and memcpy is replaced with
> kmemdup.
> diff --git a/drivers/net/wireless/mwifiex/scan.c b/drivers/net/wireless/mwifiex/scan.c
[]
> @@ -1986,17 +1986,20 @@ mwifiex_save_curr_bcn(struct mwifiex_private *priv)
>  		priv->curr_bcn_size = curr_bss->beacon_buf_size;
>  
>  		kfree(priv->curr_bcn_buf);
> -		priv->curr_bcn_buf = kzalloc(curr_bss->beacon_buf_size,
> +
> +		priv->curr_bcn_buf = kmemdup(curr_bss->beacon_buf,
> +						curr_bss->beacon_buf_size,
>  						GFP_KERNEL);

I think this block, because of the new else below it,
isn't an improvement.

This kfree/kzalloc should probably just be krealloc with
the memset unchanged.

>  		if (!priv->curr_bcn_buf) {
>  			dev_err(priv->adapter->dev,
>  					"failed to alloc curr_bcn_buf\n");
>  			return;
>  		}
> +	} else {
> +		memcpy(priv->curr_bcn_buf, curr_bss->beacon_buf,
> +			curr_bss->beacon_buf_size);
>  	}
>  
> -	memcpy(priv->curr_bcn_buf, curr_bss->beacon_buf,
> -		curr_bss->beacon_buf_size);
>  	dev_dbg(priv->adapter->dev, "info: current beacon saved %d\n",
>  		priv->curr_bcn_size);

RE: [PATCH v2] mwifiex: replace kmalloc & memcpy sequence with kmemdup

[Bing Zhao]

SGkgSm9lLA0KDQpUaGFua3MgZm9yIHlvdXIgY29tbWVudC4NCg0KPiA+IFNlcXVlbmNlIG9mIGtt
YWxsb2Mva3phbGxvYyBhbmQgbWVtY3B5IGlzIHJlcGxhY2VkIHdpdGgNCj4gPiBrbWVtZHVwLg0K
PiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL25ldC93aXJlbGVzcy9td2lmaWV4L3NjYW4uYyBiL2Ry
aXZlcnMvbmV0L3dpcmVsZXNzL213aWZpZXgvc2Nhbi5jDQo+IFtdDQo+ID4gQEAgLTE5ODYsMTcg
KzE5ODYsMjAgQEAgbXdpZmlleF9zYXZlX2N1cnJfYmNuKHN0cnVjdCBtd2lmaWV4X3ByaXZhdGUg
KnByaXYpDQo+ID4gIAkJcHJpdi0+Y3Vycl9iY25fc2l6ZSA9IGN1cnJfYnNzLT5iZWFjb25fYnVm
X3NpemU7DQo+ID4NCj4gPiAgCQlrZnJlZShwcml2LT5jdXJyX2Jjbl9idWYpOw0KPiA+IC0JCXBy
aXYtPmN1cnJfYmNuX2J1ZiA9IGt6YWxsb2MoY3Vycl9ic3MtPmJlYWNvbl9idWZfc2l6ZSwNCj4g
PiArDQo+ID4gKwkJcHJpdi0+Y3Vycl9iY25fYnVmID0ga21lbWR1cChjdXJyX2Jzcy0+YmVhY29u
X2J1ZiwNCj4gPiArCQkJCQkJY3Vycl9ic3MtPmJlYWNvbl9idWZfc2l6ZSwNCj4gPiAgCQkJCQkJ
R0ZQX0tFUk5FTCk7DQo+IA0KPiBJIHRoaW5rIHRoaXMgYmxvY2ssIGJlY2F1c2Ugb2YgdGhlIG5l
dyBlbHNlIGJlbG93IGl0LA0KPiBpc24ndCBhbiBpbXByb3ZlbWVudC4NCg0KT0suIFRoZW4gd2Ug
Y2FuIGp1c3QgdXNlIGttYWxsb2MgaW5zdGVhZCBvZiBremFsbG9jIHRvIHNhdmUgYSBidWZmZXIg
emVyb2luZyBvdXQgaW4gdjMuDQoNCj4gDQo+IFRoaXMga2ZyZWUva3phbGxvYyBzaG91bGQgcHJv
YmFibHkganVzdCBiZSBrcmVhbGxvYyB3aXRoDQo+IHRoZSBtZW1zZXQgdW5jaGFuZ2VkLg0KDQpU
aGUga3JlYWxsb2MgZG9lc24ndCBzZWVtIHN1aXRhYmxlIGhlcmUgYmVjYXVzZSBrcmVhbGxvYyBk
b2VzIGEgdXNlbGVzcyBtZW1jcHkuDQoNClRoYW5rcywNCkJpbmcNCg0KPiANCj4gPiAgCQlpZiAo
IXByaXYtPmN1cnJfYmNuX2J1Zikgew0KPiA+ICAJCQlkZXZfZXJyKHByaXYtPmFkYXB0ZXItPmRl
diwNCj4gPiAgCQkJCQkiZmFpbGVkIHRvIGFsbG9jIGN1cnJfYmNuX2J1ZlxuIik7DQo+ID4gIAkJ
CXJldHVybjsNCj4gPiAgCQl9DQo+ID4gKwl9IGVsc2Ugew0KPiA+ICsJCW1lbWNweShwcml2LT5j
dXJyX2Jjbl9idWYsIGN1cnJfYnNzLT5iZWFjb25fYnVmLA0KPiA+ICsJCQljdXJyX2Jzcy0+YmVh
Y29uX2J1Zl9zaXplKTsNCj4gPiAgCX0NCj4gPg0KPiA+IC0JbWVtY3B5KHByaXYtPmN1cnJfYmNu
X2J1ZiwgY3Vycl9ic3MtPmJlYWNvbl9idWYsDQo+ID4gLQkJY3Vycl9ic3MtPmJlYWNvbl9idWZf
c2l6ZSk7DQo+ID4gIAlkZXZfZGJnKHByaXYtPmFkYXB0ZXItPmRldiwgImluZm86IGN1cnJlbnQg
YmVhY29uIHNhdmVkICVkXG4iLA0KPiA+ICAJCXByaXYtPmN1cnJfYmNuX3NpemUpOw0KPiANCg0K