Reviewed-by: Josh Durgin <[email protected]>
On 10/26/2012 04:00 PM, Alex Elder wrote:
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 <[email protected]> --- drivers/block/rbd.c | 58 +++++++++++++++++++++++++-------------------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index cae7423..f900f3b 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);
-- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
