On Wed, 9 May 2018 16:44:38 +0300
Claudiu Beznea <Claudiu.Beznea@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
> > ---
> > 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); }
> > }
> >
> >
