| Message ID | 1426216532-24058-15-git-send-email-m.v.b@runbox.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Kalle Valo |
| Headers | show |
On 03/12/2015 10:15 PM, M. Vefa Bicakci wrote: > Improve the readability of an if statement and its very long trace > output line in rtl8723au's rtw_security.c. > > Signed-off-by: M. Vefa Bicakci <m.v.b@runbox.com> > --- > drivers/staging/rtl8723au/core/rtw_security.c | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/drivers/staging/rtl8723au/core/rtw_security.c b/drivers/staging/rtl8723au/core/rtw_security.c > index dbbe7b348d..fd04ae78bb 100644 > --- a/drivers/staging/rtl8723au/core/rtw_security.c > +++ b/drivers/staging/rtl8723au/core/rtw_security.c > @@ -759,9 +759,21 @@ int rtw_tkip_decrypt23a(struct rtw_adapter *padapter, > > *((u32 *)crc) = le32_to_cpu(getcrc32(payload, length-4)); > > - if (crc[3] != payload[length - 1] || crc[2] != payload[length - 2] || crc[1] != payload[length - 3] || crc[0] != payload[length - 4]) { > - RT_TRACE(_module_rtl871x_security_c_, _drv_err_, ("rtw_wep_decrypt23a:icv error crc[3](%x)!= payload[length-1](%x) || crc[2](%x)!= payload[length-2](%x) || crc[1](%x)!= payload[length-3](%x) || crc[0](%x)!= payload[length-4](%x)\n", > - crc[3], payload[length - 1], crc[2], payload[length - 2], crc[1], payload[length - 3], crc[0], payload[length - 4])); > + if (crc[3] != payload[length - 1] || > + crc[2] != payload[length - 2] || > + crc[1] != payload[length - 3] || > + crc[0] != payload[length - 4]) { The continuation lines should be lined up with the opening paren. This should be if (crc[3] != payload[length - 1] || crc[2] != payload[length - 2] || crc[1] != payload[length - 3] || crc[0] != payload[length - 4]) { > + > + RT_TRACE(_module_rtl871x_security_c_, _drv_err_, > + ("rtw_wep_decrypt23a:icv error " > + "crc[3](%x)!= payload[length-1](%x) || " > + "crc[2](%x)!= payload[length-2](%x) || " > + "crc[1](%x)!= payload[length-3](%x) || " > + "crc[0](%x)!= payload[length-4](%x)\n", > + crc[3], payload[length - 1], > + crc[2], payload[length - 2], > + crc[1], payload[length - 3], > + crc[0], payload[length - 4])); Same problem here. Larry -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2015-03-13 at 10:44 -0500, Larry Finger wrote: > On 03/12/2015 10:15 PM, M. Vefa Bicakci wrote: > > Improve the readability of an if statement and its very long trace > > output line in rtl8723au's rtw_security.c. [] > > diff --git a/drivers/staging/rtl8723au/core/rtw_security.c b/drivers/staging/rtl8723au/core/rtw_security.c [] > > @@ -759,9 +759,21 @@ int rtw_tkip_decrypt23a(struct rtw_adapter *padapter, > > > > *((u32 *)crc) = le32_to_cpu(getcrc32(payload, length-4)); > > > > - if (crc[3] != payload[length - 1] || crc[2] != payload[length - 2] || crc[1] != payload[length - 3] || crc[0] != payload[length - 4]) { > > - RT_TRACE(_module_rtl871x_security_c_, _drv_err_, ("rtw_wep_decrypt23a:icv error crc[3](%x)!= payload[length-1](%x) || crc[2](%x)!= payload[length-2](%x) || crc[1](%x)!= payload[length-3](%x) || crc[0](%x)!= payload[length-4](%x)\n", > > - crc[3], payload[length - 1], crc[2], payload[length - 2], crc[1], payload[length - 3], crc[0], payload[length - 4])); > > + if (crc[3] != payload[length - 1] || > > + crc[2] != payload[length - 2] || > > + crc[1] != payload[length - 3] || > > + crc[0] != payload[length - 4]) { > > The continuation lines should be lined up with the opening paren. This should be > > if (crc[3] != payload[length - 1] || > crc[2] != payload[length - 2] || > crc[1] != payload[length - 3] || > crc[0] != payload[length - 4]) { Maybe: if (memcmp(&crc, &payload[length - 4], 4)) { RT_TRACE(etc, "%08x vs %4phN", crc, &payload[length - 4]) -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 13/03/15 12:24 PM, Joe Perches wrote: > On Fri, 2015-03-13 at 10:44 -0500, Larry Finger wrote: >> On 03/12/2015 10:15 PM, M. Vefa Bicakci wrote: >>> Improve the readability of an if statement and its very long trace >>> output line in rtl8723au's rtw_security.c. > [] >>> diff --git a/drivers/staging/rtl8723au/core/rtw_security.c b/drivers/staging/rtl8723au/core/rtw_security.c > [] >>> @@ -759,9 +759,21 @@ int rtw_tkip_decrypt23a(struct rtw_adapter *padapter, >>> >>> *((u32 *)crc) = le32_to_cpu(getcrc32(payload, length-4)); >>> >>> - if (crc[3] != payload[length - 1] || crc[2] != payload[length - 2] || crc[1] != payload[length - 3] || crc[0] != payload[length - 4]) { >>> - RT_TRACE(_module_rtl871x_security_c_, _drv_err_, ("rtw_wep_decrypt23a:icv error crc[3](%x)!= payload[length-1](%x) || crc[2](%x)!= payload[length-2](%x) || crc[1](%x)!= payload[length-3](%x) || crc[0](%x)!= payload[length-4](%x)\n", >>> - crc[3], payload[length - 1], crc[2], payload[length - 2], crc[1], payload[length - 3], crc[0], payload[length - 4])); >>> + if (crc[3] != payload[length - 1] || >>> + crc[2] != payload[length - 2] || >>> + crc[1] != payload[length - 3] || >>> + crc[0] != payload[length - 4]) { >> >> The continuation lines should be lined up with the opening paren. This should be >> >> if (crc[3] != payload[length - 1] || >> crc[2] != payload[length - 2] || >> crc[1] != payload[length - 3] || >> crc[0] != payload[length - 4]) { > > Maybe: > if (memcmp(&crc, &payload[length - 4], 4)) { > RT_TRACE(etc, "%08x vs %4phN", > crc, &payload[length - 4]) Hello Larry and Joe, Thank you for the reviews! I have addressed your comments in my local repository, and I hope to send out the second version of the commits for review by tomorrow. Vefa -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, 2015-03-14 at 16:16 -0400, M. Vefa Bicakci wrote: > On 13/03/15 12:24 PM, Joe Perches wrote: > > On Fri, 2015-03-13 at 10:44 -0500, Larry Finger wrote: > >> On 03/12/2015 10:15 PM, M. Vefa Bicakci wrote: > >>> Improve the readability of an if statement and its very long trace > >>> output line in rtl8723au's rtw_security.c. > > [] > >>> diff --git a/drivers/staging/rtl8723au/core/rtw_security.c b/drivers/staging/rtl8723au/core/rtw_security.c > > [] > >>> @@ -759,9 +759,21 @@ int rtw_tkip_decrypt23a(struct rtw_adapter *padapter, > >>> > >>> *((u32 *)crc) = le32_to_cpu(getcrc32(payload, length-4)); > >>> > >>> - if (crc[3] != payload[length - 1] || crc[2] != payload[length - 2] || crc[1] != payload[length - 3] || crc[0] != payload[length - 4]) { > >>> - RT_TRACE(_module_rtl871x_security_c_, _drv_err_, ("rtw_wep_decrypt23a:icv error crc[3](%x)!= payload[length-1](%x) || crc[2](%x)!= payload[length-2](%x) || crc[1](%x)!= payload[length-3](%x) || crc[0](%x)!= payload[length-4](%x)\n", > >>> - crc[3], payload[length - 1], crc[2], payload[length - 2], crc[1], payload[length - 3], crc[0], payload[length - 4])); > >>> + if (crc[3] != payload[length - 1] || > >>> + crc[2] != payload[length - 2] || > >>> + crc[1] != payload[length - 3] || > >>> + crc[0] != payload[length - 4]) { > >> > >> The continuation lines should be lined up with the opening paren. This should be > >> > >> if (crc[3] != payload[length - 1] || > >> crc[2] != payload[length - 2] || > >> crc[1] != payload[length - 3] || > >> crc[0] != payload[length - 4]) { > > > > Maybe: > > if (memcmp(&crc, &payload[length - 4], 4)) { > > RT_TRACE(etc, "%08x vs %4phN", > > crc, &payload[length - 4]) [] > I have addressed your comments in my local repository, and I hope to send out > the second version of the commits for review by tomorrow. Maybe it'd be more sensible to use something like: u32 crc, orig_crc; [...] crc = le32_to_cpu(getcrc32(payload, length - 4)); orig_crc = le32_to_cpu(get_unaligned_le32(&payload[length - 4])); if (crc != orig_crc) { RT_TRACE(etc, "bad crc: %08x vs %08x", crc, orig_crc); -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 14/03/15 05:55 PM, Joe Perches wrote: > On Sat, 2015-03-14 at 16:16 -0400, M. Vefa Bicakci wrote: >> On 13/03/15 12:24 PM, Joe Perches wrote: >>> On Fri, 2015-03-13 at 10:44 -0500, Larry Finger wrote: >>>> On 03/12/2015 10:15 PM, M. Vefa Bicakci wrote: >>>>> Improve the readability of an if statement and its very long trace >>>>> output line in rtl8723au's rtw_security.c. >>> [] >>>>> diff --git a/drivers/staging/rtl8723au/core/rtw_security.c b/drivers/staging/rtl8723au/core/rtw_security.c >>> [] >>>>> @@ -759,9 +759,21 @@ int rtw_tkip_decrypt23a(struct rtw_adapter *padapter, >>>>> >>>>> *((u32 *)crc) = le32_to_cpu(getcrc32(payload, length-4)); >>>>> >>>>> - if (crc[3] != payload[length - 1] || crc[2] != payload[length - 2] || crc[1] != payload[length - 3] || crc[0] != payload[length - 4]) { >>>>> - RT_TRACE(_module_rtl871x_security_c_, _drv_err_, ("rtw_wep_decrypt23a:icv error crc[3](%x)!= payload[length-1](%x) || crc[2](%x)!= payload[length-2](%x) || crc[1](%x)!= payload[length-3](%x) || crc[0](%x)!= payload[length-4](%x)\n", >>>>> - crc[3], payload[length - 1], crc[2], payload[length - 2], crc[1], payload[length - 3], crc[0], payload[length - 4])); >>>>> + if (crc[3] != payload[length - 1] || >>>>> + crc[2] != payload[length - 2] || >>>>> + crc[1] != payload[length - 3] || >>>>> + crc[0] != payload[length - 4]) { >>>> >>>> The continuation lines should be lined up with the opening paren. This should be >>>> >>>> if (crc[3] != payload[length - 1] || >>>> crc[2] != payload[length - 2] || >>>> crc[1] != payload[length - 3] || >>>> crc[0] != payload[length - 4]) { >>> >>> Maybe: >>> if (memcmp(&crc, &payload[length - 4], 4)) { >>> RT_TRACE(etc, "%08x vs %4phN", >>> crc, &payload[length - 4]) > [] >> I have addressed your comments in my local repository, and I hope to send out >> the second version of the commits for review by tomorrow. > > Maybe it'd be more sensible to use something like: > > u32 crc, orig_crc; > > [...] > > crc = le32_to_cpu(getcrc32(payload, length - 4)); > orig_crc = le32_to_cpu(get_unaligned_le32(&payload[length - 4])); > > if (crc != orig_crc) { > RT_TRACE(etc, "bad crc: %08x vs %08x", crc, orig_crc); > Hello Joe, Thank you for the suggestion! I have just sent out the second version of the commits for review, and I implemented your suggestion as well. Vefa -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/staging/rtl8723au/core/rtw_security.c b/drivers/staging/rtl8723au/core/rtw_security.c index dbbe7b348d..fd04ae78bb 100644 --- a/drivers/staging/rtl8723au/core/rtw_security.c +++ b/drivers/staging/rtl8723au/core/rtw_security.c @@ -759,9 +759,21 @@ int rtw_tkip_decrypt23a(struct rtw_adapter *padapter, *((u32 *)crc) = le32_to_cpu(getcrc32(payload, length-4)); - if (crc[3] != payload[length - 1] || crc[2] != payload[length - 2] || crc[1] != payload[length - 3] || crc[0] != payload[length - 4]) { - RT_TRACE(_module_rtl871x_security_c_, _drv_err_, ("rtw_wep_decrypt23a:icv error crc[3](%x)!= payload[length-1](%x) || crc[2](%x)!= payload[length-2](%x) || crc[1](%x)!= payload[length-3](%x) || crc[0](%x)!= payload[length-4](%x)\n", - crc[3], payload[length - 1], crc[2], payload[length - 2], crc[1], payload[length - 3], crc[0], payload[length - 4])); + if (crc[3] != payload[length - 1] || + crc[2] != payload[length - 2] || + crc[1] != payload[length - 3] || + crc[0] != payload[length - 4]) { + + RT_TRACE(_module_rtl871x_security_c_, _drv_err_, + ("rtw_wep_decrypt23a:icv error " + "crc[3](%x)!= payload[length-1](%x) || " + "crc[2](%x)!= payload[length-2](%x) || " + "crc[1](%x)!= payload[length-3](%x) || " + "crc[0](%x)!= payload[length-4](%x)\n", + crc[3], payload[length - 1], + crc[2], payload[length - 2], + crc[1], payload[length - 3], + crc[0], payload[length - 4])); res = _FAIL; }
Improve the readability of an if statement and its very long trace output line in rtl8723au's rtw_security.c. Signed-off-by: M. Vefa Bicakci <m.v.b@runbox.com> --- drivers/staging/rtl8723au/core/rtw_security.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)