svn commit: r225947 - head/sys/netinet

[Bjoern A. Zeeb]

On 10. Oct 2011, at 08:44 , Qing Li wrote:

Hey,

> 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.

I am a bit confused by this entire thing; it seems it's only parts of what
I had looked at initially but maybe the commit was split due to follow-ups
on an early change.

I am however happy that some things I had mentioned initially are now being
addressed in the cleanup patch.  I have not checked the logic changes on it
however.

Thanks,


Bjoern

-- 
Bjoern A. Zeeb                                 You have to have visions!
         Stop bit received. Insert coin for new address family.