2008-07-25 20:08:13

by Julia Lawall

[permalink] [raw]
Subject: [PATCH] fs/nfsd/export.c: Adjust error handling code involving auth_domain_put

From: Julia Lawall <[email protected]>

Once clp is assigned, it never becomes NULL, so we can make a label for it
in the error handling code. Because the call to path_lookup follows the
call to auth_domain_find, its error handling code should jump to this new
label.

The semantic match that finds this problem is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// <smpl>
@[email protected]
expression x,E;
statement S;
position p1,p2,p3;
@@

(
if ((x = [email protected](...)) == NULL || ...) S
|
x = [email protected](...)
... when != x
if (x == NULL || ...) S
)
<...
[email protected] (...) { ... when != auth_domain_put(x)
when != if (x) { ... auth_domain_put(x); ...}
[email protected] ...;
}
...>
(
return x;
|
return 0;
|
x = E
|
E = x
|
auth_domain_put(x)
)

@[email protected]
position r.p1,r.p2,r.p3;
expression x;
int ret != 0;
statement S;
@@

* x = [email protected](...)
<...
* [email protected] (...)
S
...>
* [email protected] \(NULL\|ret\);
// </smpl>

Signed-off-by: Julia Lawall <[email protected]>

---
fs/nfsd/export.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index 33bfcf0..9dc036f 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -1023,7 +1023,7 @@ exp_export(struct nfsctl_export *nxp)
/* Look up the dentry */
err = path_lookup(nxp->ex_path, 0, &nd);
if (err)
- goto out_unlock;
+ goto out_put_clp;
err = -EINVAL;

exp = exp_get_by_name(clp, nd.path.mnt, nd.path.dentry, NULL);
@@ -1090,9 +1090,9 @@ finish:
exp_put(exp);
if (fsid_key && !IS_ERR(fsid_key))
cache_put(&fsid_key->h, &svc_expkey_cache);
- if (clp)
- auth_domain_put(clp);
path_put(&nd.path);
+out_put_clp:
+ auth_domain_put(clp);
out_unlock:
exp_writeunlock();
out:


2008-07-30 17:24:56

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH] fs/nfsd/export.c: Adjust error handling code involving auth_domain_put

On Fri, Jul 25, 2008 at 10:08:09PM +0200, Julia Lawall wrote:
> From: Julia Lawall <[email protected]>
>
> Once clp is assigned, it never becomes NULL, so we can make a label for it
> in the error handling code. Because the call to path_lookup follows the
> call to auth_domain_find, its error handling code should jump to this new
> label.

Yes, that looks correct to me; thanks! Applied.--b.

>
> The semantic match that finds this problem is as follows:
> (http://www.emn.fr/x-info/coccinelle/)
>
> // <smpl>
> @[email protected]
> expression x,E;
> statement S;
> position p1,p2,p3;
> @@
>
> (
> if ((x = [email protected](...)) == NULL || ...) S
> |
> x = [email protected](...)
> ... when != x
> if (x == NULL || ...) S
> )
> <...
> [email protected] (...) { ... when != auth_domain_put(x)
> when != if (x) { ... auth_domain_put(x); ...}
> [email protected] ...;
> }
> ...>
> (
> return x;
> |
> return 0;
> |
> x = E
> |
> E = x
> |
> auth_domain_put(x)
> )
>
> @[email protected]
> position r.p1,r.p2,r.p3;
> expression x;
> int ret != 0;
> statement S;
> @@
>
> * x = [email protected](...)
> <...
> * [email protected] (...)
> S
> ...>
> * [email protected] \(NULL\|ret\);
> // </smpl>
>
> Signed-off-by: Julia Lawall <[email protected]>
>
> ---
> fs/nfsd/export.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
> index 33bfcf0..9dc036f 100644
> --- a/fs/nfsd/export.c
> +++ b/fs/nfsd/export.c
> @@ -1023,7 +1023,7 @@ exp_export(struct nfsctl_export *nxp)
> /* Look up the dentry */
> err = path_lookup(nxp->ex_path, 0, &nd);
> if (err)
> - goto out_unlock;
> + goto out_put_clp;
> err = -EINVAL;
>
> exp = exp_get_by_name(clp, nd.path.mnt, nd.path.dentry, NULL);
> @@ -1090,9 +1090,9 @@ finish:
> exp_put(exp);
> if (fsid_key && !IS_ERR(fsid_key))
> cache_put(&fsid_key->h, &svc_expkey_cache);
> - if (clp)
> - auth_domain_put(clp);
> path_put(&nd.path);
> +out_put_clp:
> + auth_domain_put(clp);
> out_unlock:
> exp_writeunlock();
> out: