On Thu, May 20, 2004 at 11:01:46PM +0200, Andre Oppermann wrote: +> Pawel Jakub Dawidek wrote: +> > +> > Hello. +> > +> > In in_pcblookup_hash() function, in the last loop if we find exact +> > match, we return immediately, if it is "wild", we store a pointer and +> > we countinue looking for exact match. +> > I wonder if this is ok, that we change pointer every time we find a +> > "wild" match. Is it inteded? Shouldn't it be: +> > +> > http://people.freebsd.org/~pjd/patches/in_pcb.c.2.patch +> +> No. This is a stack variable which is unconditionally initialized to +> NULL a few lines earlier. Checking for variable == NULL is always +> going to be true and makes your 'optimization' just redundand.
But we have loop there:
local_wild = NULL;
[...]
LIST_FOREACH(...) {
[...]
local_wild = <something>;
[...]
}
Isn't that possible that local_wild will be set few times?
+> > While I'm here, I want to improve code readability a bit:
+> >
+> > http://people.freebsd.org/~pjd/patches/in_pcb.c.3.patch
+> >
+> > Is it ok?
+>
+> No. You change the logic of the 'if' statements to something totally
+> different. This ain't going to work in any way.
Hmm, no:
for (...) {
if (a == b && c == d) {
/* do something */
}
/* nothing here */
}
is equivalent to:
for (...) {
if (a != b || c != d)
continue;
/* do something */
}
isn't it?
--
Pawel Jakub Dawidek http://www.FreeBSD.org
[EMAIL PROTECTED] http://garage.freebsd.pl
FreeBSD committer Am I Evil? Yes, I Am!
pgpJhFhhvth2K.pgp
Description: PGP signature
