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 <jlayton@redhat.com>

diff --git a/src/client/Client.cc b/src/client/Client.cc
index f540df5cbcb9b2d156bd3583b4034e066eb8b94f..989f32c4f2804dd1154de3af569201431a609f69 100644 (file)
--- 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 b30e2e0ba44bfc204e06394e7fd404cffeca7908..6c3bf7c0a2da7a7edef35ce9cf8a014ca8c2e408 100644 (file)
--- 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));