On Tue, 2007-12-06 at 15:45 +0200, Patrick McHardy wrote:
> Looks good too me, just a few minor nitpicks as usual :)
I like the nitpicks - they make the code better (as long as we put
a time limit on them ;->)
>
> ^^ please delete empty line
will do.
> > + if (x->km.state != XFRM_STATE_VALID)
> > + continue;
>
> ^ one indentation level too much
will fix.
> The whole thing could be compacted by moving the XFRM_STATE_VALID
> check to the first condition:
>
> if (x->props.family == family &&
> x->props.reqid == reqid &&
> !(x->props.flags & XFRM_STATE_WILDRECV) &&
> xfrm_state_addr_check(x, daddr, saddr, family) &&
> mode == x->props.mode &&
> proto == x->id.proto &&
> x->km.state == XFRM_STATE_VALID) {
> rx = x;
> break;
> }
>
> or alternatively turn the != XFRM_STATE_VALID into == if you
> want to keep the first condition similar to xfrm_state_find
> (but the mode and proto conditions are reversed anyways).
>
Will do.
> BTW, wouldn't it make sense to allow use of the SPI as well?
SPI is the least user friendly parameter - but i could add it later.
I want to add tunnel mode next then i can revisit SPI.
Thanks for taking the time to review this Patrick.
cheers,
jamal
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
