From: John Eaglesham <redacted>
Date: Sun, 1 Jul 2012 00:01:38 -0700
quoted hunk ↗ jump to hunk
- if (skb->protocol == htons(ETH_P_IP)) {
+ if (skb->protocol == htons(ETH_P_IP) &&
+ skb_network_header_len(skb) >= sizeof(struct iphdr)) {
This is not indented properly, the goal isn't to use only TAB
characters to indent, the goal it to line things up right after
the openning parenthesis on the previous line, so this should
be:
if (skb->protocol == htons(ETH_P_IP) &&
skb_network_header_len(skb) >= sizeof(struct iphdr)) {
+ } else if (skb->protocol == htons(ETH_P_IPV6) &&
+ skb_network_header_len(skb) >= sizeof(struct ipv6hdr)) {
Likewise, in this case you even under-indented it.
quoted hunk ↗ jump to hunk
+ v6hash =
+ (ipv6h->saddr.s6_addr32[1] ^ ipv6h->daddr.s6_addr32[1]) ^
+ (ipv6h->saddr.s6_addr32[2] ^ ipv6h->daddr.s6_addr32[2]) ^
+ (ipv6h->saddr.s6_addr32[3] ^ ipv6h->daddr.s6_addr32[3]);
This is rediculous, just put &ipv6h->saddr into a local pointer named
's' and then you won't have use such gymnastics to indent the code.
quoted hunk ↗ jump to hunk
if (!ip_is_fragment(iph) &&
- (iph->protocol == IPPROTO_TCP ||
- iph->protocol == IPPROTO_UDP)) {
+ (iph->protocol == IPPROTO_TCP ||
+ iph->protocol == IPPROTO_UDP)) {
This is what _REALLY_ bothers me. You took an existing conditional
which _WAS_ indented properly and you made erroneously re-indented.
In fact your only change here is to break the indentation.
Just remove these changes entirely.
+ if (iph->ihl * sizeof(u32) + sizeof(__be16) * 2 >
+ skb_headlen(skb) - skb_network_offset(skb))
Similarly, this is indented improperly.
quoted hunk ↗ jump to hunk
+ } else if (skb_network_header_len(skb) < sizeof(struct iphdr)) {
+ goto short_header;
}
Single line basic blocks do not use openning and closing braces.
quoted hunk ↗ jump to hunk
- return (layer4_xor ^
- ((ntohl(iph->saddr ^ iph->daddr)) & 0xffff)) % count;
-
+ return (layer4_xor ^ ((ntohl(iph->saddr ^ iph->daddr)) & 0xffff)) % count;
Don't reindent code unless you can do it properly, now this line is way
over 80 columns. The previous code was perfectly fine, leave it alone.
+ if (ipv6h->nexthdr == IPPROTO_TCP || ipv6h->nexthdr == IPPROTO_UDP) {
Line is too long, write it as:
if (ipv6h->nexthdr == IPPROTO_TCP ||
ipv6h->nexthdr == IPPROTO_UDP) {
+ __be16 *layer4hdrv6 = (__be16 *)((u8 *)ipv6h + sizeof(struct ipv6hdr));
Likewise, line is too long. Just do the variable declaration seperate from the
assignment:
__be16 *layer4hdrv6;
layer4hdrv6 = BLAH BLAH BLAH;
+ if (sizeof(struct ipv6hdr) + sizeof(__be16) * 2 >
+ skb_headlen(skb) - skb_network_offset(skb))
Improperly indented, fix.
quoted hunk ↗ jump to hunk
+ } else if (skb_network_header_len(skb) < sizeof(struct ipv6hdr)) {
+ goto short_header;
+ }
Since line basic block, no braces.
quoted hunk ↗ jump to hunk
+ layer4_xor ^=
+ (ipv6h->saddr.s6_addr32[1] ^ ipv6h->daddr.s6_addr32[1]) ^
+ (ipv6h->saddr.s6_addr32[2] ^ ipv6h->daddr.s6_addr32[2]) ^
+ (ipv6h->saddr.s6_addr32[3] ^ ipv6h->daddr.s6_addr32[3]);
This is gross, do as I said above using a local pointer variable.