rbd: remove options args from rbd_add_parse_args()

They "options" argument to rbd_add_parse_args() (and it's partner
options_size) is now only needed within the function, so there's no
need to have the caller allocate and pass the options buffer.  Just
allocate the options buffer within the function using dup_token().

Also distinguish between failures due to failed memory allocation
and failing because a required argument was missing.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>

1 files changed, 29 insertions, 29 deletions

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 7bd23139b948..62df67a11321 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -2844,54 +2844,58 @@ static inline char *dup_token(const char **buf, size_t *lenp)
  */
 static struct ceph_options *rbd_add_parse_args(struct rbd_device *rbd_dev,
                                                 const char *buf,
-                                                char *options,
-                                                size_t options_size,
                                                 char **snap_name)
 {
         size_t len;
         const char *mon_addrs;
         size_t mon_addrs_size;
+        char *options;
+        struct ceph_options *err_ptr = ERR_PTR(-EINVAL);
         struct rbd_options rbd_opts;
         struct ceph_options *ceph_opts;
-        struct ceph_options *err_ptr = ERR_PTR(-EINVAL);
 
         /* The first four tokens are required */
 
         len = next_token(&buf);
         if (!len)
-                return err_ptr;
-        mon_addrs_size = len + 1;
+                return err_ptr; /* Missing monitor address(es) */
         mon_addrs = buf;
-
+        mon_addrs_size = len + 1;
         buf += len;
 
-        len = copy_token(&buf, options, options_size);
-        if (!len || len >= options_size)
-                return err_ptr;
+        options = dup_token(&buf, NULL);
+        if (!options)
+                goto out_mem;
+        if (!*options)
+                goto out_err;   /* Missing options */
 
-        err_ptr = ERR_PTR(-ENOMEM);
         rbd_dev->pool_name = dup_token(&buf, NULL);
         if (!rbd_dev->pool_name)
-                goto out_err;
+                goto out_mem;
+        if (!*rbd_dev->pool_name)
+                goto out_err;   /* Missing pool name */
 
         rbd_dev->image_name = dup_token(&buf, &rbd_dev->image_name_len);
         if (!rbd_dev->image_name)
-                goto out_err;
-
-        /* Snapshot name is optional; default is to use "head" */
+                goto out_mem;
+        if (!*rbd_dev->image_name)
+                goto out_err;   /* Missing image name */
 
+        /*
+         * Snapshot name is optional; default is to use "-"
+         * (indicating the head/no snapshot).
+         */
         len = next_token(&buf);
-        if (len > RBD_MAX_SNAP_NAME_LEN) {
-                err_ptr = ERR_PTR(-ENAMETOOLONG);
-                goto out_err;
-        }
         if (!len) {
                 buf = RBD_SNAP_HEAD_NAME; /* No snapshot supplied */
                 len = sizeof (RBD_SNAP_HEAD_NAME) - 1;
+        } else if (len > RBD_MAX_SNAP_NAME_LEN) {
+                err_ptr = ERR_PTR(-ENAMETOOLONG);
+                goto out_err;
         }
         *snap_name = kmalloc(len + 1, GFP_KERNEL);
         if (!*snap_name)
-                goto out_err;
+                goto out_mem;
         memcpy(*snap_name, buf, len);
         *(*snap_name + len) = '\0';
 
@@ -2902,20 +2906,23 @@ static struct ceph_options *rbd_add_parse_args(struct rbd_device *rbd_dev,
         ceph_opts = ceph_parse_options(options, mon_addrs,
                                         mon_addrs + mon_addrs_size - 1,
                                         parse_rbd_opts_token, &rbd_opts);
+        kfree(options);
 
         /* Record the parsed rbd options */
 
-        if (!IS_ERR(ceph_opts)) {
+        if (!IS_ERR(ceph_opts))
                 rbd_dev->mapping.read_only = rbd_opts.read_only;
-        }
 
         return ceph_opts;
+out_mem:
+        err_ptr = ERR_PTR(-ENOMEM);
 out_err:
         kfree(rbd_dev->image_name);
         rbd_dev->image_name = NULL;
         rbd_dev->image_name_len = 0;
         kfree(rbd_dev->pool_name);
         rbd_dev->pool_name = NULL;
+        kfree(options);
 
         return err_ptr;
 }
@@ -3124,7 +3131,6 @@ static ssize_t rbd_add(struct bus_type *bus,
                        const char *buf,
                        size_t count)
 {
-        char *options;
         struct rbd_device *rbd_dev = NULL;
         char *snap_name;
         struct ceph_options *ceph_opts;
@@ -3134,9 +3140,6 @@ static ssize_t rbd_add(struct bus_type *bus,
         if (!try_module_get(THIS_MODULE))
                 return -ENODEV;
 
-        options = kmalloc(count, GFP_KERNEL);
-        if (!options)
-                goto err_out_mem;
         rbd_dev = kzalloc(sizeof(*rbd_dev), GFP_KERNEL);
         if (!rbd_dev)
                 goto err_out_mem;
@@ -3148,8 +3151,7 @@ static ssize_t rbd_add(struct bus_type *bus,
         init_rwsem(&rbd_dev->header_rwsem);
 
         /* parse add command */
-        ceph_opts = rbd_add_parse_args(rbd_dev, buf, options, count,
-                                &snap_name);
+        ceph_opts = rbd_add_parse_args(rbd_dev, buf, &snap_name);
         if (IS_ERR(ceph_opts)) {
                 rc = PTR_ERR(ceph_opts);
                 goto err_out_mem;
@@ -3233,7 +3235,6 @@ err_out_bus:
         /* this will also clean up rest of rbd_dev stuff */
 
         rbd_bus_del_dev(rbd_dev);
-        kfree(options);
         return rc;
 
 err_out_disk:
@@ -3258,7 +3259,6 @@ err_out_args:
         kfree(rbd_dev->pool_name);
 err_out_mem:
         kfree(rbd_dev);
-        kfree(options);
 
         dout("Error adding device %s\n", buf);
         module_put(THIS_MODULE);