diff mbox

[03/30] staging: wilc1000: fix line over 80 chars in handle_key()

Message ID 1525682614-3824-4-git-send-email-ajay.kathat@microchip.com (mailing list archive)
State Not Applicable
Delegated to: Kalle Valo
Headers show

Commit Message

Ajay Singh May 7, 2018, 8:43 a.m. UTC 
Fix checkpatch reported issue of line over 80 char in handle_key().
Introduced new functions by spliting existing function to address the
checkpatch issue.

Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
---
 drivers/staging/wilc1000/host_interface.c | 59 +++++++++++++++++++------------
 1 file changed, 37 insertions(+), 22 deletions(-)

Comments

Claudiu Beznea May 9, 2018, 1:44 p.m. UTC | #1 
On 07.05.2018 11:43, Ajay Singh wrote:
> Fix checkpatch reported issue of line over 80 char in handle_key().
> Introduced new functions by spliting existing function to address the
> checkpatch issue.
> 
> Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
> ---
>  drivers/staging/wilc1000/host_interface.c | 59 +++++++++++++++++++------------
>  1 file changed, 37 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
> index 4ba868c..29f9907 100644
> --- a/drivers/staging/wilc1000/host_interface.c
> +++ b/drivers/staging/wilc1000/host_interface.c
> @@ -1498,12 +1498,45 @@ static s32 handle_rcvd_gnrl_async_info(struct wilc_vif *vif,
>  	return result;
>  }
>  
> +static int wilc_pmksa_key_copy(struct wilc_vif *vif, struct key_attr *hif_key)
> +{
> +	int i;
> +	int ret;
> +	struct wid wid;
> +	u8 *key_buf;
> +
> +	key_buf = kmalloc((hif_key->attr.pmkid.numpmkid * PMKSA_KEY_LEN) + 1,
> +			  GFP_KERNEL);
> +	if (!key_buf)
> +		return -ENOMEM;
> +
> +	key_buf[0] = hif_key->attr.pmkid.numpmkid;
> +
> +	for (i = 0; i < hif_key->attr.pmkid.numpmkid; i++) {
> +		memcpy(key_buf + ((PMKSA_KEY_LEN * i) + 1),
> +		       hif_key->attr.pmkid.pmkidlist[i].bssid, ETH_ALEN);
> +		memcpy(key_buf + ((PMKSA_KEY_LEN * i) + ETH_ALEN + 1),
> +		       hif_key->attr.pmkid.pmkidlist[i].pmkid, PMKID_LEN);
> +	}
> +
> +	wid.id = (u16)WID_PMKID_INFO;
> +	wid.type = WID_STR;
> +	wid.val = (s8 *)key_buf;

Is this cast really needed?

> +	wid.size = (hif_key->attr.pmkid.numpmkid * PMKSA_KEY_LEN) + 1;
> +
> +	ret = wilc_send_config_pkt(vif, SET_CFG, &wid, 1,
> +				   wilc_get_vif_idx(vif));
> +
> +	kfree(key_buf);
> +
> +	return ret;
> +}
> +
>  static int handle_key(struct wilc_vif *vif, struct key_attr *hif_key)
>  {
>  	int result = 0;
>  	struct wid wid;
>  	struct wid wid_list[5];
> -	u8 i;
>  	u8 *key_buf;
>  	s8 s8idxarray[1];
>  	struct host_if_drv *hif_drv = vif->hif_drv;
> @@ -1547,7 +1580,8 @@ static int handle_key(struct wilc_vif *vif, struct key_attr *hif_key)
>  						      wilc_get_vif_idx(vif));
>  			kfree(key_buf);
>  		} else if (hif_key->action & ADDKEY) {
> -			key_buf = kmalloc(hif_key->attr.wep.key_len + 2, GFP_KERNEL);
> +			key_buf = kmalloc(hif_key->attr.wep.key_len + 2,
> +					  GFP_KERNEL);
>  			if (!key_buf) {
>  				result = -ENOMEM;
>  				goto out_wep;
> @@ -1715,26 +1749,7 @@ static int handle_key(struct wilc_vif *vif, struct key_attr *hif_key)
>  		break;
>  
>  	case PMKSA:
> -		key_buf = kmalloc((hif_key->attr.pmkid.numpmkid * PMKSA_KEY_LEN) + 1, GFP_KERNEL);
> -		if (!key_buf)
> -			return -ENOMEM;
> -
> -		key_buf[0] = hif_key->attr.pmkid.numpmkid;
> -
> -		for (i = 0; i < hif_key->attr.pmkid.numpmkid; i++) {
> -			memcpy(key_buf + ((PMKSA_KEY_LEN * i) + 1), hif_key->attr.pmkid.pmkidlist[i].bssid, ETH_ALEN);
> -			memcpy(key_buf + ((PMKSA_KEY_LEN * i) + ETH_ALEN + 1), hif_key->attr.pmkid.pmkidlist[i].pmkid, PMKID_LEN);
> -		}
> -
> -		wid.id = (u16)WID_PMKID_INFO;
> -		wid.type = WID_STR;
> -		wid.val = (s8 *)key_buf;
> -		wid.size = (hif_key->attr.pmkid.numpmkid * PMKSA_KEY_LEN) + 1;
> -
> -		result = wilc_send_config_pkt(vif, SET_CFG, &wid, 1,
> -					      wilc_get_vif_idx(vif));
> -
> -		kfree(key_buf);
> +		result = wilc_pmksa_key_copy(vif, hif_key);
>  		break;
>  	}
>  
>
Ajay Singh May 9, 2018, 6:36 p.m. UTC | #2 
On Wed, 9 May 2018 16:44:47 +0300
Claudiu Beznea <Claudiu.Beznea@microchip.com> wrote:

> On 07.05.2018 11:43, Ajay Singh wrote:
> > Fix checkpatch reported issue of line over 80 char in handle_key().
> > Introduced new functions by spliting existing function to address
> > the checkpatch issue.
> > 
> > Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
> > ---
> >  drivers/staging/wilc1000/host_interface.c | 59
> > +++++++++++++++++++------------ 1 file changed, 37 insertions(+),
> > 22 deletions(-)
> > 
> > diff --git a/drivers/staging/wilc1000/host_interface.c
> > b/drivers/staging/wilc1000/host_interface.c index 4ba868c..29f9907
> > 100644 --- a/drivers/staging/wilc1000/host_interface.c
> > +++ b/drivers/staging/wilc1000/host_interface.c
> > @@ -1498,12 +1498,45 @@ static s32
> > handle_rcvd_gnrl_async_info(struct wilc_vif *vif, return result;
> >  }
> >  
> > +static int wilc_pmksa_key_copy(struct wilc_vif *vif, struct
> > key_attr *hif_key) +{
> > +	int i;
> > +	int ret;
> > +	struct wid wid;
> > +	u8 *key_buf;
> > +
> > +	key_buf = kmalloc((hif_key->attr.pmkid.numpmkid *
> > PMKSA_KEY_LEN) + 1,
> > +			  GFP_KERNEL);
> > +	if (!key_buf)
> > +		return -ENOMEM;
> > +
> > +	key_buf[0] = hif_key->attr.pmkid.numpmkid;
> > +
> > +	for (i = 0; i < hif_key->attr.pmkid.numpmkid; i++) {
> > +		memcpy(key_buf + ((PMKSA_KEY_LEN * i) + 1),
> > +		       hif_key->attr.pmkid.pmkidlist[i].bssid,
> > ETH_ALEN);
> > +		memcpy(key_buf + ((PMKSA_KEY_LEN * i) + ETH_ALEN +
> > 1),
> > +		       hif_key->attr.pmkid.pmkidlist[i].pmkid,
> > PMKID_LEN);
> > +	}
> > +
> > +	wid.id = (u16)WID_PMKID_INFO;
> > +	wid.type = WID_STR;
> > +	wid.val = (s8 *)key_buf;  
> 
> Is this cast really needed?
> 

This patch is only to address line over 80 chars checkpatch issue.
It is not good to club these change with type cast related
modification. For removing unnecessary cast we can submit
a separate patch series.
These are my views. What do you think?


> > +	wid.size = (hif_key->attr.pmkid.numpmkid * PMKSA_KEY_LEN)
> > + 1; +
> > +	ret = wilc_send_config_pkt(vif, SET_CFG, &wid, 1,
> > +				   wilc_get_vif_idx(vif));
> > +
> > +	kfree(key_buf);
> > +
> > +	return ret;
> > +}
> > +
> >  static int handle_key(struct wilc_vif *vif, struct key_attr
> > *hif_key) {
> >  	int result = 0;
> >  	struct wid wid;
> >  	struct wid wid_list[5];
> > -	u8 i;
> >  	u8 *key_buf;
> >  	s8 s8idxarray[1];
> >  	struct host_if_drv *hif_drv = vif->hif_drv;
> > @@ -1547,7 +1580,8 @@ static int handle_key(struct wilc_vif *vif,
> > struct key_attr *hif_key) wilc_get_vif_idx(vif));
> >  			kfree(key_buf);
> >  		} else if (hif_key->action & ADDKEY) {
> > -			key_buf =
> > kmalloc(hif_key->attr.wep.key_len + 2, GFP_KERNEL);
> > +			key_buf =
> > kmalloc(hif_key->attr.wep.key_len + 2,
> > +					  GFP_KERNEL);
> >  			if (!key_buf) {
> >  				result = -ENOMEM;
> >  				goto out_wep;
> > @@ -1715,26 +1749,7 @@ static int handle_key(struct wilc_vif *vif,
> > struct key_attr *hif_key) break;
> >  
> >  	case PMKSA:
> > -		key_buf = kmalloc((hif_key->attr.pmkid.numpmkid *
> > PMKSA_KEY_LEN) + 1, GFP_KERNEL);
> > -		if (!key_buf)
> > -			return -ENOMEM;
> > -
> > -		key_buf[0] = hif_key->attr.pmkid.numpmkid;
> > -
> > -		for (i = 0; i < hif_key->attr.pmkid.numpmkid; i++)
> > {
> > -			memcpy(key_buf + ((PMKSA_KEY_LEN * i) +
> > 1), hif_key->attr.pmkid.pmkidlist[i].bssid, ETH_ALEN);
> > -			memcpy(key_buf + ((PMKSA_KEY_LEN * i) +
> > ETH_ALEN + 1), hif_key->attr.pmkid.pmkidlist[i].pmkid, PMKID_LEN);
> > -		}
> > -
> > -		wid.id = (u16)WID_PMKID_INFO;
> > -		wid.type = WID_STR;
> > -		wid.val = (s8 *)key_buf;
> > -		wid.size = (hif_key->attr.pmkid.numpmkid *
> > PMKSA_KEY_LEN) + 1; -
> > -		result = wilc_send_config_pkt(vif, SET_CFG, &wid,
> > 1,
> > -
> > wilc_get_vif_idx(vif)); -
> > -		kfree(key_buf);
> > +		result = wilc_pmksa_key_copy(vif, hif_key);
> >  		break;
> >  	}
> >  
> >
Claudiu Beznea May 10, 2018, 5:21 a.m. UTC | #3 
On 09.05.2018 21:36, Ajay Singh wrote:
> On Wed, 9 May 2018 16:44:47 +0300
> Claudiu Beznea <Claudiu.Beznea@microchip.com> wrote:
> 
>> On 07.05.2018 11:43, Ajay Singh wrote:
>>> Fix checkpatch reported issue of line over 80 char in handle_key().
>>> Introduced new functions by spliting existing function to address
>>> the checkpatch issue.
>>>
>>> Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
>>> ---
>>>  drivers/staging/wilc1000/host_interface.c | 59
>>> +++++++++++++++++++------------ 1 file changed, 37 insertions(+),
>>> 22 deletions(-)
>>>
>>> diff --git a/drivers/staging/wilc1000/host_interface.c
>>> b/drivers/staging/wilc1000/host_interface.c index 4ba868c..29f9907
>>> 100644 --- a/drivers/staging/wilc1000/host_interface.c
>>> +++ b/drivers/staging/wilc1000/host_interface.c
>>> @@ -1498,12 +1498,45 @@ static s32
>>> handle_rcvd_gnrl_async_info(struct wilc_vif *vif, return result;
>>>  }
>>>  
>>> +static int wilc_pmksa_key_copy(struct wilc_vif *vif, struct
>>> key_attr *hif_key) +{
>>> +	int i;
>>> +	int ret;
>>> +	struct wid wid;
>>> +	u8 *key_buf;
>>> +
>>> +	key_buf = kmalloc((hif_key->attr.pmkid.numpmkid *
>>> PMKSA_KEY_LEN) + 1,
>>> +			  GFP_KERNEL);
>>> +	if (!key_buf)
>>> +		return -ENOMEM;
>>> +
>>> +	key_buf[0] = hif_key->attr.pmkid.numpmkid;
>>> +
>>> +	for (i = 0; i < hif_key->attr.pmkid.numpmkid; i++) {
>>> +		memcpy(key_buf + ((PMKSA_KEY_LEN * i) + 1),
>>> +		       hif_key->attr.pmkid.pmkidlist[i].bssid,
>>> ETH_ALEN);
>>> +		memcpy(key_buf + ((PMKSA_KEY_LEN * i) + ETH_ALEN +
>>> 1),
>>> +		       hif_key->attr.pmkid.pmkidlist[i].pmkid,
>>> PMKID_LEN);
>>> +	}
>>> +
>>> +	wid.id = (u16)WID_PMKID_INFO;
>>> +	wid.type = WID_STR;
>>> +	wid.val = (s8 *)key_buf;  
>>
>> Is this cast really needed?
>>
> 
> This patch is only to address line over 80 chars checkpatch issue.
> It is not good to club these change with type cast related
> modification. For removing unnecessary cast we can submit
> a separate patch series.
> These are my views. What do you think?
> 

I'm ok with this. I was thinking that since you introduced this new function
you may want to also address this, if any.

> 
>>> +	wid.size = (hif_key->attr.pmkid.numpmkid * PMKSA_KEY_LEN)
>>> + 1; +
>>> +	ret = wilc_send_config_pkt(vif, SET_CFG, &wid, 1,
>>> +				   wilc_get_vif_idx(vif));
>>> +
>>> +	kfree(key_buf);
>>> +
>>> +	return ret;
>>> +}
>>> +
>>>  static int handle_key(struct wilc_vif *vif, struct key_attr
>>> *hif_key) {
>>>  	int result = 0;
>>>  	struct wid wid;
>>>  	struct wid wid_list[5];
>>> -	u8 i;
>>>  	u8 *key_buf;
>>>  	s8 s8idxarray[1];
>>>  	struct host_if_drv *hif_drv = vif->hif_drv;
>>> @@ -1547,7 +1580,8 @@ static int handle_key(struct wilc_vif *vif,
>>> struct key_attr *hif_key) wilc_get_vif_idx(vif));
>>>  			kfree(key_buf);
>>>  		} else if (hif_key->action & ADDKEY) {
>>> -			key_buf =
>>> kmalloc(hif_key->attr.wep.key_len + 2, GFP_KERNEL);
>>> +			key_buf =
>>> kmalloc(hif_key->attr.wep.key_len + 2,
>>> +					  GFP_KERNEL);
>>>  			if (!key_buf) {
>>>  				result = -ENOMEM;
>>>  				goto out_wep;
>>> @@ -1715,26 +1749,7 @@ static int handle_key(struct wilc_vif *vif,
>>> struct key_attr *hif_key) break;
>>>  
>>>  	case PMKSA:
>>> -		key_buf = kmalloc((hif_key->attr.pmkid.numpmkid *
>>> PMKSA_KEY_LEN) + 1, GFP_KERNEL);
>>> -		if (!key_buf)
>>> -			return -ENOMEM;
>>> -
>>> -		key_buf[0] = hif_key->attr.pmkid.numpmkid;
>>> -
>>> -		for (i = 0; i < hif_key->attr.pmkid.numpmkid; i++)
>>> {
>>> -			memcpy(key_buf + ((PMKSA_KEY_LEN * i) +
>>> 1), hif_key->attr.pmkid.pmkidlist[i].bssid, ETH_ALEN);
>>> -			memcpy(key_buf + ((PMKSA_KEY_LEN * i) +
>>> ETH_ALEN + 1), hif_key->attr.pmkid.pmkidlist[i].pmkid, PMKID_LEN);
>>> -		}
>>> -
>>> -		wid.id = (u16)WID_PMKID_INFO;
>>> -		wid.type = WID_STR;
>>> -		wid.val = (s8 *)key_buf;
>>> -		wid.size = (hif_key->attr.pmkid.numpmkid *
>>> PMKSA_KEY_LEN) + 1; -
>>> -		result = wilc_send_config_pkt(vif, SET_CFG, &wid,
>>> 1,
>>> -
>>> wilc_get_vif_idx(vif)); -
>>> -		kfree(key_buf);
>>> +		result = wilc_pmksa_key_copy(vif, hif_key);
>>>  		break;
>>>  	}
>>>  
>>>   
> 
> 
>
Dan Carpenter May 15, 2018, 8:22 a.m. UTC | #4 
On Thu, May 10, 2018 at 08:21:52AM +0300, Claudiu Beznea wrote:
> >>> +	wid.val = (s8 *)key_buf;  
> >>
> >> Is this cast really needed?
> >>
> > 
> > This patch is only to address line over 80 chars checkpatch issue.
> > It is not good to club these change with type cast related
> > modification. For removing unnecessary cast we can submit
> > a separate patch series.
> > These are my views. What do you think?
> > 
> 
> I'm ok with this. I was thinking that since you introduced this new function
> you may want to also address this, if any.

No.  Please don't.  That kind of thing is sort of an unrelated change
from just moving the lines around and it messes up my review scripts.

It's really really easy to review the changes when they're split into
multiple chunks.

regards,
dan carpenter
diff mbox

Patch

diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index 4ba868c..29f9907 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -1498,12 +1498,45 @@  static s32 handle_rcvd_gnrl_async_info(struct wilc_vif *vif,
 	return result;
 }
 
+static int wilc_pmksa_key_copy(struct wilc_vif *vif, struct key_attr *hif_key)
+{
+	int i;
+	int ret;
+	struct wid wid;
+	u8 *key_buf;
+
+	key_buf = kmalloc((hif_key->attr.pmkid.numpmkid * PMKSA_KEY_LEN) + 1,
+			  GFP_KERNEL);
+	if (!key_buf)
+		return -ENOMEM;
+
+	key_buf[0] = hif_key->attr.pmkid.numpmkid;
+
+	for (i = 0; i < hif_key->attr.pmkid.numpmkid; i++) {
+		memcpy(key_buf + ((PMKSA_KEY_LEN * i) + 1),
+		       hif_key->attr.pmkid.pmkidlist[i].bssid, ETH_ALEN);
+		memcpy(key_buf + ((PMKSA_KEY_LEN * i) + ETH_ALEN + 1),
+		       hif_key->attr.pmkid.pmkidlist[i].pmkid, PMKID_LEN);
+	}
+
+	wid.id = (u16)WID_PMKID_INFO;
+	wid.type = WID_STR;
+	wid.val = (s8 *)key_buf;
+	wid.size = (hif_key->attr.pmkid.numpmkid * PMKSA_KEY_LEN) + 1;
+
+	ret = wilc_send_config_pkt(vif, SET_CFG, &wid, 1,
+				   wilc_get_vif_idx(vif));
+
+	kfree(key_buf);
+
+	return ret;
+}
+
 static int handle_key(struct wilc_vif *vif, struct key_attr *hif_key)
 {
 	int result = 0;
 	struct wid wid;
 	struct wid wid_list[5];
-	u8 i;
 	u8 *key_buf;
 	s8 s8idxarray[1];
 	struct host_if_drv *hif_drv = vif->hif_drv;
@@ -1547,7 +1580,8 @@  static int handle_key(struct wilc_vif *vif, struct key_attr *hif_key)
 						      wilc_get_vif_idx(vif));
 			kfree(key_buf);
 		} else if (hif_key->action & ADDKEY) {
-			key_buf = kmalloc(hif_key->attr.wep.key_len + 2, GFP_KERNEL);
+			key_buf = kmalloc(hif_key->attr.wep.key_len + 2,
+					  GFP_KERNEL);
 			if (!key_buf) {
 				result = -ENOMEM;
 				goto out_wep;
@@ -1715,26 +1749,7 @@  static int handle_key(struct wilc_vif *vif, struct key_attr *hif_key)
 		break;
 
 	case PMKSA:
-		key_buf = kmalloc((hif_key->attr.pmkid.numpmkid * PMKSA_KEY_LEN) + 1, GFP_KERNEL);
-		if (!key_buf)
-			return -ENOMEM;
-
-		key_buf[0] = hif_key->attr.pmkid.numpmkid;
-
-		for (i = 0; i < hif_key->attr.pmkid.numpmkid; i++) {
-			memcpy(key_buf + ((PMKSA_KEY_LEN * i) + 1), hif_key->attr.pmkid.pmkidlist[i].bssid, ETH_ALEN);
-			memcpy(key_buf + ((PMKSA_KEY_LEN * i) + ETH_ALEN + 1), hif_key->attr.pmkid.pmkidlist[i].pmkid, PMKID_LEN);
-		}
-
-		wid.id = (u16)WID_PMKID_INFO;
-		wid.type = WID_STR;
-		wid.val = (s8 *)key_buf;
-		wid.size = (hif_key->attr.pmkid.numpmkid * PMKSA_KEY_LEN) + 1;
-
-		result = wilc_send_config_pkt(vif, SET_CFG, &wid, 1,
-					      wilc_get_vif_idx(vif));
-
-		kfree(key_buf);
+		result = wilc_pmksa_key_copy(vif, hif_key);
 		break;
 	}