Reviewed-by: Josh Durgin <josh.durgin@xxxxxxxxxxx>
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 <elder@xxxxxxxxxxx>
---
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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
