Socket selection.

[Pawel Jakub Dawidek]

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
pjd at FreeBSD.org                           http://garage.freebsd.pl
FreeBSD committer                         Am I Evil? Yes, I Am!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 187 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-net/attachments/20040520/547f733d/attachment.bin