* Re: [PATCH 3/3] staging: wilc1000: Reduce scope for a few variables in mac_ioctl()
@ 2016-07-25 19:17 Lino Sanfilippo
2016-07-25 19:32 ` walter harms
2016-07-26 6:25 ` SF Markus Elfring
0 siblings, 2 replies; 5+ messages in thread
From: Lino Sanfilippo @ 2016-07-25 19:17 UTC (permalink / raw)
To: SF Markus Elfring; +Cc: LKML, kernel-janitors, linux-wireless
>
> - if (strncasecmp(buff, "RSSI", length) == 0) {
> + if (strncasecmp(buff, "RSSI", 0) == 0) {
> + s8 rssi;
> +
Um, please think a second about if it makes any sense at all to compare
zero chars of two strings.
Lino
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 3/3] staging: wilc1000: Reduce scope for a few variables in mac_ioctl()
2016-07-25 19:17 [PATCH 3/3] staging: wilc1000: Reduce scope for a few variables in mac_ioctl() Lino Sanfilippo
@ 2016-07-25 19:32 ` walter harms
2016-07-26 6:25 ` SF Markus Elfring
1 sibling, 0 replies; 5+ messages in thread
From: walter harms @ 2016-07-25 19:32 UTC (permalink / raw)
To: Lino Sanfilippo; +Cc: SF Markus Elfring, LKML, kernel-janitors, linux-wireless
Am 25.07.2016 21:17, schrieb Lino Sanfilippo:
>>
>> - if (strncasecmp(buff, "RSSI", length) == 0) {
>> + if (strncasecmp(buff, "RSSI", 0) == 0) {
>> + s8 rssi;
>> +
>
> Um, please think a second about if it makes any sense at all to compare
> zero chars of two strings.
>
> Lino
also:
the switch has only one case and default.
re,
wh
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: staging: wilc1000: Reduce scope for a few variables in mac_ioctl()
2016-07-25 19:17 [PATCH 3/3] staging: wilc1000: Reduce scope for a few variables in mac_ioctl() Lino Sanfilippo
2016-07-25 19:32 ` walter harms
@ 2016-07-26 6:25 ` SF Markus Elfring
2016-07-28 11:45 ` Aw: " Lino Sanfilippo
1 sibling, 1 reply; 5+ messages in thread
From: SF Markus Elfring @ 2016-07-26 6:25 UTC (permalink / raw)
To: Lino Sanfilippo
Cc: LKML, kernel-janitors, linux-wireless, devel, Greg Kroah-Hartman,
Steve Caldwell
>> - if (strncasecmp(buff, "RSSI", length) == 0) {
>> + if (strncasecmp(buff, "RSSI", 0) == 0) {
>> + s8 rssi;
>> +
>
> Um, please think a second about if it makes any sense at all to compare
> zero chars of two strings.
Under which circumstances should the variable "length" contain an other
value than zero?
How can this open issue be fixed better?
Regards,
Markus
^ permalink raw reply [flat|nested] 5+ messages in thread
* Aw: Re: staging: wilc1000: Reduce scope for a few variables in mac_ioctl()
2016-07-26 6:25 ` SF Markus Elfring
@ 2016-07-28 11:45 ` Lino Sanfilippo
2016-07-28 16:00 ` SF Markus Elfring
0 siblings, 1 reply; 5+ messages in thread
From: Lino Sanfilippo @ 2016-07-28 11:45 UTC (permalink / raw)
To: SF Markus Elfring
Cc: LKML, kernel-janitors, linux-wireless, devel, Greg Kroah-Hartman,
Steve Caldwell
> Gesendet: Dienstag, 26. Juli 2016 um 08:25 Uhr
> Von: "SF Markus Elfring" <elfring@users.sourceforge.net>
>
> >> - if (strncasecmp(buff, "RSSI", length) == 0) {
> >> + if (strncasecmp(buff, "RSSI", 0) == 0) {
> >> + s8 rssi;
> >> +
> >
> > Um, please think a second about if it makes any sense at all to compare
> > zero chars of two strings.
>
> Under which circumstances should the variable "length" contain an other
> value than zero?
Which circumstances do "not any sense at all" imply?
>
> How can this open issue be fixed better?
The code is not too complicated and I think it is very obvious which value/variable
should be passed instead of 0. I suggest to fix this since it is indeed a bug, instead of
doing "micro optimizations" - which is the last thing that code in the staging area
needs (as IIRC you have already been told by others, including the staging maintainer).
Regards,
Lino
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: staging: wilc1000: Reduce scope for a few variables in mac_ioctl()
2016-07-28 11:45 ` Aw: " Lino Sanfilippo
@ 2016-07-28 16:00 ` SF Markus Elfring
0 siblings, 0 replies; 5+ messages in thread
From: SF Markus Elfring @ 2016-07-28 16:00 UTC (permalink / raw)
To: Lino Sanfilippo
Cc: LKML, kernel-janitors, linux-wireless, devel, Greg Kroah-Hartman,
Steve Caldwell
> Which circumstances do "not any sense at all" imply?
Should the expression 'strlen("RSSI")' be passed for the parameter 'length' instead?
> I suggest to fix this since it is indeed a bug,
We can agree that this function implementation was broken for a while there.
> instead of doing "micro optimizations" - which is the last thing that code in the staging area
> needs (as IIRC you have already been told by others, including the staging maintainer).
The acceptance might grow a bit more for such software fine-tuning
(like refactoring around variable usage).
Regards,
Markus
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-07-28 16:01 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-25 19:17 [PATCH 3/3] staging: wilc1000: Reduce scope for a few variables in mac_ioctl() Lino Sanfilippo
2016-07-25 19:32 ` walter harms
2016-07-26 6:25 ` SF Markus Elfring
2016-07-28 11:45 ` Aw: " Lino Sanfilippo
2016-07-28 16:00 ` SF Markus Elfring
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).