From 436b7a661f0c9c9e38b8184a18d437215d19c3ba Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Mon, 19 Sep 2016 15:20:58 -0400 Subject: [PATCH] client: reinstate clearing of *fhp in ll_create and don't allow fhp == NULL The logic in ll_create relies on *fhp being zeroed out on entry into the function, but that's no longer being done since commit e08210dda. Fix this by reinstating that initialization. Luckily, it looks like the two main callers (FUSE and nfs-ganesha) already handle this safely, but the SyntheticClient does not. Move the zeroing of the pointer into ll_create, and allow callers to pass in a pointer to an uninitialized value. With this, we can remove the initialization in the fuse client code as well. Also, we can get rid of some unneeded NULL pointer checks if we don't allow callers to pass in a NULL fhp pointer. As best I can tell, no one does that currently and no comments over ceph_ll_create seem to claim that this should be allowed. Signed-off-by: Jeff Layton --- src/client/Client.cc | 19 ++++++++++--------- src/client/fuse_ll.cc | 2 +- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/client/Client.cc b/src/client/Client.cc index f540df5cbcb..989f32c4f28 100644 --- a/src/client/Client.cc +++ b/src/client/Client.cc @@ -11605,6 +11605,8 @@ int Client::ll_create(Inode *parent, const char *name, mode_t mode, int flags, struct stat *attr, Inode **outp, Fh **fhp, int uid, int gid) { + *fhp = NULL; + Mutex::Locker lock(client_lock); vinodeno_t vparent = _get_vino(parent); @@ -11630,8 +11632,8 @@ int Client::ll_create(Inode *parent, const char *name, mode_t mode, if (r < 0) goto out; } - r = _create(parent, name, flags, mode, &in, fhp /* may be NULL */, - 0, 0, 0, NULL, &created, uid, gid); + r = _create(parent, name, flags, mode, &in, fhp, 0, 0, 0, NULL, &created, + uid, gid); if (r < 0) goto out; } @@ -11647,14 +11649,14 @@ int Client::ll_create(Inode *parent, const char *name, mode_t mode, if (!cct->_conf->fuse_default_permissions) { r = may_open(in.get(), flags, uid, gid); if (r < 0) { - if (fhp && *fhp) { + if (*fhp) { int release_r = _release_fh(*fhp); assert(release_r == 0); // during create, no async data ops should have happened } goto out; } } - if (fhp && (*fhp == NULL)) { + if (*fhp == NULL) { r = _open(in.get(), flags, mode, fhp, uid, gid); if (r < 0) goto out; @@ -11665,14 +11667,13 @@ out: if (r < 0) attr->st_ino = 0; - Fh *fhptr = fhp ? *fhp : NULL; - if (fhptr) { - ll_unclosed_fh_set.insert(fhptr); + if (*fhp) { + ll_unclosed_fh_set.insert(*fhp); } - tout(cct) << (unsigned long)fhptr << std::endl; + tout(cct) << (unsigned long)*fhp << std::endl; tout(cct) << attr->st_ino << std::endl; ldout(cct, 3) << "ll_create " << parent << " " << name << " 0" << oct << - mode << dec << " " << flags << " = " << r << " (" << fhptr << " " << + mode << dec << " " << flags << " = " << r << " (" << *fhp << " " << hex << attr->st_ino << dec << ")" << dendl; // passing an Inode in outp requires an additional ref diff --git a/src/client/fuse_ll.cc b/src/client/fuse_ll.cc index b30e2e0ba44..6c3bf7c0a2d 100644 --- a/src/client/fuse_ll.cc +++ b/src/client/fuse_ll.cc @@ -658,7 +658,7 @@ static void fuse_ll_create(fuse_req_t req, fuse_ino_t parent, const char *name, const struct fuse_ctx *ctx = fuse_req_ctx(req); Inode *i1 = cfuse->iget(parent), *i2; struct fuse_entry_param fe; - Fh *fh = NULL; + Fh *fh; memset(&fe, 0, sizeof(fe)); -- 2.39.5