| Message ID | 1525682614-3824-15-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 in > add_network_to_shadow() by using temporary variable. I, personally, don't like this way of fixing line over 80. From my point of view this introduces a new future patch. Maybe, in future, somebody will remove this temporary variable stating that there is no need for it. > > Signed-off-by: Ajay Singh <ajay.kathat@microchip.com> > --- > drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 52 +++++++++++------------ > 1 file changed, 25 insertions(+), 27 deletions(-) > > diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c > index f1ebaea..0ae2065 100644 > --- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c > +++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c > @@ -300,6 +300,7 @@ static void add_network_to_shadow(struct network_info *nw_info, > int ap_found = is_network_in_shadow(nw_info, user_void); > u32 ap_index = 0; > u8 rssi_index = 0; > + struct network_info *shadow_nw_info; > > if (last_scanned_cnt >= MAX_NUM_SCANNED_NETWORKS_SHADOW) > return; > @@ -310,37 +311,34 @@ static void add_network_to_shadow(struct network_info *nw_info, > } else { > ap_index = ap_found; > } > - rssi_index = last_scanned_shadow[ap_index].rssi_history.index; > - last_scanned_shadow[ap_index].rssi_history.samples[rssi_index++] = nw_info->rssi; > + shadow_nw_info = &last_scanned_shadow[ap_index]; > + rssi_index = shadow_nw_info->rssi_history.index; > + shadow_nw_info->rssi_history.samples[rssi_index++] = nw_info->rssi; > if (rssi_index == NUM_RSSI) { > rssi_index = 0; > - last_scanned_shadow[ap_index].rssi_history.full = true; > - } > - last_scanned_shadow[ap_index].rssi_history.index = rssi_index; > - last_scanned_shadow[ap_index].rssi = nw_info->rssi; > - last_scanned_shadow[ap_index].cap_info = nw_info->cap_info; > - last_scanned_shadow[ap_index].ssid_len = nw_info->ssid_len; > - memcpy(last_scanned_shadow[ap_index].ssid, > - nw_info->ssid, nw_info->ssid_len); > - memcpy(last_scanned_shadow[ap_index].bssid, > - nw_info->bssid, ETH_ALEN); > - last_scanned_shadow[ap_index].beacon_period = nw_info->beacon_period; > - last_scanned_shadow[ap_index].dtim_period = nw_info->dtim_period; > - last_scanned_shadow[ap_index].ch = nw_info->ch; > - last_scanned_shadow[ap_index].ies_len = nw_info->ies_len; > - last_scanned_shadow[ap_index].tsf_hi = nw_info->tsf_hi; > + shadow_nw_info->rssi_history.full = true; > + } > + shadow_nw_info->rssi_history.index = rssi_index; > + shadow_nw_info->rssi = nw_info->rssi; > + shadow_nw_info->cap_info = nw_info->cap_info; > + shadow_nw_info->ssid_len = nw_info->ssid_len; > + memcpy(shadow_nw_info->ssid, nw_info->ssid, nw_info->ssid_len); > + memcpy(shadow_nw_info->bssid, nw_info->bssid, ETH_ALEN); > + shadow_nw_info->beacon_period = nw_info->beacon_period; > + shadow_nw_info->dtim_period = nw_info->dtim_period; > + shadow_nw_info->ch = nw_info->ch; > + shadow_nw_info->ies_len = nw_info->ies_len; > + shadow_nw_info->tsf_hi = nw_info->tsf_hi; > if (ap_found != -1) > - kfree(last_scanned_shadow[ap_index].ies); > - last_scanned_shadow[ap_index].ies = kmalloc(nw_info->ies_len, > - GFP_KERNEL); > - memcpy(last_scanned_shadow[ap_index].ies, > - nw_info->ies, nw_info->ies_len); > - last_scanned_shadow[ap_index].time_scan = jiffies; > - last_scanned_shadow[ap_index].time_scan_cached = jiffies; > - last_scanned_shadow[ap_index].found = 1; > + kfree(shadow_nw_info->ies); > + shadow_nw_info->ies = kmalloc(nw_info->ies_len, GFP_KERNEL); > + memcpy(shadow_nw_info->ies, nw_info->ies, nw_info->ies_len); > + shadow_nw_info->time_scan = jiffies; > + shadow_nw_info->time_scan_cached = jiffies; > + shadow_nw_info->found = 1; > if (ap_found != -1) > - kfree(last_scanned_shadow[ap_index].join_params); > - last_scanned_shadow[ap_index].join_params = join_params; > + kfree(shadow_nw_info->join_params); > + shadow_nw_info->join_params = join_params; > } > > static void cfg_scan_result(enum scan_event scan_event, >
On Wed, 9 May 2018 16:43:14 +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 in > > add_network_to_shadow() by using temporary variable. > > I, personally, don't like this way of fixing line over 80. From my > point of view this introduces a new future patch. Maybe, in future, > somebody will remove this temporary variable stating that there is > no need for it. > In my opinion, just by removing this temporary variable the patch might not go through because it will definitely have line over 80 character issue. As per guideline its recommended to run the checkpatch before submitting the patch. Only using short variables names might help to resolve that issue but using short variable names will not give clear meaning for the code. I don't want to shorten the variable name as they don't convey the complete meaning. Do you have any suggestion/code which can help to understand how to resolve this without using temp/variables name changes. > > > > Signed-off-by: Ajay Singh <ajay.kathat@microchip.com> > > --- > > drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 52 > > +++++++++++------------ 1 file changed, 25 insertions(+), 27 > > deletions(-) > > > > diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c > > b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c index > > f1ebaea..0ae2065 100644 --- > > a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c +++ > > b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c @@ -300,6 > > +300,7 @@ static void add_network_to_shadow(struct network_info > > *nw_info, int ap_found = is_network_in_shadow(nw_info, user_void); > > u32 ap_index = 0; u8 rssi_index = 0; > > + struct network_info *shadow_nw_info; > > > > if (last_scanned_cnt >= MAX_NUM_SCANNED_NETWORKS_SHADOW) > > return; > > @@ -310,37 +311,34 @@ static void add_network_to_shadow(struct > > network_info *nw_info, } else { > > ap_index = ap_found; > > } > > - rssi_index = > > last_scanned_shadow[ap_index].rssi_history.index; > > - > > last_scanned_shadow[ap_index].rssi_history.samples[rssi_index++] = > > nw_info->rssi; > > + shadow_nw_info = &last_scanned_shadow[ap_index]; > > + rssi_index = shadow_nw_info->rssi_history.index; > > + shadow_nw_info->rssi_history.samples[rssi_index++] = > > nw_info->rssi; if (rssi_index == NUM_RSSI) { > > rssi_index = 0; > > - last_scanned_shadow[ap_index].rssi_history.full = > > true; > > - } > > - last_scanned_shadow[ap_index].rssi_history.index = > > rssi_index; > > - last_scanned_shadow[ap_index].rssi = nw_info->rssi; > > - last_scanned_shadow[ap_index].cap_info = nw_info->cap_info; > > - last_scanned_shadow[ap_index].ssid_len = nw_info->ssid_len; > > - memcpy(last_scanned_shadow[ap_index].ssid, > > - nw_info->ssid, nw_info->ssid_len); > > - memcpy(last_scanned_shadow[ap_index].bssid, > > - nw_info->bssid, ETH_ALEN); > > - last_scanned_shadow[ap_index].beacon_period = > > nw_info->beacon_period; > > - last_scanned_shadow[ap_index].dtim_period = > > nw_info->dtim_period; > > - last_scanned_shadow[ap_index].ch = nw_info->ch; > > - last_scanned_shadow[ap_index].ies_len = nw_info->ies_len; > > - last_scanned_shadow[ap_index].tsf_hi = nw_info->tsf_hi; > > + shadow_nw_info->rssi_history.full = true; > > + } > > + shadow_nw_info->rssi_history.index = rssi_index; > > + shadow_nw_info->rssi = nw_info->rssi; > > + shadow_nw_info->cap_info = nw_info->cap_info; > > + shadow_nw_info->ssid_len = nw_info->ssid_len; > > + memcpy(shadow_nw_info->ssid, nw_info->ssid, > > nw_info->ssid_len); > > + memcpy(shadow_nw_info->bssid, nw_info->bssid, ETH_ALEN); > > + shadow_nw_info->beacon_period = nw_info->beacon_period; > > + shadow_nw_info->dtim_period = nw_info->dtim_period; > > + shadow_nw_info->ch = nw_info->ch; > > + shadow_nw_info->ies_len = nw_info->ies_len; > > + shadow_nw_info->tsf_hi = nw_info->tsf_hi; > > if (ap_found != -1) > > - kfree(last_scanned_shadow[ap_index].ies); > > - last_scanned_shadow[ap_index].ies = > > kmalloc(nw_info->ies_len, > > - GFP_KERNEL); > > - memcpy(last_scanned_shadow[ap_index].ies, > > - nw_info->ies, nw_info->ies_len); > > - last_scanned_shadow[ap_index].time_scan = jiffies; > > - last_scanned_shadow[ap_index].time_scan_cached = jiffies; > > - last_scanned_shadow[ap_index].found = 1; > > + kfree(shadow_nw_info->ies); > > + shadow_nw_info->ies = kmalloc(nw_info->ies_len, > > GFP_KERNEL); > > + memcpy(shadow_nw_info->ies, nw_info->ies, > > nw_info->ies_len); > > + shadow_nw_info->time_scan = jiffies; > > + shadow_nw_info->time_scan_cached = jiffies; > > + shadow_nw_info->found = 1; > > if (ap_found != -1) > > - kfree(last_scanned_shadow[ap_index].join_params); > > - last_scanned_shadow[ap_index].join_params = join_params; > > + kfree(shadow_nw_info->join_params); > > + shadow_nw_info->join_params = join_params; > > } > > > > static void cfg_scan_result(enum scan_event scan_event, > >
On 09.05.2018 21:42, Ajay Singh wrote: > On Wed, 9 May 2018 16:43:14 +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 in >>> add_network_to_shadow() by using temporary variable. >> >> I, personally, don't like this way of fixing line over 80. From my >> point of view this introduces a new future patch. Maybe, in future, >> somebody will remove this temporary variable stating that there is >> no need for it. >> > > In my opinion, just by removing this temporary variable the patch > might not go through because it will definitely have line over > 80 character issue. As per guideline its recommended to run the > checkpatch before submitting the patch. > > Only using short variables names might help to resolve that issue but > using short variable names will not give clear meaning for the code. > I don't want to shorten the variable name as they don't convey the > complete meaning. > > Do you have any suggestion/code which can help to understand how to > resolve this without using temp/variables name changes. No, for this one I have not. Maybe further refactoring... > >>> >>> Signed-off-by: Ajay Singh <ajay.kathat@microchip.com> >>> --- >>> drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 52 >>> +++++++++++------------ 1 file changed, 25 insertions(+), 27 >>> deletions(-) >>> >>> diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c >>> b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c index >>> f1ebaea..0ae2065 100644 --- >>> a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c +++ >>> b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c @@ -300,6 >>> +300,7 @@ static void add_network_to_shadow(struct network_info >>> *nw_info, int ap_found = is_network_in_shadow(nw_info, user_void); >>> u32 ap_index = 0; u8 rssi_index = 0; >>> + struct network_info *shadow_nw_info; >>> >>> if (last_scanned_cnt >= MAX_NUM_SCANNED_NETWORKS_SHADOW) >>> return; >>> @@ -310,37 +311,34 @@ static void add_network_to_shadow(struct >>> network_info *nw_info, } else { >>> ap_index = ap_found; >>> } >>> - rssi_index = >>> last_scanned_shadow[ap_index].rssi_history.index; >>> - >>> last_scanned_shadow[ap_index].rssi_history.samples[rssi_index++] = >>> nw_info->rssi; >>> + shadow_nw_info = &last_scanned_shadow[ap_index]; >>> + rssi_index = shadow_nw_info->rssi_history.index; >>> + shadow_nw_info->rssi_history.samples[rssi_index++] = >>> nw_info->rssi; if (rssi_index == NUM_RSSI) { >>> rssi_index = 0; >>> - last_scanned_shadow[ap_index].rssi_history.full = >>> true; >>> - } >>> - last_scanned_shadow[ap_index].rssi_history.index = >>> rssi_index; >>> - last_scanned_shadow[ap_index].rssi = nw_info->rssi; >>> - last_scanned_shadow[ap_index].cap_info = nw_info->cap_info; >>> - last_scanned_shadow[ap_index].ssid_len = nw_info->ssid_len; >>> - memcpy(last_scanned_shadow[ap_index].ssid, >>> - nw_info->ssid, nw_info->ssid_len); >>> - memcpy(last_scanned_shadow[ap_index].bssid, >>> - nw_info->bssid, ETH_ALEN); >>> - last_scanned_shadow[ap_index].beacon_period = >>> nw_info->beacon_period; >>> - last_scanned_shadow[ap_index].dtim_period = >>> nw_info->dtim_period; >>> - last_scanned_shadow[ap_index].ch = nw_info->ch; >>> - last_scanned_shadow[ap_index].ies_len = nw_info->ies_len; >>> - last_scanned_shadow[ap_index].tsf_hi = nw_info->tsf_hi; >>> + shadow_nw_info->rssi_history.full = true; >>> + } >>> + shadow_nw_info->rssi_history.index = rssi_index; >>> + shadow_nw_info->rssi = nw_info->rssi; >>> + shadow_nw_info->cap_info = nw_info->cap_info; >>> + shadow_nw_info->ssid_len = nw_info->ssid_len; >>> + memcpy(shadow_nw_info->ssid, nw_info->ssid, >>> nw_info->ssid_len); >>> + memcpy(shadow_nw_info->bssid, nw_info->bssid, ETH_ALEN); >>> + shadow_nw_info->beacon_period = nw_info->beacon_period; >>> + shadow_nw_info->dtim_period = nw_info->dtim_period; >>> + shadow_nw_info->ch = nw_info->ch; >>> + shadow_nw_info->ies_len = nw_info->ies_len; >>> + shadow_nw_info->tsf_hi = nw_info->tsf_hi; >>> if (ap_found != -1) >>> - kfree(last_scanned_shadow[ap_index].ies); >>> - last_scanned_shadow[ap_index].ies = >>> kmalloc(nw_info->ies_len, >>> - GFP_KERNEL); >>> - memcpy(last_scanned_shadow[ap_index].ies, >>> - nw_info->ies, nw_info->ies_len); >>> - last_scanned_shadow[ap_index].time_scan = jiffies; >>> - last_scanned_shadow[ap_index].time_scan_cached = jiffies; >>> - last_scanned_shadow[ap_index].found = 1; >>> + kfree(shadow_nw_info->ies); >>> + shadow_nw_info->ies = kmalloc(nw_info->ies_len, >>> GFP_KERNEL); >>> + memcpy(shadow_nw_info->ies, nw_info->ies, >>> nw_info->ies_len); >>> + shadow_nw_info->time_scan = jiffies; >>> + shadow_nw_info->time_scan_cached = jiffies; >>> + shadow_nw_info->found = 1; >>> if (ap_found != -1) >>> - kfree(last_scanned_shadow[ap_index].join_params); >>> - last_scanned_shadow[ap_index].join_params = join_params; >>> + kfree(shadow_nw_info->join_params); >>> + shadow_nw_info->join_params = join_params; >>> } >>> >>> static void cfg_scan_result(enum scan_event scan_event, >>> > > >
Hi Ajay, On 10.05.2018 08:27, Claudiu Beznea wrote: > > > On 09.05.2018 21:42, Ajay Singh wrote: >> On Wed, 9 May 2018 16:43:14 +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 in >>>> add_network_to_shadow() by using temporary variable. >>> >>> I, personally, don't like this way of fixing line over 80. From my >>> point of view this introduces a new future patch. Maybe, in future, >>> somebody will remove this temporary variable stating that there is >>> no need for it. >>> >> >> In my opinion, just by removing this temporary variable the patch >> might not go through because it will definitely have line over >> 80 character issue. As per guideline its recommended to run the >> checkpatch before submitting the patch. >> >> Only using short variables names might help to resolve that issue but >> using short variable names will not give clear meaning for the code. >> I don't want to shorten the variable name as they don't convey the >> complete meaning. >> >> Do you have any suggestion/code which can help to understand how to >> resolve this without using temp/variables name changes. > > No, for this one I have not. Maybe further refactoring... > Looking over the v2 of this series you send, and over wilc_wfi_cfgoperations.c, and remembering your last question on this patch, I was thinking that one suggestion for this would be to replace last_scanned_shadow with just scanned_shadow or nw_info or scanned_nw_info. Just an idea. Claudiu >> >>>> >>>> Signed-off-by: Ajay Singh <ajay.kathat@microchip.com> >>>> --- >>>> drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 52 >>>> +++++++++++------------ 1 file changed, 25 insertions(+), 27 >>>> deletions(-) >>>> >>>> diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c >>>> b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c index >>>> f1ebaea..0ae2065 100644 --- >>>> a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c +++ >>>> b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c @@ -300,6 >>>> +300,7 @@ static void add_network_to_shadow(struct network_info >>>> *nw_info, int ap_found = is_network_in_shadow(nw_info, user_void); >>>> u32 ap_index = 0; u8 rssi_index = 0; >>>> + struct network_info *shadow_nw_info; >>>> >>>> if (last_scanned_cnt >= MAX_NUM_SCANNED_NETWORKS_SHADOW) >>>> return; >>>> @@ -310,37 +311,34 @@ static void add_network_to_shadow(struct >>>> network_info *nw_info, } else { >>>> ap_index = ap_found; >>>> } >>>> - rssi_index = >>>> last_scanned_shadow[ap_index].rssi_history.index; >>>> - >>>> last_scanned_shadow[ap_index].rssi_history.samples[rssi_index++] = >>>> nw_info->rssi; >>>> + shadow_nw_info = &last_scanned_shadow[ap_index]; >>>> + rssi_index = shadow_nw_info->rssi_history.index; >>>> + shadow_nw_info->rssi_history.samples[rssi_index++] = >>>> nw_info->rssi; if (rssi_index == NUM_RSSI) { >>>> rssi_index = 0; >>>> - last_scanned_shadow[ap_index].rssi_history.full = >>>> true; >>>> - } >>>> - last_scanned_shadow[ap_index].rssi_history.index = >>>> rssi_index; >>>> - last_scanned_shadow[ap_index].rssi = nw_info->rssi; >>>> - last_scanned_shadow[ap_index].cap_info = nw_info->cap_info; >>>> - last_scanned_shadow[ap_index].ssid_len = nw_info->ssid_len; >>>> - memcpy(last_scanned_shadow[ap_index].ssid, >>>> - nw_info->ssid, nw_info->ssid_len); >>>> - memcpy(last_scanned_shadow[ap_index].bssid, >>>> - nw_info->bssid, ETH_ALEN); >>>> - last_scanned_shadow[ap_index].beacon_period = >>>> nw_info->beacon_period; >>>> - last_scanned_shadow[ap_index].dtim_period = >>>> nw_info->dtim_period; >>>> - last_scanned_shadow[ap_index].ch = nw_info->ch; >>>> - last_scanned_shadow[ap_index].ies_len = nw_info->ies_len; >>>> - last_scanned_shadow[ap_index].tsf_hi = nw_info->tsf_hi; >>>> + shadow_nw_info->rssi_history.full = true; >>>> + } >>>> + shadow_nw_info->rssi_history.index = rssi_index; >>>> + shadow_nw_info->rssi = nw_info->rssi; >>>> + shadow_nw_info->cap_info = nw_info->cap_info; >>>> + shadow_nw_info->ssid_len = nw_info->ssid_len; >>>> + memcpy(shadow_nw_info->ssid, nw_info->ssid, >>>> nw_info->ssid_len); >>>> + memcpy(shadow_nw_info->bssid, nw_info->bssid, ETH_ALEN); >>>> + shadow_nw_info->beacon_period = nw_info->beacon_period; >>>> + shadow_nw_info->dtim_period = nw_info->dtim_period; >>>> + shadow_nw_info->ch = nw_info->ch; >>>> + shadow_nw_info->ies_len = nw_info->ies_len; >>>> + shadow_nw_info->tsf_hi = nw_info->tsf_hi; >>>> if (ap_found != -1) >>>> - kfree(last_scanned_shadow[ap_index].ies); >>>> - last_scanned_shadow[ap_index].ies = >>>> kmalloc(nw_info->ies_len, >>>> - GFP_KERNEL); >>>> - memcpy(last_scanned_shadow[ap_index].ies, >>>> - nw_info->ies, nw_info->ies_len); >>>> - last_scanned_shadow[ap_index].time_scan = jiffies; >>>> - last_scanned_shadow[ap_index].time_scan_cached = jiffies; >>>> - last_scanned_shadow[ap_index].found = 1; >>>> + kfree(shadow_nw_info->ies); >>>> + shadow_nw_info->ies = kmalloc(nw_info->ies_len, >>>> GFP_KERNEL); >>>> + memcpy(shadow_nw_info->ies, nw_info->ies, >>>> nw_info->ies_len); >>>> + shadow_nw_info->time_scan = jiffies; >>>> + shadow_nw_info->time_scan_cached = jiffies; >>>> + shadow_nw_info->found = 1; >>>> if (ap_found != -1) >>>> - kfree(last_scanned_shadow[ap_index].join_params); >>>> - last_scanned_shadow[ap_index].join_params = join_params; >>>> + kfree(shadow_nw_info->join_params); >>>> + shadow_nw_info->join_params = join_params; >>>> } >>>> >>>> static void cfg_scan_result(enum scan_event scan_event, >>>> >> >> >> >
Hi Claudiu, On Mon, 14 May 2018 11:57:24 +0300 Claudiu Beznea <Claudiu.Beznea@microchip.com> wrote: > Hi Ajay, > > On 10.05.2018 08:27, Claudiu Beznea wrote: > > > > > > On 09.05.2018 21:42, Ajay Singh wrote: > >> On Wed, 9 May 2018 16:43:14 +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 in > >>>> add_network_to_shadow() by using temporary variable. > >>> > >>> I, personally, don't like this way of fixing line over 80. From my > >>> point of view this introduces a new future patch. Maybe, in > >>> future, somebody will remove this temporary variable stating that > >>> there is no need for it. > >>> > >> > >> In my opinion, just by removing this temporary variable the patch > >> might not go through because it will definitely have line over > >> 80 character issue. As per guideline its recommended to run the > >> checkpatch before submitting the patch. > >> > >> Only using short variables names might help to resolve that issue > >> but using short variable names will not give clear meaning for the > >> code. I don't want to shorten the variable name as they don't > >> convey the complete meaning. > >> > >> Do you have any suggestion/code which can help to understand how to > >> resolve this without using temp/variables name changes. > > > > No, for this one I have not. Maybe further refactoring... > > > > Looking over the v2 of this series you send, and over > wilc_wfi_cfgoperations.c, and remembering your last question on this > patch, I was thinking that one suggestion for this would be to > replace last_scanned_shadow with just scanned_shadow or nw_info or > scanned_nw_info. Just an idea. > I avoided use of short name for 'last_scanned_shadow' because it might not give clear meaning as there are variables like 'last_scanned_cnt', which also uses same prefix 'last_' and its helpful to know its related data. > >> > >>>> > >>>> Signed-off-by: Ajay Singh <ajay.kathat@microchip.com> > >>>> --- > >>>> drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 52 > >>>> +++++++++++------------ 1 file changed, 25 insertions(+), 27 > >>>> deletions(-) > >>>> > >>>> diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c > >>>> b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c index > >>>> f1ebaea..0ae2065 100644 --- > >>>> a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c +++ > >>>> b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c @@ -300,6 > >>>> +300,7 @@ static void add_network_to_shadow(struct network_info > >>>> *nw_info, int ap_found = is_network_in_shadow(nw_info, > >>>> user_void); u32 ap_index = 0; u8 rssi_index = 0; > >>>> + struct network_info *shadow_nw_info; > >>>> > >>>> if (last_scanned_cnt >= MAX_NUM_SCANNED_NETWORKS_SHADOW) > >>>> return; > >>>> @@ -310,37 +311,34 @@ static void add_network_to_shadow(struct > >>>> network_info *nw_info, } else { > >>>> ap_index = ap_found; > >>>> } > >>>> - rssi_index = > >>>> last_scanned_shadow[ap_index].rssi_history.index; > >>>> - > >>>> last_scanned_shadow[ap_index].rssi_history.samples[rssi_index++] > >>>> = nw_info->rssi; > >>>> + shadow_nw_info = &last_scanned_shadow[ap_index]; > >>>> + rssi_index = shadow_nw_info->rssi_history.index; > >>>> + shadow_nw_info->rssi_history.samples[rssi_index++] = > >>>> nw_info->rssi; if (rssi_index == NUM_RSSI) { > >>>> rssi_index = 0; > >>>> - last_scanned_shadow[ap_index].rssi_history.full > >>>> = true; > >>>> - } > >>>> - last_scanned_shadow[ap_index].rssi_history.index = > >>>> rssi_index; > >>>> - last_scanned_shadow[ap_index].rssi = nw_info->rssi; > >>>> - last_scanned_shadow[ap_index].cap_info = > >>>> nw_info->cap_info; > >>>> - last_scanned_shadow[ap_index].ssid_len = > >>>> nw_info->ssid_len; > >>>> - memcpy(last_scanned_shadow[ap_index].ssid, > >>>> - nw_info->ssid, nw_info->ssid_len); > >>>> - memcpy(last_scanned_shadow[ap_index].bssid, > >>>> - nw_info->bssid, ETH_ALEN); > >>>> - last_scanned_shadow[ap_index].beacon_period = > >>>> nw_info->beacon_period; > >>>> - last_scanned_shadow[ap_index].dtim_period = > >>>> nw_info->dtim_period; > >>>> - last_scanned_shadow[ap_index].ch = nw_info->ch; > >>>> - last_scanned_shadow[ap_index].ies_len = > >>>> nw_info->ies_len; > >>>> - last_scanned_shadow[ap_index].tsf_hi = nw_info->tsf_hi; > >>>> + shadow_nw_info->rssi_history.full = true; > >>>> + } > >>>> + shadow_nw_info->rssi_history.index = rssi_index; > >>>> + shadow_nw_info->rssi = nw_info->rssi; > >>>> + shadow_nw_info->cap_info = nw_info->cap_info; > >>>> + shadow_nw_info->ssid_len = nw_info->ssid_len; > >>>> + memcpy(shadow_nw_info->ssid, nw_info->ssid, > >>>> nw_info->ssid_len); > >>>> + memcpy(shadow_nw_info->bssid, nw_info->bssid, ETH_ALEN); > >>>> + shadow_nw_info->beacon_period = nw_info->beacon_period; > >>>> + shadow_nw_info->dtim_period = nw_info->dtim_period; > >>>> + shadow_nw_info->ch = nw_info->ch; > >>>> + shadow_nw_info->ies_len = nw_info->ies_len; > >>>> + shadow_nw_info->tsf_hi = nw_info->tsf_hi; > >>>> if (ap_found != -1) > >>>> - kfree(last_scanned_shadow[ap_index].ies); > >>>> - last_scanned_shadow[ap_index].ies = > >>>> kmalloc(nw_info->ies_len, > >>>> - GFP_KERNEL); > >>>> - memcpy(last_scanned_shadow[ap_index].ies, > >>>> - nw_info->ies, nw_info->ies_len); > >>>> - last_scanned_shadow[ap_index].time_scan = jiffies; > >>>> - last_scanned_shadow[ap_index].time_scan_cached = > >>>> jiffies; > >>>> - last_scanned_shadow[ap_index].found = 1; > >>>> + kfree(shadow_nw_info->ies); > >>>> + shadow_nw_info->ies = kmalloc(nw_info->ies_len, > >>>> GFP_KERNEL); > >>>> + memcpy(shadow_nw_info->ies, nw_info->ies, > >>>> nw_info->ies_len); > >>>> + shadow_nw_info->time_scan = jiffies; > >>>> + shadow_nw_info->time_scan_cached = jiffies; > >>>> + shadow_nw_info->found = 1; > >>>> if (ap_found != -1) > >>>> - > >>>> kfree(last_scanned_shadow[ap_index].join_params); > >>>> - last_scanned_shadow[ap_index].join_params = join_params; > >>>> + kfree(shadow_nw_info->join_params); > >>>> + shadow_nw_info->join_params = join_params; > >>>> } > >>>> > >>>> static void cfg_scan_result(enum scan_event scan_event, > >>>> > >> > >> > >> > >
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c index f1ebaea..0ae2065 100644 --- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c +++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c @@ -300,6 +300,7 @@ static void add_network_to_shadow(struct network_info *nw_info, int ap_found = is_network_in_shadow(nw_info, user_void); u32 ap_index = 0; u8 rssi_index = 0; + struct network_info *shadow_nw_info; if (last_scanned_cnt >= MAX_NUM_SCANNED_NETWORKS_SHADOW) return; @@ -310,37 +311,34 @@ static void add_network_to_shadow(struct network_info *nw_info, } else { ap_index = ap_found; } - rssi_index = last_scanned_shadow[ap_index].rssi_history.index; - last_scanned_shadow[ap_index].rssi_history.samples[rssi_index++] = nw_info->rssi; + shadow_nw_info = &last_scanned_shadow[ap_index]; + rssi_index = shadow_nw_info->rssi_history.index; + shadow_nw_info->rssi_history.samples[rssi_index++] = nw_info->rssi; if (rssi_index == NUM_RSSI) { rssi_index = 0; - last_scanned_shadow[ap_index].rssi_history.full = true; - } - last_scanned_shadow[ap_index].rssi_history.index = rssi_index; - last_scanned_shadow[ap_index].rssi = nw_info->rssi; - last_scanned_shadow[ap_index].cap_info = nw_info->cap_info; - last_scanned_shadow[ap_index].ssid_len = nw_info->ssid_len; - memcpy(last_scanned_shadow[ap_index].ssid, - nw_info->ssid, nw_info->ssid_len); - memcpy(last_scanned_shadow[ap_index].bssid, - nw_info->bssid, ETH_ALEN); - last_scanned_shadow[ap_index].beacon_period = nw_info->beacon_period; - last_scanned_shadow[ap_index].dtim_period = nw_info->dtim_period; - last_scanned_shadow[ap_index].ch = nw_info->ch; - last_scanned_shadow[ap_index].ies_len = nw_info->ies_len; - last_scanned_shadow[ap_index].tsf_hi = nw_info->tsf_hi; + shadow_nw_info->rssi_history.full = true; + } + shadow_nw_info->rssi_history.index = rssi_index; + shadow_nw_info->rssi = nw_info->rssi; + shadow_nw_info->cap_info = nw_info->cap_info; + shadow_nw_info->ssid_len = nw_info->ssid_len; + memcpy(shadow_nw_info->ssid, nw_info->ssid, nw_info->ssid_len); + memcpy(shadow_nw_info->bssid, nw_info->bssid, ETH_ALEN); + shadow_nw_info->beacon_period = nw_info->beacon_period; + shadow_nw_info->dtim_period = nw_info->dtim_period; + shadow_nw_info->ch = nw_info->ch; + shadow_nw_info->ies_len = nw_info->ies_len; + shadow_nw_info->tsf_hi = nw_info->tsf_hi; if (ap_found != -1) - kfree(last_scanned_shadow[ap_index].ies); - last_scanned_shadow[ap_index].ies = kmalloc(nw_info->ies_len, - GFP_KERNEL); - memcpy(last_scanned_shadow[ap_index].ies, - nw_info->ies, nw_info->ies_len); - last_scanned_shadow[ap_index].time_scan = jiffies; - last_scanned_shadow[ap_index].time_scan_cached = jiffies; - last_scanned_shadow[ap_index].found = 1; + kfree(shadow_nw_info->ies); + shadow_nw_info->ies = kmalloc(nw_info->ies_len, GFP_KERNEL); + memcpy(shadow_nw_info->ies, nw_info->ies, nw_info->ies_len); + shadow_nw_info->time_scan = jiffies; + shadow_nw_info->time_scan_cached = jiffies; + shadow_nw_info->found = 1; if (ap_found != -1) - kfree(last_scanned_shadow[ap_index].join_params); - last_scanned_shadow[ap_index].join_params = join_params; + kfree(shadow_nw_info->join_params); + shadow_nw_info->join_params = join_params; } static void cfg_scan_result(enum scan_event scan_event,
Fix line over 80 characters issue reported by checkpatch in add_network_to_shadow() by using temporary variable. Signed-off-by: Ajay Singh <ajay.kathat@microchip.com> --- drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 52 +++++++++++------------ 1 file changed, 25 insertions(+), 27 deletions(-)