hipl-core team mailing list archive

[Christof Mroz <christof.mroz@xxxxxxxxxxxxxx>]

On Thu, 26 Aug 2010 11:39:13 +0200, René Hummen  
<rene.hummen@xxxxxxxxxxxxxxxxx> wrote:

> > static int hip_fw_handle_other_output(hip_fw_context_t *ctx)
> > {
> > -    int verdict           = accept_normal_traffic_by_default;
> > +    HIP_DEBUG("accept_normal_traffic_by_default = %d\n",
> > +        accept_normal_traffic_by_default);
>
> Miss-aligned.

OK; it's 8 spaces, right?

> > -    HIP_DEBUG("\n");
> > +    int verdict = accept_normal_traffic_by_default;
>
> It's good practice to first declare variables used in a function and to  
> then add the remaining code afterwards.

OK, sorry.

> >     if (ctx->ip_version == 6 && hip_userspace_ipsec) {
> >
> > @@ -1565,9 +1566,13 @@
> >             // this might be a TCP packet for opportunistic mode
> >             HIP_DEBUG("plain TCP packet\n");
> >
> > +            struct tcphdr *tcphdr = (struct tcphdr*)
> > +                (((char *) iphdr) + ip_hdr_len);
> >             ctx->packet_type       = TCP_PACKET;
> > -            ctx->transport_hdr.tcp = (struct tcphdr *)
> > -                    (((char *) iphdr) + ip_hdr_len);
> > +            ctx->transport_hdr.tcp = tcphdr;
> > +
>
> Unrelated and unnecessary change.

tcphdr was used only once before, now it's used multiple times (stored in  
ctx and used in logging ports). See following lines:

> > +            HIP_DEBUG("src port: %u\n", ntohs(tcphdr->source));
> > +            HIP_DEBUG("dst port: %u\n", ntohs(tcphdr->dest));

Would inlining it be preferred? I'll fix as soon as I know this.