svn commit: r225947 - head/sys/netinet

Qing Li qingli at freebsd.org
Mon Oct 10 08:44:32 UTC 2011


Okay, now I know what's confusing you ... it's that bug I introduced :-)

The 2nd "if()" check on the RTF_GATEWAY flag should have been
an "else if()".

In a nutshell, the logic is any indirect route should fail the check,
except for that special host route.

Attached is the rework of that part of the patch, with some cleaning up
per your suggestion.

Thank you very much for catching that bug.

--Qing


> Q> > Well, after third review it is clear, that
> Q> > next if() case would definitely be true, and you would proceed
> Q> > with return. But that is difficult to see from first glance.
> Q>
> Q>    Not so, only for an indirect prefix route.
> Q>
> Q> > I'd suggest to remove error variable, return immediately in
> Q> > all error cases, and also the RTF_GATEWAY check can be shifted up,
> Q> > since it is the most simple and the most usual to be true.
> Q> >
> Q>
> Q>   No, the RTF_GATEWAY check cannot be shifted up because if we did
> Q>   that, the (indirect host route, with destination matching the gateway IP)
> Q>   would never be executed, if when that set of conditions are true, which is
> Q>   allowed and the reason for the patch in the first place.
>
> Can you elaborate on that please? As far as I see, any rtentry that has
> RTF_GATEWAY would return with EINVAL. The first if() clause doesn't
> do any actual processing, only checking flags and memcmp()ing. The third
> clause either. The error is never reset to 0. So, I don't see any
> difference in returning EINVAL for RTF_GATEWAY immediately or later
> after other checks.
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: in.c.diff
Type: application/octet-stream
Size: 2424 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/svn-src-head/attachments/20111010/8b7620ca/in.c.obj


More information about the svn-src-head mailing list