2000-12-26 Neal H Walfield <neal@cs.uml.edu>

	* cache.c: Change cache/hash table size to 509, a prime.  Use
	memcpy/memcmp not bcopy/bcmp.  Verify return value from malloc and
	check the result of rpc transaction _and_ do not act if failed.
	* main.c: Correct the wording of the help messages.  Do not
	bother initializing global variable to 0.  Use memcpy/memcmp not
	bcopy/bcmp.  Verify return value from malloc and check the result
	of rpc transaction _and_ do not act if failed.
	* mount.c: Check return values of initialize_rpc.  Use
	memcpy/memcmp not bcopy/bcmp.  Verify return value from malloc and
	strdup.  Correct comments.
	(mount_root): Check result of connect.  Handle errors
	consistently.  Reverse loops that are if (! c) {} else when
	appropriate.
	* mount.h: Protect header with #ifdef.
	* name-cache.c: Correct dangerous NPARTIALS macro.  Use
	memcpy/memcmp not bcopy/bcmp.
	(find_cache): Use PARTIAL_THRESH, not the constant.
	* nfs-spec.h: Protect header with #ifdef.
	* nfs.c: Use memcpy/memcmp not bcopy/bcmp.
	* nfs.h: Likewise.
        * ops.c (netfs_attempt_mkdir): Check return values of initialize_rpc.
	Use memcpy/memcmp not bcopy/bcmp.  Verify return value from malloc and
	check the result of rpc transaction _and_ do not act if failed.
	(netfs_attempt_link): Unlock the directory before the rpc transaction.
	Check the result of rpc transaction _and_ do not act if failed.
        * pager.c: Remove, we do not use it.
	* rpc.c:  Use memcpy/memcmp not bcopy/bcmp.  Verify return value from
	malloc and check the result of rpc transaction _and_ do not act if
	failed.
	(initialize_rpc): Use AUTH_NONE, not the depreciated
	AUTH_NULL.  Return sane values on failure.
	(generate_xid): Make inline.
	(link_rpc): New function.  Complements unlink_rpc.
	(conduct_rpc): Use link_rpc.
	(rpc_receive_thread): Reroll to a single loop.

11 files changed, 436 insertions, 181 deletions

diff --git a/nfs/ChangeLog b/nfs/ChangeLog
index 8894c435..0ece7b21 100644
--- a/nfs/ChangeLog
+++ b/nfs/ChangeLog
@@ -1,3 +1,41 @@
+2000-12-26  Neal H Walfield <neal@cs.uml.edu>
+
+	* cache.c: Change cache/hash table size to 509, a prime.  Use
+	memcpy/memcmp not bcopy/bcmp.  Verify return value from malloc and
+	check the result of rpc transaction _and_ do not act if failed.
+	* main.c: Correct the wording of the help messages.  Do not
+	bother initializing global variable to 0.  Use memcpy/memcmp not
+	bcopy/bcmp.  Verify return value from malloc and check the result
+	of rpc transaction _and_ do not act if failed.
+	* mount.c: Check return values of initialize_rpc.  Use
+	memcpy/memcmp not bcopy/bcmp.  Verify return value from malloc and
+	strdup.  Correct comments.
+	(mount_root): Check result of connect.  Handle errors
+	consistently.  Reverse loops that are if (! c) {} else when
+	appropriate.
+	* mount.h: Protect header with #ifdef.
+	* name-cache.c: Correct dangerous NPARTIALS macro.  Use
+	memcpy/memcmp not bcopy/bcmp.
+	(find_cache): Use PARTIAL_THRESH, not the constant.
+	* nfs-spec.h: Protect header with #ifdef.
+	* nfs.c: Use memcpy/memcmp not bcopy/bcmp.
+	* nfs.h: Likewise.
+        * ops.c (netfs_attempt_mkdir): Check return values of initialize_rpc.
+	Use memcpy/memcmp not bcopy/bcmp.  Verify return value from malloc and
+	check the result of rpc transaction _and_ do not act if failed.
+	(netfs_attempt_link): Unlock the directory before the rpc transaction.
+	Check the result of rpc transaction _and_ do not act if failed.
+        * pager.c: Remove, we do not use it.
+	* rpc.c:  Use memcpy/memcmp not bcopy/bcmp.  Verify return value from
+	malloc and check the result of rpc transaction _and_ do not act if
+	failed.
+	(initialize_rpc): Use AUTH_NONE, not the depreciated
+	AUTH_NULL.  Return sane values on failure.
+	(generate_xid): Make inline.
+	(link_rpc): New function.  Complements unlink_rpc.
+	(conduct_rpc): Use link_rpc.
+	(rpc_receive_thread): Reroll to a single loop.
+
 2000-11-26  Marcus Brinkmann  <marcus@gnu.org>
 
 	* ops.c (netfs_attempt_mkdir): Add casts -1 -> (struct iouser *) -1
diff --git a/nfs/cache.c b/nfs/cache.c
index 183660b0..3cdb527f 100644
--- a/nfs/cache.c
+++ b/nfs/cache.c
@@ -23,8 +23,11 @@
 #include <string.h>
 #include <netinet/in.h>
 
-/* Hash table containing all the nodes currently active. */
-#define CACHESIZE 512
+/* Hash table containing all the nodes currently active.
+   XXX Was 512, however, a prime is much nice for the hash
+   function. 509 is nice as not only is it prime, it keeps
+   the array within a page or two */
+#define CACHESIZE 509
 static struct node *nodehash [CACHESIZE];
 
 /* Compute and return a hash key for NFS file handle DATA of LEN bytes. */
@@ -44,7 +47,8 @@ hash (int *data, size_t len)
 /* Lookup the file handle P (length LEN) in the hash table.  If it is
    not present, initialize a new node structure and insert it into the
    hash table.  Whichever course, a new reference is generated and the
-   node is returned in *NPP.  */
+   node is returned in *NPP; the lock on the node, (*NPP)->LOCK, is
+   held.  */
 void
 lookup_fhandle (void *p, size_t len, struct node **npp)
 {
@@ -58,7 +62,7 @@ lookup_fhandle (void *p, size_t len, struct node **npp)
   for (np = nodehash[h]; np; np = np->nn->hnext)
     {
       if (np->nn->handle.size != len
-	  || bcmp (np->nn->handle.data, p, len) != 0)
+	  || memcmp (np->nn->handle.data, p, len) != 0)
 	continue;
       
       np->references++;
@@ -68,9 +72,12 @@ lookup_fhandle (void *p, size_t len, struct node **npp)
       return;
     }
   
+  /* Could not find it */
   nn = malloc (sizeof (struct netnode));
+  assert (nn);
+
   nn->handle.size = len;
-  bcopy (p, nn->handle.data, len);
+  memcpy (nn->handle.data, p, len);
   nn->stat_updated = 0;
   nn->dtrans = NOT_POSSIBLE;
   nn->dead_dir = 0;
@@ -112,8 +119,9 @@ forked_node_delete (any_t arg)
 };
 
 /* Called by libnetfs when node NP has no more references.  (See
-   <hurd/libnetfs.h> for details.  Just clear local state and remove
-   from the hash table. */
+   <hurd/libnetfs.h> for details.  Just clear its local state and
+   remove it from the hash table.  Called and expected to leave with
+   NETFS_NODE_REFCNT_LOCK held. */
 void
 netfs_node_norefs (struct node *np)
 {
@@ -122,6 +130,7 @@ netfs_node_norefs (struct node *np)
       struct fnd *args;
 
       args = malloc (sizeof (struct fnd));
+      assert (args);
 
       np->references++;
       spin_unlock (&netfs_node_refcnt_lock);
@@ -153,7 +162,7 @@ netfs_node_norefs (struct node *np)
 
 /* Change the file handle used for node NP to be the handle at P.
    Make sure the hash table stays up to date.  Return the address
-   after the hnadle. */
+   after the handle.  The lock on the node should be held. */
 int *
 recache_handle (int *p, struct node *np)
 {
@@ -165,14 +174,17 @@ recache_handle (int *p, struct node *np)
   else
     len = ntohl (*p++);
   
+  /* Unlink it */
   spin_lock (&netfs_node_refcnt_lock);
   *np->nn->hprevp = np->nn->hnext;
   if (np->nn->hnext)
     np->nn->hnext->nn->hprevp = np->nn->hprevp;
 
+  /* Change the name */
   np->nn->handle.size = len;
-  bcopy (p, np->nn->handle.data, len);
+  memcpy (np->nn->handle.data, p, len);
   
+  /* Reinsert it */
   h = hash (p, len);
   np->nn->hnext = nodehash[h];
   if (np->nn->hnext)
@@ -183,4 +195,3 @@ recache_handle (int *p, struct node *np)
   return p + len / sizeof (int);
 }
 
-
diff --git a/nfs/main.c b/nfs/main.c
index 92a49861..e38f4061 100644
--- a/nfs/main.c
+++ b/nfs/main.c
@@ -34,13 +34,13 @@
 extern char *localhost ();
 
 /* Default number of times to retry RPCs when mounted soft. */
-#define DEFAULT_SOFT_RETRIES  3	
+#define DEFAULT_SOFT_RETRIES  3
 
 /* Default number of seconds to timeout cached stat information. */
 #define DEFAULT_STAT_TIMEOUT  3
 
 /* Default number of seconds to timeout cached file contents. */
-#define DEFAULT_CACHE_TIMEOUT 3	
+#define DEFAULT_CACHE_TIMEOUT 3
 
 /* Default number of seconds to timeout cache positive dir hits. */
 #define DEFAULT_NAME_CACHE_TIMEOUT 3
@@ -49,10 +49,10 @@ extern char *localhost ();
 #define DEFAULT_NAME_CACHE_NEG_TIMEOUT 3
 
 /* Default maximum number of bytes to read at once. */
-#define DEFAULT_READ_SIZE     8192 
+#define DEFAULT_READ_SIZE     8192
 
 /* Default maximum number of bytes to write at once. */
-#define DEFAULT_WRITE_SIZE    8192 
+#define DEFAULT_WRITE_SIZE    8192
 
 
 /* Number of seconds to timeout cached stat information. */
@@ -114,8 +114,8 @@ static const struct argp_option common_options[] =
 {
   {0,0,0,0,0,1},
   {"soft",		    OPT_SOFT,	   "RETRIES", OPTION_ARG_OPTIONAL,
-     "File system requests will eventually fail, after RETRIES tries if"
-     " specified, otherwise " _D(SOFT_RETRIES)},
+     "File system requests will eventually fail, after RETRIES tries"
+     " (default " _D(SOFT_RETRIES) ")" },
   {"hard",		    OPT_HARD, 0, 0,
      "Retry file systems requests until they succeed"},
 
@@ -134,10 +134,10 @@ static const struct argp_option common_options[] =
      "Timeout for cached file data (default " _D(CACHE_TIMEOUT) ")"},
   {"name-cache-timeout",    OPT_NCACHE_TO, "SEC", 0,
      "Timeout for positive directory cache entries (default " 
-   _D(NAME_CACHE_TIMEOUT) ")"},
+     _D(NAME_CACHE_TIMEOUT) ")"},
   {"name-cache-neg-timeout", OPT_NCACHE_NEG_TO, "SEC", 0,
      "Timeout for negative directory cache entires (default "
-   _D(NAME_CACHE_NEG_TIMEOUT) ")"},
+      _D(NAME_CACHE_NEG_TIMEOUT) ")"},
   {"init-transmit-timeout", OPT_INIT_TR_TO,"SEC", 0}, 
   {"max-transmit-timeout",  OPT_MAX_TR_TO, "SEC", 0}, 
 
@@ -197,7 +197,7 @@ static const struct argp_option startup_options[] = {
 static char *args_doc = "REMOTE_FS [HOST]";
 static char *doc = "Hurd nfs translator"
 "\vIf HOST is not specified, an attempt is made to extract"
-" it from REMOTE_FS, using either the `HOST:FS' or `FS@HOST' notations.";
+" it from REMOTE_FS using either the `HOST:FS' or `FS@HOST' notations.";
 
 static const struct argp_child
 runtime_argp_children[] = { {&netfs_std_runtime_argp}, {0} };
@@ -205,12 +205,12 @@ static struct argp
 runtime_argp = { common_options, parse_common_opt, 0, 0,
 		 runtime_argp_children };
 
-/* Use by netfs_set_options to handle runtime option parsing.  */
+/* Used by netfs_set_options to handle runtime option parsing.  */
 struct argp *netfs_runtime_argp = &runtime_argp;
 
 /* Where to find the remote filesystem.  */
-static char *remote_fs = 0;
-static char *host = 0;
+static char *remote_fs; /* = 0; */
+static char *host; /* = 0; */
 
 /* Return an argz string describing the current options.  Fill *ARGZ
    with a pointer to newly malloced storage holding the list and *LEN
@@ -272,6 +272,8 @@ extract_nfs_args (char *spec, char **remote_fs, char **host)
   char *sep;
 
   spec = strdup (spec);		/* So we can trash it.  */
+  if (! spec)
+    return NULL;
 
   sep = index (spec, ':');
   if (sep)
diff --git a/nfs/mount.c b/nfs/mount.c
index 92af75f8..a3a6887e 100644
--- a/nfs/mount.c
+++ b/nfs/mount.c
@@ -87,13 +87,13 @@ mount_initialize_rpc (int procnum, void **buf)
 }
 
 /* Using the mount protocol, lookup NAME at host HOST.
-   Return a node for it or null for an error. */
+   Return a node for it or null for an error.  If an
+   error occurs, a message is automatically sent to stderr. */
 struct node *
 mount_root (char *name, char *host)
 {
   struct sockaddr_in addr;
   struct hostent *h;
-  struct servent *s;
   int *p;
   void *rpcbuf;
   int port;
@@ -103,6 +103,14 @@ mount_root (char *name, char *host)
   /* Lookup the portmapper port number */
   if (pmap_service_name)
     {
+      struct servent *s;
+
+      /* XXX This will always fail! pmap_service_name will always be "sunrpc"
+         What should pmap_service_name really be?  By definition the second
+	 argument is either "tcp" or "udp"  Thus, is this backwards
+	 (as service_name suggests)?  If so, should it read:
+             s = getservbyname (pmap_service_name, "udp");
+         or is there something I am missing here? */
       s = getservbyname ("sunrpc", pmap_service_name);
       if (s)
 	pmapport = s->s_port;
@@ -121,17 +129,29 @@ mount_root (char *name, char *host)
     }
 
   addr.sin_family = h->h_addrtype;
-  bcopy (h->h_addr_list[0], &addr.sin_addr, h->h_length);
+  memcpy (&addr.sin_addr, h->h_addr_list[0], h->h_length);
   addr.sin_port = pmapport;
 
-  connect (main_udp_socket,
-	   (struct sockaddr *)&addr, sizeof (struct sockaddr_in));
-
-  if (!mount_port_override)
+  if (mount_port_override)
+    addr.sin_port = htons (mount_port);
+  else
     {
       /* Formulate and send a PMAPPROC_GETPORT request
 	 to lookup the mount program on the server.  */
+      if (connect (main_udp_socket, (struct sockaddr *)&addr,
+	           sizeof (struct sockaddr_in)) == -1)
+	{
+	  perror ("server mount program");
+	  return 0;
+	}
+
       p = pmap_initialize_rpc (PMAPPROC_GETPORT, &rpcbuf);
+      if (! p)
+	{
+	  perror ("creating rpc packet");
+	  return 0;
+	}
+
       *p++ = htonl (MOUNTPROG);
       *p++ = htonl (MOUNTVERS);
       *p++ = htonl (IPPROTO_UDP);
@@ -146,38 +166,43 @@ mount_root (char *name, char *host)
 	addr.sin_port = htons (mount_port);
       else
 	{
-	  free (rpcbuf);
 	  perror ("portmap of mount");
-	  return 0;
+	  goto error_with_rpcbuf;
 	}
       free (rpcbuf);
     }
-  else
-    addr.sin_port = htons (mount_port);
-
 
-  /* Now talking to the mount program, fetch the file handle
+  /* Now, talking to the mount program, fetch a file handle
      for the root. */
-  connect (main_udp_socket,
-	   (struct sockaddr *) &addr, sizeof (struct sockaddr_in));
+  if (connect (main_udp_socket, (struct sockaddr *) &addr,
+	       sizeof (struct sockaddr_in)) == -1)
+    {
+      perror ("connect");
+      goto error_with_rpcbuf;
+    }
+
   p = mount_initialize_rpc (MOUNTPROC_MNT, &rpcbuf);
+  if (! p)
+    {
+      perror ("rpc");
+      goto error_with_rpcbuf;
+    }
+
   p = xdr_encode_string (p, name);
   errno = conduct_rpc (&rpcbuf, &p);
   if (errno)
     {
-      free (rpcbuf);
       perror (name);
-      return 0;
+      goto error_with_rpcbuf;
     }
   /* XXX Protocol spec says this should be a "unix error code"; we'll
-     pretend that an NFS error code is what's meant, the numbers match
+     pretend that an NFS error code is what's meant; the numbers match
      anyhow.  */
   errno = nfs_error_trans (htonl (*p++));
   if (errno)
     {
-      free (rpcbuf);
       perror (name);
-      return 0;
+      goto error_with_rpcbuf;
     }
 
   /* Create the node for root */
@@ -185,13 +210,25 @@ mount_root (char *name, char *host)
   free (rpcbuf);
   mutex_unlock (&np->lock);
 
-  if (!nfs_port_override)
+  if (nfs_port_override)
+    port = nfs_port;
+  else
     {
-      /* Now send another PMAPPROC_GETPORT request to lookup the nfs server. */
+      /* Send another PMAPPROC_GETPORT request to lookup the nfs server. */
       addr.sin_port = pmapport;
-      connect (main_udp_socket,
-	       (struct sockaddr *) &addr, sizeof (struct sockaddr_in));
+      if (connect (main_udp_socket, (struct sockaddr *) &addr,
+	           sizeof (struct sockaddr_in)) == -1)
+	{
+	  perror ("connect");
+	  return 0;
+	}
+
       p = pmap_initialize_rpc (PMAPPROC_GETPORT, &rpcbuf);
+      if (! p)
+	{
+	  perror ("rpc");
+	  goto error_with_rpcbuf;
+	}
       *p++ = htonl (NFS_PROGRAM);
       *p++ = htonl (NFS_VERSION);
       *p++ = htonl (IPPROTO_UDP);
@@ -203,18 +240,24 @@ mount_root (char *name, char *host)
 	port = nfs_port;
       else
 	{
-	  free (rpcbuf);
 	  perror ("portmap of nfs server");
-	  return 0;
+	  goto error_with_rpcbuf;
 	}
       free (rpcbuf);
     }
-  else
-    port = nfs_port;
 
   addr.sin_port = htons (port);
-  connect (main_udp_socket,
-	   (struct sockaddr *) &addr, sizeof (struct sockaddr_in));
+  if (connect (main_udp_socket, (struct sockaddr *) &addr,
+	       sizeof (struct sockaddr_in)) == -1)
+    {
+      perror ("connect");
+      return 0;
+    }
 
   return np;
+
+error_with_rpcbuf:
+  free (rpcbuf);
+
+  return 0;
 }
diff --git a/nfs/mount.h b/nfs/mount.h
index fce8ee4c..4cb62a83 100644
--- a/nfs/mount.h
+++ b/nfs/mount.h
@@ -20,6 +20,9 @@
 
 /* These constants define the RPC mount protocol; see RFC 1094. */
 
+#ifndef NFS_MOUNT_H
+#define NFS_MOUNT_H
+
 #define MOUNTPROG 100005
 #define MOUNTVERS 1
 
@@ -33,3 +36,5 @@
 #define MOUNTPROC_UMNT 3
 #define MOUNTPROC_UMNTALL 4
 #define MOUNTPROC_EXPORT 5
+
+#endif /* NFS_MOUNT_H */
diff --git a/nfs/name-cache.c b/nfs/name-cache.c
index bc5babe6..845cda30 100644
--- a/nfs/name-cache.c
+++ b/nfs/name-cache.c
@@ -24,7 +24,7 @@
 #include <cacheq.h>
 
 
-/* Maximum number of names to cache at once */
+/* Maximum number of names to cache at any given time */
 #define MAXCACHE 200
 
 /* Maximum length of file name we bother caching */
@@ -73,7 +73,7 @@ static struct stats
 } statistics;
 
 #define PARTIAL_THRESH 100
-#define NPARTIALS MAXCACHE / PARTIAL_THRESH
+#define NPARTIALS (MAXCACHE / PARTIAL_THRESH)
 struct stats partial_stats [NPARTIALS];
 
 
@@ -93,19 +93,19 @@ find_cache (char *dir, size_t len, const char *name, size_t name_len)
     if (c->name_len == name_len
 	&& c->dir_cache_len == len
 	&& c->name[0] == name[0] 
-	&& bcmp (c->dir_cache_fh, dir, len) == 0
+	&& memcmp (c->dir_cache_fh, dir, len) == 0
 	&& strcmp (c->name, name) == 0)
       {
-	c->stati = i / 100;
+	c->stati = i / PARTIAL_THRESH;
 	return c;
       }
   
   return 0;
 }
 
-/* Node NP has just been found in DIR with NAME.  If NP is null, that
-   means that this name has been confirmed as absent in the directory.
-   DIR is the fhandle of the directory; its length is LEN.  */
+/* Node NP has just been found in DIR with NAME.  If NP is null, this
+   name has been confirmed as absent in the directory.  DIR is the
+   fhandle of the directory and LEN is its length.  */
 void
 enter_lookup_cache (char *dir, size_t len, struct node *np, char *name)
 {
@@ -127,7 +127,7 @@ enter_lookup_cache (char *dir, size_t len, struct node *np, char *name)
   c = find_cache (dir, len, name, name_len) ?: lookup_cache.lru;
 
   /* Fill C with the new entry.  */
-  bcopy (dir, c->dir_cache_fh, len);
+  memcpy (c->dir_cache_fh, dir, len);
   c->dir_cache_len = len;
   if (c->np)
     netfs_nrele (c->np);
@@ -158,7 +158,8 @@ purge_lookup_cache (struct node *dp, char *name, size_t namelen)
 
       if (c->name_len == namelen
 	  && c->dir_cache_len == dp->nn->handle.size
-	  && bcmp (c->dir_cache_fh, dp->nn->handle.data, c->dir_cache_len) == 0
+	  && memcmp (c->dir_cache_fh, dp->nn->handle.data,
+	             c->dir_cache_len) == 0
 	  && strcmp (c->name, name) == 0)
 	{
 	  if (c->np)
@@ -237,11 +238,12 @@ register_miss ()
 
 
 
-/* Scan the cache looking for NAME inside DIR.  If we don't know
-   anything entry at all, then return 0.  If the entry is confirmed to
-   not exist, then return -1.  Otherwise, return NP for the entry, with
-   a newly allocated reference.  For any return value but 0, unlock
-   DP before returning.  */
+/* Scan the cache looking for NAME inside DIR.  If we know nothing
+   about the entry, then return 0.  If the entry is confirmed to not
+   exist, then return -1.  Otherwise, return NP for the entry, with
+   a newly allocated reference.  For all return values other than 0,
+   unlock DIR->LOCK before returning.  For positive hits, lock the
+   returned node. */
 struct node *
 check_lookup_cache (struct node *dir, char *name)
 {
diff --git a/nfs/nfs-spec.h b/nfs/nfs-spec.h
index 32f5b09d..bed03874 100644
--- a/nfs/nfs-spec.h
+++ b/nfs/nfs-spec.h
@@ -1,3 +1,6 @@
+#ifndef NFS_NFS_SPEC_H
+#define NFS_NFS_SPEC_H
+
 #define NFS_PORT 2049
 #define NFS_MAXDATA 8192
 #define NFS_MAXPATHLEN 1024
@@ -162,3 +165,4 @@ enum createmode
 #define NFS3PROC_PATHCONF 20
 #define NFS3PROC_COMMIT 21
 
+#endif /* NFS_NFS_SPEC_H */
diff --git a/nfs/nfs.c b/nfs/nfs.c
index 46c95c22..27a3a4fe 100644
--- a/nfs/nfs.c
+++ b/nfs/nfs.c
@@ -154,7 +154,7 @@ xdr_encode_data (int *p, char *data, size_t len)
 
   p[nints] = 0;
   *p++ = htonl (len);
-  bcopy (data, p, len);
+  memcpy (p, data, len);
   return p + nints;
 }
 
@@ -379,6 +379,7 @@ xdr_decode_fhandle (int *p, struct node **npp)
   size_t len;
 
   len = protocol_version == 2 ? NFS2_FHSIZE : ntohl (*p++);
+  /* Enter into cache */
   lookup_fhandle (p, len, npp);
   return p + len / sizeof (int);
 }
diff --git a/nfs/nfs.h b/nfs/nfs.h
index 2c32ff9b..5fb62c50 100644
--- a/nfs/nfs.h
+++ b/nfs/nfs.h
@@ -15,6 +15,9 @@
    along with this program; if not, write to the Free Software
    Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. */
 
+#ifndef NFS_NFS_H
+#define NFS_NFS_H
+
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/mman.h>
@@ -26,11 +29,11 @@ struct fhandle
 {
   size_t size;
 
-  /* Leave enough room for the biggest possible fhandle. */
+  /* Leave enough room for the largest possible fhandle. */
   char data[NFS3_FHSIZE];
 };
 
-/* One of these exists for private data needed by the client for each
+/* There exists one of there for the private data needed by each client
    node. */
 struct netnode
 {
@@ -66,8 +69,8 @@ struct netnode
   struct user_pager_info *fileinfo;
 
   /* If this node has been renamed by "deletion" then
-     this is the directory and name in that directory which
-     is holding the node */
+     this is the directory and the name in that directory
+     which is holding the node */
   struct node *dead_dir;
   char *dead_name;
 };
@@ -150,7 +153,7 @@ extern int nfs_port_override;
 extern int protocol_version;
 
 
-/* Count how many four-byte chunks it takss to hold LEN bytes. */
+/* Count how many four-byte chunks it takes to hold LEN bytes. */
 #define INTSIZE(len) (((len)+3)>>2)
 
 
@@ -193,3 +196,5 @@ void enter_lookup_cache (char *, size_t, struct node *, char *);
 void purge_lookup_cache (struct node *, char *, size_t);
 struct node *check_lookup_cache (struct node *, char *);
 void purge_lookup_cache_node (struct node *);
+
+#endif /* NFS_NFS_H */
diff --git a/nfs/ops.c b/nfs/ops.c
index cca39996..8017abcb 100644
--- a/nfs/ops.c
+++ b/nfs/ops.c
@@ -25,9 +25,9 @@
 #include <unistd.h>
 #include <maptime.h>
 
-/* We have fresh stat information for NP; the fattr structure is at
-   P.  Update our entry.  Return the address of the next int after
-   the fattr structure.  */
+/* We have fresh stat information for NP; the file attribute (fattr)
+   structure is at P.  Update our entry.  Return the address of the next
+   int after the fattr structure.  */
 int *
 register_fresh_stat (struct node *np, int *p)
 {
@@ -83,12 +83,12 @@ register_fresh_stat (struct node *np, int *p)
 int *
 process_returned_stat (struct node *np, int *p, int mod)
 {
-  int attrs_exist;
-
   if (protocol_version == 2)
     return register_fresh_stat (np, p);
   else
     {
+      int attrs_exist;
+      
       attrs_exist = ntohl (*p++);
       if (attrs_exist)
 	p = register_fresh_stat (np, p);
@@ -144,6 +144,9 @@ netfs_validate_stat (struct node *np, struct iouser *cred)
 
   p = nfs_initialize_rpc (NFSPROC_GETATTR (protocol_version),
 			  (struct iouser *) -1, 0, &rpcbuf, np, -1);
+  if (! p)
+    return errno;
+
   p = xdr_encode_fhandle (p, &np->nn->handle);
 
   err = conduct_rpc (&rpcbuf, &p);
@@ -168,6 +171,9 @@ netfs_attempt_chown (struct iouser *cred, struct node *np,
 
   p = nfs_initialize_rpc (NFSPROC_SETATTR (protocol_version),
 			  cred, 0, &rpcbuf, np, gid);
+  if (! p)
+    return errno;
+
   p = xdr_encode_fhandle (p, &np->nn->handle);
   p = xdr_encode_sattr_ids (p, uid, gid);
   if (protocol_version == 3)
@@ -210,6 +216,8 @@ netfs_attempt_chmod (struct iouser *cred, struct node *np,
       err = netfs_validate_stat (np, cred);
       if (err)
 	return err;
+
+      /* Has the file type changed? (e.g. from symlink to directory) */
       if ((mode & S_IFMT) != (np->nn_stat.st_mode & S_IFMT))
 	{
 	  char *f = 0;
@@ -242,6 +250,9 @@ netfs_attempt_chmod (struct iouser *cred, struct node *np,
 
   p = nfs_initialize_rpc (NFSPROC_SETATTR (protocol_version),
 			  cred, 0, &rpcbuf, np, -1);
+  if (! p)
+    return errno;
+
   p = xdr_encode_fhandle (p, &np->nn->handle);
   p = xdr_encode_sattr_mode (p, mode);
   if (protocol_version == 3)
@@ -291,6 +302,9 @@ netfs_attempt_utimes (struct iouser *cred, struct node *np,
 
   p = nfs_initialize_rpc (NFSPROC_SETATTR (protocol_version),
 			  cred, 0, &rpcbuf, np, -1);
+  if (! p)
+    return errno;
+
   p = xdr_encode_fhandle (p, &np->nn->handle);
   p = xdr_encode_sattr_times (p, 
 			      atime ?: &current, 
@@ -322,6 +336,9 @@ netfs_attempt_set_size (struct iouser *cred, struct node *np,
 
   p = nfs_initialize_rpc (NFSPROC_SETATTR (protocol_version),
 			  cred, 0, &rpcbuf, np, -1);
+  if (! p)
+    return errno;
+
   p = xdr_encode_fhandle (p, &np->nn->handle);
   p = xdr_encode_sattr_size (p, size);
   if (protocol_version == 3)
@@ -344,12 +361,9 @@ netfs_attempt_set_size (struct iouser *cred, struct node *np,
      does have the file open for writing.  */
   if (err == EACCES)
     {
-      err = netfs_validate_stat (np, cred);
-      if (!err && np->nn_stat.st_size == size)
+      int error = netfs_validate_stat (np, cred);
+      if (!error && np->nn_stat.st_size == size)
 	err = 0;
-      else
-	/* Never mind, put the old error back */
-	err = EACCES;
     }
 
   free (rpcbuf);
@@ -367,6 +381,9 @@ netfs_attempt_statfs (struct iouser *cred, struct node *np,
   error_t err;
 
   p = nfs_initialize_rpc (NFS2PROC_STATFS, cred, 0, &rpcbuf, np, -1);
+  if (! p)
+    return errno;
+
   p = xdr_encode_fhandle (p, &np->nn->handle);
 
   err = conduct_rpc (&rpcbuf, &p);
@@ -429,6 +446,9 @@ netfs_attempt_read (struct iouser *cred, struct node *np,
 
       p = nfs_initialize_rpc (NFSPROC_READ (protocol_version),
 			      cred, 0, &rpcbuf, np, -1);
+      if (! p)
+        return errno;
+
       p = xdr_encode_fhandle (p, &np->nn->handle);
       *p++ = htonl (offset);
       *p++ = htonl (thisamt);
@@ -458,7 +478,7 @@ netfs_attempt_read (struct iouser *cred, struct node *np,
 	  else
 	    eof = (trans_len < thisamt);
 
-	  bcopy (p, data, trans_len);
+	  memcpy (data, p, trans_len);
 	  free (rpcbuf);
 
 	  data += trans_len;
@@ -495,6 +515,9 @@ netfs_attempt_write (struct iouser *cred, struct node *np,
 
       p = nfs_initialize_rpc (NFSPROC_WRITE (protocol_version),
 			      cred, thisamt, &rpcbuf, np, -1);
+      if (! p)
+        return errno;
+
       p = xdr_encode_fhandle (p, &np->nn->handle);
       if (protocol_version == 2)
 	*p++ = 0;
@@ -524,24 +547,23 @@ netfs_attempt_write (struct iouser *cred, struct node *np,
 		/* assume it wrote the whole thing */
 		count = thisamt;
 
-	      free (rpcbuf);
 	      amt -= count;
 	      data += count;
 	      offset += count;
 	    }
 	}
 
+      free (rpcbuf);
+
       if (err == EINTR && amt != *len)
 	{
 	  *len -= amt;
-	  free (rpcbuf);
 	  return 0;
 	}
 
       if (err)
 	{
 	  *len = 0;
-	  free (rpcbuf);
 	  return err;
 	}
     }
@@ -564,6 +586,9 @@ verify_nonexistent (struct iouser *cred, struct node *dir,
 
   p = nfs_initialize_rpc (NFSPROC_LOOKUP (protocol_version),
 			  cred, 0, &rpcbuf, dir, -1);
+  if (! p)
+    return errno;
+
   p = xdr_encode_fhandle (p, &dir->nn->handle);
   p = xdr_encode_string (p, name);
 
@@ -571,6 +596,8 @@ verify_nonexistent (struct iouser *cred, struct node *dir,
   if (!err)
     err = nfs_error_trans (ntohl (*p++));
 
+  free (rpcbuf);
+
   if (!err)
     return EEXIST;
   else
@@ -604,13 +631,16 @@ netfs_attempt_lookup (struct iouser *cred, struct node *np,
 
   p = nfs_initialize_rpc (NFSPROC_LOOKUP (protocol_version),
 			  cred, 0, &rpcbuf, np, -1);
+  if (! p)
+    return errno;
+
   p = xdr_encode_fhandle (p, &np->nn->handle);
   p = xdr_encode_string (p, name);
 
   /* Remember the directory handle for later cache use. */
 
   dirlen = np->nn->handle.size;
-  bcopy (np->nn->handle.data, dirhandle, dirlen);
+  memcpy (dirhandle, np->nn->handle.data, dirlen);
 
   mutex_unlock (&np->lock);
 
@@ -630,7 +660,7 @@ netfs_attempt_lookup (struct iouser *cred, struct node *np,
 	  if (*newnp)
 	    mutex_unlock (&(*newnp)->lock);
 	  mutex_lock (&np->lock);
-	  p = process_returned_stat (np, p, 0);
+	  p = process_returned_stat (np, p, 0); /* XXX Do we have to lock np? */
 	  mutex_unlock (&np->lock);
 	  if (*newnp)
 	    mutex_lock (&(*newnp)->lock);
@@ -672,6 +702,9 @@ netfs_attempt_mkdir (struct iouser *cred, struct node *np,
 
   p = nfs_initialize_rpc (NFSPROC_MKDIR (protocol_version),
 			  cred, 0, &rpcbuf, np, -1);
+  if (! p)
+    return errno;
+
   p = xdr_encode_fhandle (p, &np->nn->handle);
   p = xdr_encode_string (p, name);
   p = xdr_encode_create_state (p, mode, owner);
@@ -680,17 +713,20 @@ netfs_attempt_mkdir (struct iouser *cred, struct node *np,
   if (!err)
     err = nfs_error_trans (ntohl (*p++));
 
-  p = xdr_decode_fhandle (p, &newnp);
-  p = process_returned_stat (newnp, p, 1);
+  if (!err)
+    {
+      p = xdr_decode_fhandle (p, &newnp);
+      p = process_returned_stat (newnp, p, 1);
 
-  /* Did we set the owner correctly?  If not, try, but ignore failures. */
-  if (!netfs_validate_stat (newnp, (struct iouser *) -1)
-      && newnp->nn_stat.st_uid != owner)
-    netfs_attempt_chown ((struct iouser *) -1, newnp, owner,
-			 newnp->nn_stat.st_gid);
+      /* Did we set the owner correctly?  If not, try, but ignore failures. */
+      if (!netfs_validate_stat (newnp, (struct iouser *) -1)
+          && newnp->nn_stat.st_uid != owner)
+        netfs_attempt_chown ((struct iouser *) -1, newnp, owner,
+			     newnp->nn_stat.st_gid);
 
-  /* We don't actually return this. */
-  netfs_nput (newnp);
+      /* We don't actually return this. */
+      netfs_nput (newnp);
+    }
 
   free (rpcbuf);
   return err;
@@ -712,6 +748,9 @@ netfs_attempt_rmdir (struct iouser *cred, struct node *np,
 
   p = nfs_initialize_rpc (NFSPROC_RMDIR (protocol_version),
 			  cred, 0, &rpcbuf, np, -1);
+  if (! p)
+    return errno;
+
   p = xdr_encode_fhandle (p, &np->nn->handle);
   p = xdr_encode_string (p, name);
 
@@ -758,6 +797,12 @@ netfs_attempt_link (struct iouser *cred, struct node *dir,
       mutex_lock (&dir->lock);
       p = nfs_initialize_rpc (NFSPROC_LINK (protocol_version),
 			      cred, 0, &rpcbuf, dir, -1);
+      if (! p)
+	{
+          mutex_unlock (&dir->lock);
+          return errno;
+	}
+
       mutex_unlock (&dir->lock);
 
       mutex_lock (&np->lock);
@@ -783,6 +828,12 @@ netfs_attempt_link (struct iouser *cred, struct node *dir,
       mutex_lock (&dir->lock);
       p = nfs_initialize_rpc (NFSPROC_SYMLINK (protocol_version),
 			      cred, 0, &rpcbuf, dir, -1);
+      if (! p)
+	{
+          mutex_unlock (&dir->lock);
+          return errno;
+	}
+
       p = xdr_encode_fhandle (p, &dir->nn->handle);
       mutex_unlock (&dir->lock);
 
@@ -825,6 +876,11 @@ netfs_attempt_link (struct iouser *cred, struct node *dir,
 		 fhandle, so we have to fetch one now. */
 	      p = nfs_initialize_rpc (NFSPROC_LOOKUP (protocol_version),
 				      cred, 0, &rpcbuf, dir, -1);
+	      if (! p)
+		{
+		  mutex_unlock (&dir->lock);
+		  return errno;
+		}
 	      p = xdr_encode_fhandle (p, &dir->nn->handle);
 	      p = xdr_encode_string (p, name);
 
@@ -880,6 +936,12 @@ netfs_attempt_link (struct iouser *cred, struct node *dir,
 
 	  p = nfs_initialize_rpc (NFSPROC_CREATE (protocol_version),
 				  cred, 0, &rpcbuf, dir, -1);
+          if (! p)
+	    {
+	      mutex_unlock (&dir->lock);
+              return errno;
+            }
+
 	  p = xdr_encode_fhandle (p, &dir->nn->handle);
 	  p = xdr_encode_string (p, name);
 	  mutex_unlock (&dir->lock);
@@ -898,21 +960,31 @@ netfs_attempt_link (struct iouser *cred, struct node *dir,
 
 	  mutex_lock (&dir->lock);
 	  purge_lookup_cache (dir, name, strlen (name));
+	  mutex_unlock (&dir->lock); /* XXX Should this really be after the
+					_lengthy_ (blocking) conduct_rpc? */
 	  err = conduct_rpc (&rpcbuf, &p);
 	  if (!err)
 	    err = nfs_error_trans (ntohl (*p++));
-	  mutex_unlock (&dir->lock);
 
-	  mutex_lock (&np->lock);
-	  p = recache_handle (p, np);
-	  register_fresh_stat (np, p);
-	  mutex_unlock (&np->lock);
+	  if (!err)
+	    {
+	      mutex_lock (&np->lock);
+	      p = recache_handle (p, np);
+	      register_fresh_stat (np, p);
+	      mutex_unlock (&np->lock);
+	    }
+
 	  free (rpcbuf);
 	}
-      else
+      else /* protocol_version != 2 */
 	{
 	  mutex_lock (&dir->lock);
 	  p = nfs_initialize_rpc (NFS3PROC_MKNOD, cred, 0, &rpcbuf, dir, -1);
+	  if (! p)
+	    {
+	      mutex_unlock (&dir->lock);
+	      return errno;
+	    }
 	  p = xdr_encode_fhandle (p, &dir->nn->handle);
 	  p = xdr_encode_string (p, name);
 	  mutex_unlock (&dir->lock);
@@ -996,13 +1068,17 @@ netfs_attempt_mkfile (struct iouser *cred, struct node *dir,
   /* This is the best we can do. */
 
   name = malloc (50);
+  if (! name)
+    return ENOMEM;
 
   do
     {
       sprintf (name, ".nfstmpgnu.%d", n++);
       err = netfs_attempt_create_file (cred, dir, name, mode, newnp);
       if (err == EEXIST)
-	mutex_lock (&dir->lock);
+	mutex_lock (&dir->lock);  /* XXX is this right? does create need this
+				     and drop this on error? Doesn't look
+				     like it. */
     }
   while (err == EEXIST);
 
@@ -1059,6 +1135,9 @@ netfs_attempt_create_file (struct iouser *cred, struct node *np,
 
   p = nfs_initialize_rpc (NFSPROC_CREATE (protocol_version),
 			  cred, 0, &rpcbuf, np, -1);
+  if (! p)
+    return errno;
+
   p = xdr_encode_fhandle (p, &np->nn->handle);
   p = xdr_encode_string (p, name);
   if (protocol_version == 3)
@@ -1149,6 +1228,13 @@ netfs_attempt_unlink (struct iouser *cred, struct node *dir,
       mutex_unlock (&dir->lock);
 
       newname = malloc (50);
+      if (! newname)
+	{
+	  mutex_lock (&dir->lock);
+	  netfs_nrele (np);         /* XXX Is this the correct thing to do? */
+	  return ENOMEM;
+	}
+
       do
 	{
 	  sprintf (newname, ".nfs%xgnu.%d", (int) np, n++);
@@ -1186,6 +1272,9 @@ netfs_attempt_unlink (struct iouser *cred, struct node *dir,
 
   p = nfs_initialize_rpc (NFSPROC_REMOVE (protocol_version),
 			  cred, 0, &rpcbuf, dir, -1);
+  if (! p)
+    return errno;
+
   p = xdr_encode_fhandle (p, &dir->nn->handle);
   p = xdr_encode_string (p, name);
 
@@ -1250,6 +1339,12 @@ netfs_attempt_rename (struct iouser *cred, struct node *fromdir,
   purge_lookup_cache (fromdir, fromname, strlen (fromname));
   p = nfs_initialize_rpc (NFSPROC_RENAME (protocol_version),
 			  cred, 0, &rpcbuf, fromdir, -1);
+  if (! p)
+    {
+      mutex_unlock (&fromdir->lock);
+      return errno;
+    }
+
   p = xdr_encode_fhandle (p, &fromdir->nn->handle);
   p = xdr_encode_string (p, fromname);
   mutex_unlock (&fromdir->lock);
@@ -1264,7 +1359,7 @@ netfs_attempt_rename (struct iouser *cred, struct node *fromdir,
   if (!err)
     {
       err = nfs_error_trans (ntohl (*p++));
-      if (protocol_version == 3)
+      if (protocol_version == 3)  /* XXX Should we add `&& !err' ? */
 	{
 	  mutex_lock (&fromdir->lock);
 	  p = process_wcc_stat (fromdir, p, !err);
@@ -1294,6 +1389,9 @@ netfs_attempt_readlink (struct iouser *cred, struct node *np,
 
   p = nfs_initialize_rpc (NFSPROC_READLINK (protocol_version),
 			  cred, 0, &rpcbuf, np, -1);
+  if (! p)
+    return errno;
+
   p = xdr_encode_fhandle (p, &np->nn->handle);
 
   err = conduct_rpc (&rpcbuf, &p);
@@ -1343,7 +1441,7 @@ netfs_report_access (struct iouser *cred,
 
   if (protocol_version == 2)
     {
-      /* Hope the server means the same thing be the bits as we do. */
+      /* Hope the server means the same thing for the bits as we do. */
       *types = 0;
       if (fshelp_access (&np->nn_stat, S_IREAD, cred) == 0)
 	*types |= O_READ;
@@ -1373,6 +1471,9 @@ netfs_report_access (struct iouser *cred,
 	}
 
       p = nfs_initialize_rpc (NFS3PROC_ACCESS, cred, 0, &rpcbuf, np, -1);
+      if (! p)
+	return errno;
+
       p = xdr_encode_fhandle (p, &np->nn->handle);
       *p++ = htonl (ACCESS3_READ | write_check | execute_check);
 
@@ -1380,7 +1481,9 @@ netfs_report_access (struct iouser *cred,
       if (!err)
 	{
 	  err = nfs_error_trans (ntohl (*p++));
-	  p = process_returned_stat (np, p, 0);
+	  p = process_returned_stat (np, p, 0);   /* XXX Should this be
+						     protected by the
+						     if (!err) ? */
 	  if (!err)
 	    {
 	      ret = ntohl (*p++);
@@ -1554,7 +1657,11 @@ fetch_directory (struct iouser *cred, struct node *dir,
   int isnext;
 
   bufmalloced = read_size;
+
   buf = malloc (bufmalloced);
+  if (! buf)
+    return ENOMEM;
+
   bp = buf;
   cookie = 0;
   eof = 0;
@@ -1565,6 +1672,12 @@ fetch_directory (struct iouser *cred, struct node *dir,
       /* Fetch new directory entries */
       p = nfs_initialize_rpc (NFSPROC_READDIR (protocol_version),
 			      cred, 0, &rpcbuf, dir, -1);
+      if (! p)
+	{
+	  free (buf);
+	  return errno;
+	}
+
       p = xdr_encode_fhandle (p, &dir->nn->handle);
       *p++ = cookie;
       *p++ = ntohl (read_size);
@@ -1600,6 +1713,7 @@ fetch_directory (struct iouser *cred, struct node *dir,
 	      char *newbuf;
 
 	      newbuf = realloc (buf, bufmalloced *= 2);
+	      assert (newbuf);
 	      if (newbuf != buf)
 		bp = newbuf + (bp - buf);
 	      buf = newbuf;
@@ -1681,7 +1795,7 @@ netfs_get_dirents (struct iouser *cred, struct node *np,
 	 && thisentry < totalentries;)
       {
 	struct dirent *entry = (struct dirent *) bp;
-	bcopy (bp, userdp, entry->d_reclen);
+	memcpy (userdp, bp, entry->d_reclen);
 	bp += entry->d_reclen;
 	userdp += entry->d_reclen;
 	entries_copied++;
diff --git a/nfs/rpc.c b/nfs/rpc.c
index 90d534ba..0b9a9d8e 100644
--- a/nfs/rpc.c
+++ b/nfs/rpc.c
@@ -28,7 +28,7 @@
 #include <rpc/rpc_msg.h>
 #include <rpc/auth_unix.h>
 
-#undef malloc			/* Get rid protection.  */
+#undef malloc			/* Get rid of the sun block */
 
 #include <netinet/in.h>
 #include <assert.h>
@@ -43,7 +43,7 @@ struct rpc_list
   void *reply;
 };
 
-/* A list of all the pending RPCs. */
+/* A list of all pending RPCs. */
 static struct rpc_list *outstanding_rpcs;
 
 /* Wake up this condition when an outstanding RPC has received a reply
@@ -56,7 +56,7 @@ static struct mutex outstanding_lock = MUTEX_INITIALIZER;
 
 
 /* Generate and return a new transaction ID. */
-static int
+static inline int
 generate_xid ()
 {
   static int nextxid;
@@ -67,22 +67,31 @@ generate_xid ()
   return nextxid++;
 }
 
-/* Set up an RPC for procdeure RPC_PROC, for talking to the server
+/* Set up an RPC for procdeure RPC_PROC for talking to the server
    PROGRAM of version VERSION.  Allocate storage with malloc and point
    *BUF at it; caller must free this when done.  Allocate at least LEN
-   bytes more than the usual amount for an RPC.  Initialize the RPC
-   credential structure with UID, GID, and SECOND_GID.  (Any of those
-   may be -1 to indicate that it does not apply; exactly or two of UID
-   and GID must be -1, however.) */
+   bytes more than the usual amount for the RPC.  Initialize the RPC
+   credential structure with UID, GID, and SECOND_GID;  any of these
+   may be -1 to indicate that it does not apply, however, exactly zero
+   or two of UID and GID must be -1.  The returned address is a pointer
+   to the start of the payload.  If NULL is returned, an error occured
+   and the code is set in errno. */
 int *
 initialize_rpc (int program, int version, int rpc_proc, 
 		size_t len, void **bufp, 
 		uid_t uid, gid_t gid, gid_t second_gid)
 {
-  void *buf = malloc (len + 1024);
+  void *buf;
   int *p, *lenaddr;
   struct rpc_list *hdr;
 
+  buf = malloc (len + 1024);
+  if (! buf)
+    {
+      errno = ENOMEM;
+      return NULL;
+    }
+
   /* First the struct rpc_list bit. */
   hdr = buf;
   hdr->reply = 0;
@@ -99,40 +108,44 @@ initialize_rpc (int program, int version, int rpc_proc,
   
   assert ((uid == -1) == (gid == -1));
 
-  if (uid != -1)
+  if (uid == -1)
+    {
+      /* No authentication */
+      *p++ = htonl (AUTH_NONE);
+      *p++ = 0;
+    }
+  else
     {
+      /* Unixy authentication */
       *p++ = htonl (AUTH_UNIX);
+      /* The length of the message.  We do not yet know what this
+         is, so, just remember where we should put it when we know */
       lenaddr = p++;
       *p++ = htonl (mapped_time->seconds);
       p = xdr_encode_string (p, hostname);
       *p++ = htonl (uid);
       *p++ = htonl (gid);
-      if (second_gid != -1)
+      if (second_gid == -1)
+	*p++ = 0;
+      else
 	{
 	  *p++ = htonl (1);
 	  *p++ = htonl (second_gid);
 	}
-      else
-	*p++ = 0;
       *lenaddr = htonl ((p - (lenaddr + 1)) * sizeof (int));
     }
-  else
-    {
-      *p++ = htonl (AUTH_NULL);
-      *p++ = 0;
-    }
         
   /* VERF field */
-  *p++ = htonl (AUTH_NULL);
+  *p++ = htonl (AUTH_NONE);
   *p++ = 0;
   
   *bufp = buf;
   return p;
 }
 
-/* Remove HDR from the list of pending RPC's.  rpc_list_lock must be
-   held */
-static void
+/* Remove HDR from the list of pending RPC's.  The rpc_list's lock
+   (OUTSTANDING_LOCK) must be held */
+static inline void
 unlink_rpc (struct rpc_list *hdr)
 {
   *hdr->prevp = hdr->next;
@@ -140,12 +153,24 @@ unlink_rpc (struct rpc_list *hdr)
     hdr->next->prevp = hdr->prevp;
 }
 
+/* Insert HDR at the head of the LIST.  The rpc_list's lock
+   (OUTSTANDING_LOCK) must be held */
+static inline void
+link_rpc (struct rpc_list **list, struct rpc_list *hdr)
+{
+  hdr->next = *list;
+  if (hdr->next)
+    hdr->next->prevp = &hdr->next;
+  hdr->prevp = list;
+  *list = hdr;
+}
+
 /* Send the specified RPC message.  *RPCBUF is the initialized buffer
-   from a previous initialize_rpc call; *PP points past the filled
-   in args.  Set *PP to the address of the reply contents themselves.
-   The user will be expected to free *RPCBUF (which will have changed)
-   when done with the reply contents.  The old value of *RPCBUF will
-   be freed by this routine. */
+   from a previous initialize_rpc call; *PP, the payload, points past
+   the filledin args.  Set *PP to the address of the reply contents
+   themselves.  The user will be expected to free *RPCBUF (which will
+   have changed) when done with the reply contents.  The old value of
+   *RPCBUF will be freed by this routine. */
 error_t
 conduct_rpc (void **rpcbuf, int **pp)
 {
@@ -162,12 +187,7 @@ conduct_rpc (void **rpcbuf, int **pp)
   
   mutex_lock (&outstanding_lock);
 
-  /* Link it in */
-  hdr->next = outstanding_rpcs;
-  if (hdr->next)
-    hdr->next->prevp = &hdr->next;
-  hdr->prevp = &outstanding_rpcs;
-  outstanding_rpcs = hdr;
+  link_rpc (&outstanding_rpcs, hdr);
 
   xid = * (int *) (*rpcbuf + sizeof (struct rpc_list));
 
@@ -209,9 +229,12 @@ conduct_rpc (void **rpcbuf, int **pp)
 	  return EINTR;
 	}
 
+      /* hdr->reply will have been filled in by rpc_receive_thread,
+         if it has been filled in, then the rpc has been fulfilled,
+         otherwise, retransmit and continue to wait */
       if (!hdr->reply)
 	{
-	  timeout *=2;
+	  timeout *= 2;
 	  if (timeout > max_transmit_timeout)
 	    timeout = max_transmit_timeout;
 	}
@@ -224,10 +247,12 @@ conduct_rpc (void **rpcbuf, int **pp)
   *rpcbuf = hdr->reply;
   free (hdr);
 
-  /* Process the reply, dissecting errors.  When we're done, set *PP to
-     the rpc return contents, if there is no error. */
+  /* Process the reply, dissecting errors.  When we're done and if
+     there is no error, set *PP to the rpc return contents */ 
   p = (int *) *rpcbuf;
   
+  /* If the transmition id does not match that in the message,
+     something strange happened in rpc_receive_thread */
   assert (*p == xid);
   p++;
   
@@ -315,7 +340,8 @@ conduct_rpc (void **rpcbuf, int **pp)
   return err;
 }
 
-/* Dedicated thread to wakeup rpc_wakeup once a second. */
+/* Dedicated thread to signal those waiting on rpc_wakeup
+   once a second. */
 void
 timeout_service_thread ()
 {
@@ -333,48 +359,52 @@ timeout_service_thread ()
 void
 rpc_receive_thread ()
 {
-  int cc;
   void *buf;
-  struct rpc_list *r;
-  int xid;
+
+  /* Allocate a receive buffer */
+  buf = malloc (1024 + read_size);
+  assert (buf);
 
   while (1)
     {
-      buf = malloc (1024 + read_size);
-
-      do
-	{
-	  cc = read (main_udp_socket, buf, 1024 + read_size);
-	  if (cc == -1)
-	    {
-	      perror ("nfs read");
-	      r = 0;
+      int cc = read (main_udp_socket, buf, 1024 + read_size);
+      if (cc == -1)
+        {
+          perror ("nfs read");
+          continue;
+        }
+      else
+        {
+          struct rpc_list *r;
+          int xid = *(int *)buf;
+
+          mutex_lock (&outstanding_lock);
+
+          /* Find the rpc that we just fulfilled */
+          for (r = outstanding_rpcs; r; r = r->next)
+    	    {
+    	      if (* (int *) &r[1] == xid)
+	        {
+	          unlink_rpc (r);
+	          r->reply = buf;
+	          condition_broadcast (&rpc_wakeup);
+		  break;
+	        }
 	    }
-	  else
-	    {
-	      xid = *(int *)buf;
-	      mutex_lock (&outstanding_lock);
-	      for (r = outstanding_rpcs; r; r = r->next)
-		{
-		  if (* (int *) &r[1] == xid)
-		    {
-		      /* Remove it from the list */
-		      *r->prevp = r->next;
-		      if (r->next)
-			r->next->prevp = r->prevp;
-
-		      r->reply = buf;
-		      condition_broadcast (&rpc_wakeup);
-		      break;
-		    }
-		}
-#if notanymore
-	      if (!r)
-		fprintf (stderr, "NFS dropping reply xid %d\n", xid);
+#if 0
+	  if (! r)
+	    fprintf (stderr, "NFS dropping reply xid %d\n", xid);
 #endif
-	      mutex_unlock (&outstanding_lock);
+	  mutex_unlock (&outstanding_lock);
+
+	  /* If r is not null then we had a message from a pending
+	     (i.e. known) rpc.  Thus, it was fulfilled and if we
+	     want to get another request, a new buffer is needed */
+	  if (r)
+	    {
+              buf = malloc (1024 + read_size);
+              assert (buf);
 	    }
-	}
-      while (!r);
+        }
     }
 }