Re: [PATCH] xt_TCPOPTSTRIP 20071006 (kernel)

[Patrick McHardy <kaber@trash.net>]

Jan Engelhardt wrote:
> On Oct 8 2007 07:00, Patrick McHardy wrote:
> 
>>A few minor comments:
>>
>>
>>>+#define tcpoptstrip_set_bit(bmap, idx) \
>>>+	(bmap[(idx) >> 5] |= 1UL << (idx & 31))
>>>+#define tcpoptstrip_test_bit(bmap, idx) \
>>>+	(((1UL << (idx & 31)) & bmap[(idx) >> 5]) != 0)
>>
>>Why UL? You're using 32 bit variables for the bitmap ..
> 
> 
> Blame linux/bitmap.h. It uses 1UL, but at the same time, limits itself
> to uint32_t-based operations.


It uses purely unsigned long, just as the bitops do.

>>>+	nexthdr = ipv6h->nexthdr;
>>>+	tcphoff = ipv6_skip_exthdr(*pskb, sizeof(*ipv6h), &nexthdr);
>>
>>This needs #ifdef CONFIG_IPV6
> 
> 
> xt_TCPMSS also uses ipv6_skip_exthdr, but has no such protection.
> xt_hashlimit uses ipv6_find_hdr, would not that need CONFIG_IPV6 protection
> too?


Both use CONFIG_IP6_NF_IPTABLES, which is a superset of CONFIG_IPV6.
Will try if TCPOPTSTRIP actually needs that too.