In message <20060621201934.288DA56539@rebar.astron.com>Christos Zoulas writes
>On Jun 21,  1:02pm, jonathan@dsg.stanford.edu (jonathan@dsg.stanford.edu) wrote:

>Something like this?
>
>Index: getpwent.c
>===================================================================
>RCS file: /cvsroot/src/lib/libc/gen/getpwent.c,v
>retrieving revision 1.71
>diff -u -u -r1.71 getpwent.c
>--- getpwent.c	19 Mar 2006 03:05:57 -0000	1.71
>+++ getpwent.c	21 Jun 2006 20:15:23 -0000
>@@ -1934,9 +1934,22 @@
> 
> 			case COMPAT_FULL:
> 					/* get next user */
>-				rv = _passwdcompat_pwscan(&cpw,
>-				    cbuf, sizeof(cbuf),
>-				    _PW_KEYBYNUM, NULL, 0);
>+				switch (search) {
>+				case _PW_KEYBYNUM:
>+					rv = _passwdcompat_pwscan(&cpw, cbuf,
>+					    sizeof(cbuf), search, NULL, 0);
>+					break;
>+				case _PW_KEYBYNAME:
>+					rv = _passwdcompat_pwscan(&cpw, cbuf,
>+					    sizeof(cbuf), search, name, 0);
>+					break;
>+				case _PW_KEYBYUID:
>+					rv = _passwdcompat_pwscan(&cpw, cbuf,
>+					    sizeof(cbuf), search, NULL, uid);
>+					break;
>+				default:
>+					abort();
>+				}
> 				if (rv != NS_SUCCESS)
> 					state->mode = COMPAT_NONE;
> 				break;

That should work. (Did you see the similar code fragment I emailed you
about 9 hours ago?  I never saw a reply to that.)

The control flow is pretty hairy.  That last "break;" take us to the
code which copies cpw to pw, then does the exclusion check. if the
exclusion check doesn't reject the match, and we have a key then we
return pw to the caller. Following that code-flow is (given Stephen's
bug), a maintenance problem.

I think inverting the current logic (move the iteration for non-key
lookups inside the switch you suggest above, rather than having the
outer for(;;) loop iterate over both the local file *and* non-key compat lookups)
could yield cleaner, more maintainable code; I'm I'm still looking at that.

Also, We also need to look at, or test, lib/libc/gen/getgrent.c, to
check whether it has the same underlying flaw.  Anyone got a YP server
with 50,000 entries in group.by{name,number} to test?


>| Oh... and I noticed a bug in clnt_generic(): for RPC-over-TCP, we
>| disable RFC-896 (Nagle) processing for TCP over IPv4, but not for TCP
>| over IPv6[1].  Makes a grown man cry.
>
>And something like this?

[...]

Almost. Before calling a TCP setsockopt(), we should check that
netconfig specifies "tcp", to future-proof against TI-RPC over non-TCP
ordered reliable streams.  (ISO TP4?).  I'll fix that, and put in an
XXX citing RFC-896 and why we need to disable it here, to prevent any
future confusion.  I'll fix that, then request pullups to netbsd-2,
netbsd-3, and whatever else releng suggests.