[OpenAFS-devel] ConstructLocalPath fix for review

[Russ Allbery]

ConstructLocalPath in util/dirpath.c appears to currently be broken when
applied to relative paths due to deficient logic in LocalizePathHead.
Observe (Debian bos against Debian bosserver, where Debian's bosserver is
configured to store logs in /var/log/openafs):

(root) windlord:~> bos getlog afssvr40 BosLog
Fetching log file 'BosLog'...
bos: no such entity (while reading log)

(root) windlord:~> bos getlog afssvr40 /var/log/openafs/BosLog
Fetching log file '/var/log/openafs/BosLog'...
Mon Nov  6 10:58:34 2006: Server directory access is okay

(root) windlord:~> bos getlog afssvr40 /usr/afs/logs/BosLog
Fetching log file '/usr/afs/logs/BosLog'...
Mon Nov  6 10:58:34 2006: Server directory access is okay

In the first case, strace on the server shows that the bosserver is trying
to open /usr/afs/logs.

ConstructLocalLogPath calls ConstructLocalPath saying that the provided
path should be considered relative to the canonical wireformat log path
(/usr/afs/logs), and LocalizePathHead does the appropriate adjustment if
the provided path actually starts with that prefix.  However, if it
doesn't, LocalizePathHead does nothing, and ConstructLocalPath then
creates a path relative to the canonical wireformat path rather than the
local path.

Here's a patch.  Does this look right?

(I may also do s/current/map/ on the variable names to keep the lines
shorter than 80 columns.)

=== dirpath.c
==================================================================
--- dirpath.c	(revision 2304)
+++ dirpath.c	(local)
@@ -420,17 +420,25 @@
 /*
  * LocalizePathHead() -- Make path relative to local part
  *
- * ConstructLocalPath takes a path  and a directory that path should
- * be considered relative to.   This  function checks the given path
- * for   a prefix  that represents a canonical path.  If such a prefix
- * is found,  the path is adjusted to remove the prefix and the path
- * is considered  relative to the local version of that path.
+ * ConstructLocalPath takes a path and a directory that path should
+ * be considered relative to.  There are two possible cases:
+ *
+ * The path is an absolute path.  In this case, the relative path
+ * is ignored.  We check the path for a prefix that represents a
+ * canonical path, and if one is found, we adjust the path to remove
+ * the prefix and adjust the directory to which it should be
+ * considered relative to be the local version of that canonical path.
+ *
+ * The path is a relative path.  In this case, we check to see if the
+ * directory to which it is relative represents a canonical path, and
+ * if so, we adjust that directory to be the local version of that
+ * canonical path.  The relative path itself is left unchanged.
  */
 
 /* The following array  maps cannonical parts to local parts.  It
  * might  seem reasonable to  simply construct an array in parallel to
  * dirpatharray  but it turns out you don't want translations for all
- local paths.
+ * local paths.
 */
 
 struct canonmapping {
@@ -449,15 +457,25 @@
 LocalizePathHead(const char **path, const char **relativeTo)
 {
     struct canonmapping *current;
-    for (current = CanonicalTranslations; current->local != NULL; current++) {
-	int canonlength = strlen(current->canonical);
-	if (strncmp(*path, current->canonical, canonlength) == 0) {
-	    (*path) += canonlength;
-	    if (**path == '/')
-		(*path)++;
-	    *relativeTo = current->local;
-	    return;
+
+    if (**path == '/') {
+	for (current = CanonicalTranslations; current->local != NULL; current++) {
+	    int canonlength = strlen(current->canonical);
+	    if (strncmp(*path, current->canonical, canonlength) == 0) {
+		(*path) += canonlength;
+		if (**path == '/')
+		    (*path)++;
+		*relativeTo = current->local;
+		return;
+	    }
 	}
+    } else {
+	for (current = CanonicalTranslations; current->local != NULL; current++) {
+	    if (strcmp(*relativeTo, current->canonical) == 0) {
+		*relativeTo = current->local;
+		return;
+	    }
+	}
     }
 }
 


-- 
Russ Allbery (rra@stanford.edu)             <http://www.eyrie.org/~eagle/>