LINUX: Rearrange afs_linux_lookup cleanup 36/12636/3
authorAndrew Deason <adeason@sinenomine.net>
Thu, 15 Jun 2017 20:29:48 +0000 (15:29 -0500)
committerBenjamin Kaduk <kaduk@mit.edu>
Wed, 5 Jul 2017 15:59:08 +0000 (11:59 -0400)
Currently, the cleanup and error handling in afs_linux_lookup is
structured similar to this pseudocode:

    if (!code) {
        if (!IS_ERR(newdp)) {
            return no_error;
        } else {
            return newdp_error;
        }
    } else {
        return code_error;
    }

The multiple different nested error cases make this a little complex.
To make this easier to follow for subsequent changes, alter this
structure to be more like this:

    if (IS_ERR(newdp)) {
        return newdp_error;
    }

    if (code) {
        return code_error;
    }

    return no_error;

There should be no functional change in this commit; it is just code
reorganization.

Technically the ordering of these checks is changed, but there is no
combination of conditions that actually results in different code
being hit. That is, if 'code' is nonzero and IS_ERR(newdp) is true,
then we would go through a different path. But that cannot happen,
since if 'code' is nonzero, we have no inode and so IS_ERR(newdp)
cannot be true (d_splice_alias cannot return an error for a NULL
inode). So there is no functional change.

Change-Id: I94a3aef5239358c3d13fe5314044dcc85914d0a4
Reviewed-on: https://gerrit.openafs.org/12636
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Joe Gorse <jhgorse@gmail.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Tested-by: Michael Meffie <mmeffie@sinenomine.net>

src/afs/LINUX/osi_vnodeops.c

index 1e59e11..97b5486 100644 (file)
@@ -1589,34 +1589,30 @@ afs_linux_lookup(struct inode *dip, struct dentry *dp)
  done:
     crfree(credp);
 
-    /* It's ok for the file to not be found. That's noted by the caller by
-     * seeing that the dp->d_inode field is NULL.
-     */
-    if (!code || code == ENOENT) {
-       /*
-        * d_splice_alias can return an error (EIO) if there is an existing
-        * connected directory alias for this dentry.
-        */
-       if (!IS_ERR(newdp)) {
-           iput(ip);
-           return newdp;
-       } else {
-           d_add(dp, ip);
-           /*
-            * Depending on the kernel version, d_splice_alias may or may
-            * not drop the inode reference on error.  If it didn't, do it
-            * here.
-            */
+    if (IS_ERR(newdp)) {
+        /* d_splice_alias can return an error (EIO) if there is an existing
+         * connected directory alias for this dentry. Add our dentry manually
+         * ourselves if this happens. */
+        d_add(dp, ip);
+
 #if defined(D_SPLICE_ALIAS_LEAK_ON_ERROR)
-           iput(ip);
+        /* Depending on the kernel version, d_splice_alias may or may not drop
+         * the inode reference on error. If it didn't, do it here. */
+        iput(ip);
 #endif
-           return NULL;
-       }
-    } else {
+        return NULL;
+    }
+
+    /* It's ok for the file to not be found (ENOENT). That's noted by the
+     * caller by seeing that the dp->d_inode field is NULL. */
+    if (code && code != ENOENT) {
        if (ip)
            iput(ip);
        return ERR_PTR(afs_convert_code(code));
     }
+
+    iput(ip);
+    return newdp;
 }
 
 static int