| Message ID | 1350965397-12384-2-git-send-email-tony.cheneau@amnesiak.org |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Tony Cheneau <tony.cheneau@amnesiak.org> Date: Tue, 23 Oct 2012 00:09:43 -0400 > The current test is not RFC6282 compliant. The same issue has been found > out and fixed in Contiki. This patch is basicaly a port of their fix. > > Signed-off-by: Tony Cheneau <tony.cheneau@amnesiak.org> It's really demoralizing to sit down to read a large 15 entry patch series and this is the first thing I see: > /* > - * check whether we can compress the IID to 16 bits, > - * it's possible for unicast adresses with first 49 bits are zero only. > - */ > +* check whether we can compress the IID to 16 bits, > +* it's possible for unicast adresses with first 49 bits are zero only. > +*/ Don't break the comment indentation, it was perfectly fine. I'm not reviewing the rest of the series, you'll need to repost the entire thing after you fix this. And if you're smart you'll make sure there aren't similar coding style issues in the rest of your patches, else it's going to take a long time to merge this crap into the tree. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/23/2012 06:09 AM, Tony Cheneau wrote: > #define lowpan_is_iid_16_bit_compressable(a) \ > ((((a)->s6_addr16[4]) == 0) && \ > - (((a)->s6_addr16[5]) == 0) && \ > - (((a)->s6_addr16[6]) == 0) && \ > - ((((a)->s6_addr[14]) & 0x80) == 0)) > + (((a)->s6_addr[10]) == 0) && \ > + (((a)->s6_addr[11]) == 0xff) && \ > + (((a)->s6_addr[12]) == 0xfe) && \ > + (((a)->s6_addr[13]) == 0)) > + > > /* multicast address */ > #define is_addr_mcast(a) (((a)->s6_addr[0]) == 0xFF) > Is the additional newline needed at the end of the #define? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello David, Le 23.10.2012 08:49, David Miller a écrit : [...] > > It's really demoralizing to sit down to read a large 15 entry > patch series and this is the first thing I see: > >> /* >> - * check whether we can compress the IID to 16 bits, >> - * it's possible for unicast adresses with first 49 bits are zero >> only. >> - */ >> +* check whether we can compress the IID to 16 bits, >> +* it's possible for unicast adresses with first 49 bits are zero >> only. >> +*/ > > Don't break the comment indentation, it was perfectly fine. > > I'm not reviewing the rest of the series, you'll need to repost > the entire thing after you fix this. > > And if you're smart you'll make sure there aren't similar coding > style issues in the rest of your patches, else it's going to take > a long time to merge this crap into the tree. Sorry about that, I failed to catch that mistake of mine (and several other ones that Jan spotted). I'll rework my patchset and resubmit. Thank you for pointing out this issue. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe netdev" 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/net/ieee802154/6lowpan.h b/net/ieee802154/6lowpan.h index 8c2251f..efd1a57 100644 --- a/net/ieee802154/6lowpan.h +++ b/net/ieee802154/6lowpan.h @@ -87,14 +87,16 @@ #define is_addr_link_local(a) (((a)->s6_addr16[0]) == 0x80FE) /* - * check whether we can compress the IID to 16 bits, - * it's possible for unicast adresses with first 49 bits are zero only. - */ +* check whether we can compress the IID to 16 bits, +* it's possible for unicast adresses with first 49 bits are zero only. +*/ #define lowpan_is_iid_16_bit_compressable(a) \ ((((a)->s6_addr16[4]) == 0) && \ - (((a)->s6_addr16[5]) == 0) && \ - (((a)->s6_addr16[6]) == 0) && \ - ((((a)->s6_addr[14]) & 0x80) == 0)) + (((a)->s6_addr[10]) == 0) && \ + (((a)->s6_addr[11]) == 0xff) && \ + (((a)->s6_addr[12]) == 0xfe) && \ + (((a)->s6_addr[13]) == 0)) + /* multicast address */ #define is_addr_mcast(a) (((a)->s6_addr[0]) == 0xFF)
The current test is not RFC6282 compliant. The same issue has been found out and fixed in Contiki. This patch is basicaly a port of their fix. Signed-off-by: Tony Cheneau <tony.cheneau@amnesiak.org> --- net/ieee802154/6lowpan.h | 14 ++++++++------ 1 files changed, 8 insertions(+), 6 deletions(-)