Clean up 'devgen()' a bit.
authorDan Cross <crossd@gmail.com>
Fri, 8 Jan 2016 19:42:01 +0000 (14:42 -0500)
committerBarret Rhoden <brho@cs.berkeley.edu>
Wed, 13 Jan 2016 17:58:04 +0000 (12:58 -0500)
Try to make this a tad cleaner and clearer, and be a bit more
defensive in the implementation. In particular, we try to check
the validity of table the table index and associated entry before
dereferencing, thus ensuring the function's preconditions hold.

Clean up the comments a bit, to make it less ambiguous whether
they refer to '.' or '..'. Also fix a (presumed?) grammar error.

Signed-off-by: Dan Cross <crossd@gmail.com>
Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
kern/src/ns/dev.c

index 8b23bbd..d3dd55d 100644 (file)
@@ -85,28 +85,35 @@ devdir(struct chan *c, struct qid qid, char *n,
 }
 
 /*
- * the zeroth element of the table MUST be the directory itself for ..
- * Any entry with qid vers of -1 will return 0, indicating that the value is
- * valid but there is nothing there continue walk.
- * TODO(gvdl): Update akaros devgen man page.
-*/
+ * The zeroth element of the table MUST be the directory itself, or '.' (dot),
+ * for processing '..' (dot-dot). Specifically, if i==DEVDOTDOT, we call devdir
+ * on the *directory* (that is, dot), as opposed to children of the directory.
+ * The rest of the system assumes that the first entry in the table refers to
+ * the directory, and by convention this is named '.' (dot). This is confusing.
+ *
+ * Any entry with qid verion of -1 will return 0, indicating that the value is
+ * valid but there is nothing there, so continue walking.
+ *
+ * TODO(cross): Document devgen and clean this mess up. Devgen should probably
+ * be removed and replaced with a smarter data structure.
+ */
 int
-devgen(struct chan *c, char *unused_char_p_t, struct dirtab *tab, int ntab,
-          int i, struct dir *dp)
+devgen(struct chan *c, char *unused_name, struct dirtab *tab, int ntab,
+       int i, struct dir *dp)
 {
-       if (tab == 0)
+       if (tab == NULL)
                return -1;
        if (i != DEVDOTDOT) {
-               /* skip over the first element, that for . itself */
+               /* Skip over the first element, that for the directory itself. */
                i++;
-               if (i >= ntab)
+               if (i < 0 || ntab <= i)
                        return -1;
                tab += i;
        }
-       int ret = (tab->qid.vers == -1)? 0 : 1;
-       if (ret)
-               devdir(c, tab->qid, tab->name, tab->length, eve, tab->perm, dp);
-       return ret;
+       if (tab->qid.vers == -1)
+               return 0;
+       devdir(c, tab->qid, tab->name, tab->length, eve, tab->perm, dp);
+       return 1;
 }
 
 void devreset(void)