Jan Engelhardt <jengelh@xxxxxxxxxx> wrote:
> On Friday 2010-12-17 16:27, Florian Westphal wrote:
> >
> >+++ ebt_ip6.c
> > if (info->bitmask & EBT_IP6_DPORT) {
> >- u32 dst = ntohs(pptr->dst);
> >+ u32 dst = ntohs(pptr->tcpudphdr.dst);
>
> The actual field however is smaller, so to me, this looks itching
> to be changed to
> uint16_t dst = ntohs(pptr->tcpudphdr.dst);
Indeed.
> >@@ -103,6 +115,14 @@ static int ebt_ip6_mt_check(const struct xt_mtchk_param *par)
> > return -EINVAL;
> > if (info->bitmask & EBT_IP6_SPORT && info->sport[0] > info->sport[1])
> > return -EINVAL;
> >+ if (info->bitmask & EBT_IP6_ICMP6) {
> >+ if ((info->invflags & EBT_IP6_PROTO ||
> >+ info->protocol != IPPROTO_ICMPV6))
> >+ return -EINVAL;
>
> Your parenthesis are a little out of balance, it's the & and | that
> are commonly guarded. That probably should have been:
>
> if ((info->invflags & EBT_IP6_PROTO) ||
> info->protocol != IPPROTO_ICMPV6)
Right.
> /* check other places too */
You are right,
If (foo & bar && baz)
is used in other places as well.
But I would prefer to keep cleanups and functional changes separated.
I will make the u16 and the () changes, however I'll wait a couple of
hours to give others time to speak up.
Thanks for reviewing.
Florian
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
