| Message ID | 1525682614-3824-6-git-send-email-ajay.kathat@microchip.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Kalle Valo |
| Headers | show |
On 07.05.2018 11:43, Ajay Singh wrote: > Fix line over 80 characters issue reported by checkpatch.pl in > host_int_parse_assoc_resp_info(). > > Signed-off-by: Ajay Singh <ajay.kathat@microchip.com> > --- > drivers/staging/wilc1000/host_interface.c | 37 ++++++++++++++++++------------- > 1 file changed, 21 insertions(+), 16 deletions(-) > > diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c > index 55a61d5..a341ff1 100644 > --- a/drivers/staging/wilc1000/host_interface.c > +++ b/drivers/staging/wilc1000/host_interface.c > @@ -1293,6 +1293,23 @@ static inline void host_int_free_user_conn_req(struct host_if_drv *hif_drv) > hif_drv->usr_conn_req.ies = NULL; > } > > +static void host_int_copy_conn_info(struct connect_resp_info *conn_resp_info, > + struct connect_info *conn_info) > +{ > + conn_info->status = conn_resp_info->status; > + > + if (conn_info->status == SUCCESSFUL_STATUSCODE && conn_resp_info->ies) { > + conn_info->resp_ies = kmemdup(conn_resp_info->ies, > + conn_resp_info->ies_len, > + GFP_KERNEL); > + if (conn_info->resp_ies) > + conn_info->resp_ies_len = conn_resp_info->ies_len; > + } > + > + kfree(conn_resp_info->ies); > + kfree(conn_resp_info); > +} > + Instead of adding new function why not using wilc_parse_assoc_resp_info() to return you the conn_info that you are copying it here? The connect_resp_info object that you are passing to wilc_parse_assoc_resp_info() is not used anywhere in host_int_parse_assoc_resp_info() it is used only to copy again from it to conn_info object. Also, in wilc_parse_assoc_resp_info() you can get rid of these lines: connect_resp_info->capability = get_assoc_resp_cap_info(buffer); connect_resp_info->assoc_id = get_asoc_id(buffer); > static inline void host_int_parse_assoc_resp_info(struct wilc_vif *vif, > u8 mac_status) > { > @@ -1316,25 +1333,13 @@ static inline void host_int_parse_assoc_resp_info(struct wilc_vif *vif, > > err = wilc_parse_assoc_resp_info(rcv_assoc_resp, rcvd_assoc_resp_info_len, > &connect_resp_info); > - if (err) { > + if (err) > netdev_err(vif->ndev, > "wilc_parse_assoc_resp_info() returned error %d\n", > err); > - } else { > - conn_info.status = connect_resp_info->status; > - > - if (conn_info.status == SUCCESSFUL_STATUSCODE && > - connect_resp_info->ies) { > - conn_info.resp_ies = kmemdup(connect_resp_info->ies, > - connect_resp_info->ies_len, > - GFP_KERNEL); > - if (conn_info.resp_ies) > - conn_info.resp_ies_len = connect_resp_info->ies_len; > - } > - > - kfree(connect_resp_info->ies); > - kfree(connect_resp_info); > - } > + else > + host_int_copy_conn_info(connect_resp_info, > + &conn_info); > } > } > >
On Wed, 9 May 2018 16:44:38 +0300 Claudiu Beznea <Claudiu.Beznea@microchip.com> wrote: > On 07.05.2018 11:43, Ajay Singh wrote: > > Fix line over 80 characters issue reported by checkpatch.pl in > > host_int_parse_assoc_resp_info(). > > > > Signed-off-by: Ajay Singh <ajay.kathat@microchip.com> > > --- > > drivers/staging/wilc1000/host_interface.c | 37 > > ++++++++++++++++++------------- 1 file changed, 21 insertions(+), > > 16 deletions(-) > > > > diff --git a/drivers/staging/wilc1000/host_interface.c > > b/drivers/staging/wilc1000/host_interface.c index 55a61d5..a341ff1 > > 100644 --- a/drivers/staging/wilc1000/host_interface.c > > +++ b/drivers/staging/wilc1000/host_interface.c > > @@ -1293,6 +1293,23 @@ static inline void > > host_int_free_user_conn_req(struct host_if_drv *hif_drv) > > hif_drv->usr_conn_req.ies = NULL; } > > > > +static void host_int_copy_conn_info(struct connect_resp_info > > *conn_resp_info, > > + struct connect_info *conn_info) > > +{ > > + conn_info->status = conn_resp_info->status; > > + > > + if (conn_info->status == SUCCESSFUL_STATUSCODE && > > conn_resp_info->ies) { > > + conn_info->resp_ies = kmemdup(conn_resp_info->ies, > > + > > conn_resp_info->ies_len, > > + GFP_KERNEL); > > + if (conn_info->resp_ies) > > + conn_info->resp_ies_len = > > conn_resp_info->ies_len; > > + } > > + > > + kfree(conn_resp_info->ies); > > + kfree(conn_resp_info); > > +} > > + > > Instead of adding new function why not using > wilc_parse_assoc_resp_info() to return you the conn_info that you are > copying it here? The connect_resp_info object that you are passing to > wilc_parse_assoc_resp_info() is not used anywhere in > host_int_parse_assoc_resp_info() it is used only to copy again from > it to conn_info object. Yes, there are different function because the purpose is different. Primarily wilc_parse_assoc_resp_info() is to take care of parsing and filing the response info and later connect_resp_info is used to fill conn_info values. The elements in conn_info and connect_resp_info are different, but now most of the fields are not used in connect_resp_info. Returning 'conn_info' from wilc_parse_assoc_resp_info() might not be correct. I will check if we can go away with connect_resp_info structure itself as most of the field are not used in it. > > Also, in wilc_parse_assoc_resp_info() you can get rid of these lines: > connect_resp_info->capability = get_assoc_resp_cap_info(buffer); > connect_resp_info->assoc_id = get_asoc_id(buffer); > With above changes it can be taken care. > > > static inline void host_int_parse_assoc_resp_info(struct wilc_vif > > *vif, u8 mac_status) > > { > > @@ -1316,25 +1333,13 @@ static inline void > > host_int_parse_assoc_resp_info(struct wilc_vif *vif, > > err = > > wilc_parse_assoc_resp_info(rcv_assoc_resp, > > rcvd_assoc_resp_info_len, &connect_resp_info); > > - if (err) { > > + if (err) > > netdev_err(vif->ndev, > > "wilc_parse_assoc_resp_info() > > returned error %d\n", err); > > - } else { > > - conn_info.status = > > connect_resp_info->status; - > > - if (conn_info.status == > > SUCCESSFUL_STATUSCODE && > > - connect_resp_info->ies) { > > - conn_info.resp_ies = > > kmemdup(connect_resp_info->ies, > > - > > connect_resp_info->ies_len, > > - > > GFP_KERNEL); > > - if (conn_info.resp_ies) > > - > > conn_info.resp_ies_len = connect_resp_info->ies_len; > > - } > > - > > - kfree(connect_resp_info->ies); > > - kfree(connect_resp_info); > > - } > > + else > > + > > host_int_copy_conn_info(connect_resp_info, > > + > > &conn_info); } > > } > > > >
diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c index 55a61d5..a341ff1 100644 --- a/drivers/staging/wilc1000/host_interface.c +++ b/drivers/staging/wilc1000/host_interface.c @@ -1293,6 +1293,23 @@ static inline void host_int_free_user_conn_req(struct host_if_drv *hif_drv) hif_drv->usr_conn_req.ies = NULL; } +static void host_int_copy_conn_info(struct connect_resp_info *conn_resp_info, + struct connect_info *conn_info) +{ + conn_info->status = conn_resp_info->status; + + if (conn_info->status == SUCCESSFUL_STATUSCODE && conn_resp_info->ies) { + conn_info->resp_ies = kmemdup(conn_resp_info->ies, + conn_resp_info->ies_len, + GFP_KERNEL); + if (conn_info->resp_ies) + conn_info->resp_ies_len = conn_resp_info->ies_len; + } + + kfree(conn_resp_info->ies); + kfree(conn_resp_info); +} + static inline void host_int_parse_assoc_resp_info(struct wilc_vif *vif, u8 mac_status) { @@ -1316,25 +1333,13 @@ static inline void host_int_parse_assoc_resp_info(struct wilc_vif *vif, err = wilc_parse_assoc_resp_info(rcv_assoc_resp, rcvd_assoc_resp_info_len, &connect_resp_info); - if (err) { + if (err) netdev_err(vif->ndev, "wilc_parse_assoc_resp_info() returned error %d\n", err); - } else { - conn_info.status = connect_resp_info->status; - - if (conn_info.status == SUCCESSFUL_STATUSCODE && - connect_resp_info->ies) { - conn_info.resp_ies = kmemdup(connect_resp_info->ies, - connect_resp_info->ies_len, - GFP_KERNEL); - if (conn_info.resp_ies) - conn_info.resp_ies_len = connect_resp_info->ies_len; - } - - kfree(connect_resp_info->ies); - kfree(connect_resp_info); - } + else + host_int_copy_conn_info(connect_resp_info, + &conn_info); } }
Fix line over 80 characters issue reported by checkpatch.pl in host_int_parse_assoc_resp_info(). Signed-off-by: Ajay Singh <ajay.kathat@microchip.com> --- drivers/staging/wilc1000/host_interface.c | 37 ++++++++++++++++++------------- 1 file changed, 21 insertions(+), 16 deletions(-)