On Jun 21,  1:53pm, jonathan@dsg.stanford.edu (jonathan@dsg.stanford.edu) wrote:
-- Subject: Re: NetBSD-3 NIS-compat getpwnam()/getpwuid iterate entire map [w

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

That was what I based the code above...

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

I agree, but this is a much more intrusive change. And without good
regression tests something I fear to try.

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

I agree.

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

Thanks a lot, this sounds great.

christos