Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1449763 > unrolled thread
| Started by | Lino Sanfilippo <LinoSanfilippo@gmx.de> |
|---|---|
| First post | 2016-07-25 21:20 +0200 |
| Last post | 2016-07-28 18:10 +0200 |
| Articles | 5 — 4 participants |
Back to article view | Back to linux.kernel
Re: [PATCH 3/3] staging: wilc1000: Reduce scope for a few variables in mac_ioctl() Lino Sanfilippo <LinoSanfilippo@gmx.de> - 2016-07-25 21:20 +0200
Re: [PATCH 3/3] staging: wilc1000: Reduce scope for a few variables in mac_ioctl() walter harms <wharms@bfs.de> - 2016-07-25 21:40 +0200
Re: staging: wilc1000: Reduce scope for a few variables in mac_ioctl() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-26 08:30 +0200
Aw: Re: staging: wilc1000: Reduce scope for a few variables in mac_ioctl() "Lino Sanfilippo" <LinoSanfilippo@gmx.de> - 2016-07-28 13:50 +0200
Re: staging: wilc1000: Reduce scope for a few variables in mac_ioctl() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-28 18:10 +0200
| From | Lino Sanfilippo <LinoSanfilippo@gmx.de> |
|---|---|
| Date | 2016-07-25 21:20 +0200 |
| Subject | Re: [PATCH 3/3] staging: wilc1000: Reduce scope for a few variables in mac_ioctl() |
| Message-ID | <rYTzY-8r9-33@gated-at.bofh.it> |
>
> - 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
[toc] | [next] | [standalone]
| From | walter harms <wharms@bfs.de> |
|---|---|
| Date | 2016-07-25 21:40 +0200 |
| Message-ID | <rYTTk-60-19@gated-at.bofh.it> |
| In reply to | #1449763 |
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
[toc] | [prev] | [next] | [standalone]
| From | SF Markus Elfring <elfring@users.sourceforge.net> |
|---|---|
| Date | 2016-07-26 08:30 +0200 |
| Subject | Re: staging: wilc1000: Reduce scope for a few variables in mac_ioctl() |
| Message-ID | <rZ42m-6Xo-1@gated-at.bofh.it> |
| In reply to | #1449763 |
>> - 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
[toc] | [prev] | [next] | [standalone]
| From | "Lino Sanfilippo" <LinoSanfilippo@gmx.de> |
|---|---|
| Date | 2016-07-28 13:50 +0200 |
| Subject | Aw: Re: staging: wilc1000: Reduce scope for a few variables in mac_ioctl() |
| Message-ID | <rZRZ8-5HN-7@gated-at.bofh.it> |
| In reply to | #1450404 |
> 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
[toc] | [prev] | [next] | [standalone]
| From | SF Markus Elfring <elfring@users.sourceforge.net> |
|---|---|
| Date | 2016-07-28 18:10 +0200 |
| Subject | Re: staging: wilc1000: Reduce scope for a few variables in mac_ioctl() |
| Message-ID | <rZW2J-b8-7@gated-at.bofh.it> |
| In reply to | #1451892 |
> 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
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web