Christian S.J. Peron wrote:
Agreed,
This was a concern for me as well, I was pretty carefull about
managing the reference counts, I am currently testing this patch
with a variety of rule types to check for ucred leaks. If/before this patch
gets committed, I plan on doing another carefull scrutinization
of the ipfw code to make sure there are no return, continues or breaks
etc which could cause the ucred to be leaked.
This is exactly the problem I try to avoid. If there is the need to
double and triple check then I think there is a design error. If that
happens to any work I do I start over again unless I'm constrained by
external code. For example when I replied to your original email with
the patch I read it and said you'd re-do the lookup every time when
there is a miss. This is actually not the case. However it is not
really obvious what is going on there, even less so from just reading
the patch. And I'm well familiar with the ipfw2 code. Only after
putting it next to the full ip_fw2.c code and reading it slowly I saw
the roundabout of the function and how it does its thing. What I want
to say is that it is not obvious how it works and the next one working
on this code is almost certainly breaking it or leaking ucreds. I am
fully certain that you have tested it in any way to make sure it is
correct and I can tell you that it is correct now that I have checked
all function returns as well. But this is not write once and let it
be but write once, modify often and read all the time. I have stumbled
across too many 'short-cuts' in the past year and it made and makes
locking the network stack a real pain.
I do not object to your patch. I'm just highlighting the grief it might
cause later on. ;-)
--
Andre
_______________________________________________
[EMAIL PROTECTED] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[EMAIL PROTECTED]"
