On Sun, Nov 2, 2014 at 5:20 PM, SF Markus Elfring
<elfring@xxxxxxxxxxxxxxxxxxxxx> wrote:
> The functions ceph_put_snap_context() and iput() test whether their argument
> is NULL and then return immediately. Thus the test around the call
> is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx>
> ---
> fs/ceph/caps.c | 3 +--
> fs/ceph/mds_client.c | 6 ++----
> fs/ceph/snap.c | 9 +++------
> 3 files changed, 6 insertions(+), 12 deletions(-)
[CC'ed Zheng]
Applied, but see below.
>
> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> index 6d1cd45..7d99fc8 100644
> --- a/fs/ceph/caps.c
> +++ b/fs/ceph/caps.c
> @@ -3136,8 +3136,7 @@ flush_cap_releases:
> done:
> mutex_unlock(&session->s_mutex);
> done_unlocked:
> - if (inode)
> - iput(inode);
> + iput(inode);
> return;
>
> bad:
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index bad07c0..3b0ab05 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -523,8 +523,7 @@ void ceph_mdsc_release_request(struct kref *kref)
> }
> if (req->r_locked_dir)
> ceph_put_cap_refs(ceph_inode(req->r_locked_dir), CEPH_CAP_PIN);
> - if (req->r_target_inode)
> - iput(req->r_target_inode);
> + iput(req->r_target_inode);
> if (req->r_dentry)
> dput(req->r_dentry);
dput() also checks for NULL argument, but the check is wrapped into
unlikely(), which is why I presume it wasn't picked up. It would be
great if you could improve your coccinelle script to handle
{un,}likely() as well.
> if (req->r_old_dentry)
> @@ -995,8 +994,7 @@ out:
> session->s_cap_iterator = NULL;
> spin_unlock(&session->s_cap_lock);
>
> - if (last_inode)
> - iput(last_inode);
> + iput(last_inode);
> if (old_cap)
> ceph_put_cap(session->s_mdsc, old_cap);
>
> diff --git a/fs/ceph/snap.c b/fs/ceph/snap.c
> index f01645a..c1cc993 100644
> --- a/fs/ceph/snap.c
> +++ b/fs/ceph/snap.c
> @@ -365,8 +365,7 @@ static int build_snap_context(struct ceph_snap_realm *realm)
> realm->ino, realm, snapc, snapc->seq,
> (unsigned int) snapc->num_snaps);
>
> - if (realm->cached_context)
> - ceph_put_snap_context(realm->cached_context);
> + ceph_put_snap_context(realm->cached_context);
> realm->cached_context = snapc;
> return 0;
>
> @@ -590,15 +589,13 @@ static void queue_realm_cap_snaps(struct ceph_snap_realm
> *realm)
The patch was corrupted, that should have been a single line. I fixed
it up but you may want to look into your email client settings.
Thanks,
Ilya
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
