diff mbox

[04/47] block-rbd: Refactor two calls for memory allocations in rbd_dev_image_id()

Message ID dd52b320-58e3-48f7-f909-49bd3f388a5e@users.sourceforge.net (mailing list archive)
State New, archived
Headers show

Commit Message

SF Markus Elfring Sept. 12, 2016, 6:45 p.m. UTC
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 11 Sep 2016 14:48:41 +0200

* Pass the sizes for memory allocations to the corresponding functions
  directly without storing the calculated values in an
  intermediate variable.

* Delete the local variable "size" which became unnecessary with
  this refactoring.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/block/rbd.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

Comments

Ilya Dryomov Sept. 13, 2016, 8:03 a.m. UTC | #1
On Mon, Sep 12, 2016 at 8:45 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 11 Sep 2016 14:48:41 +0200
>
> * Pass the sizes for memory allocations to the corresponding functions
>   directly without storing the calculated values in an
>   intermediate variable.
>
> * Delete the local variable "size" which became unnecessary with
>   this refactoring.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/block/rbd.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index d61a066..c1da844 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -5833,7 +5833,6 @@ again:
>  static int rbd_dev_image_id(struct rbd_device *rbd_dev)
>  {
>         int ret;
> -       size_t size;
>         char *object_name;
>         void *response;
>         char *image_id;
> @@ -5854,17 +5853,16 @@ static int rbd_dev_image_id(struct rbd_device *rbd_dev)
>          * First, see if the format 2 image id file exists, and if
>          * so, get the image's persistent id from it.
>          */
> -       size = sizeof (RBD_ID_PREFIX) + strlen(rbd_dev->spec->image_name);
> -       object_name = kmalloc(size, GFP_NOIO);
> +       object_name = kmalloc(sizeof(RBD_ID_PREFIX)
> +                             + strlen(rbd_dev->spec->image_name),
> +                             GFP_NOIO);
>         if (!object_name)
>                 return -ENOMEM;
>         sprintf(object_name, "%s%s", RBD_ID_PREFIX, rbd_dev->spec->image_name);
>         dout("rbd id object name is %s\n", object_name);
>
>         /* Response will be an encoded string, which includes a length */
> -
> -       size = sizeof (__le32) + RBD_IMAGE_ID_LEN_MAX;
> -       response = kzalloc(size, GFP_NOIO);
> +       response = kzalloc(sizeof(__le32) + RBD_IMAGE_ID_LEN_MAX, GFP_NOIO);
>         if (!response) {
>                 ret = -ENOMEM;
>                 goto out;
> --
> 2.10.0
>

How is this any better?  If anything, it makes the first kmalloc() call
slightly less readable.

Thanks,

                Ilya
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
SF Markus Elfring Sept. 13, 2016, 8:36 a.m. UTC | #2
>> @@ -5833,7 +5833,6 @@ again:
>>  static int rbd_dev_image_id(struct rbd_device *rbd_dev)
>>  {
>>         int ret;
>> -       size_t size;
>>         char *object_name;
>>         void *response;
>>         char *image_id;
>> @@ -5854,17 +5853,16 @@ static int rbd_dev_image_id(struct rbd_device *rbd_dev)
>>          * First, see if the format 2 image id file exists, and if
>>          * so, get the image's persistent id from it.
>>          */
>> -       size = sizeof (RBD_ID_PREFIX) + strlen(rbd_dev->spec->image_name);
>> -       object_name = kmalloc(size, GFP_NOIO);
>> +       object_name = kmalloc(sizeof(RBD_ID_PREFIX)
>> +                             + strlen(rbd_dev->spec->image_name),
>> +                             GFP_NOIO);
>>         if (!object_name)
>>                 return -ENOMEM;
>>         sprintf(object_name, "%s%s", RBD_ID_PREFIX, rbd_dev->spec->image_name);
>>         dout("rbd id object name is %s\n", object_name);
>>
>>         /* Response will be an encoded string, which includes a length */
>> -
>> -       size = sizeof (__le32) + RBD_IMAGE_ID_LEN_MAX;
>> -       response = kzalloc(size, GFP_NOIO);
>> +       response = kzalloc(sizeof(__le32) + RBD_IMAGE_ID_LEN_MAX, GFP_NOIO);
>>         if (!response) {
>>                 ret = -ENOMEM;
>>                 goto out;> How is this any better?

I find it useful to omit the local variable "size" here.


> If anything, it makes the first kmalloc() call slightly less readable.

I got an other impression. The refactored function call did not fit into a single line
because of a well-known length limitation.

Does the kzalloc() call look a bit nicer for you now?

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index d61a066..c1da844 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -5833,7 +5833,6 @@  again:
 static int rbd_dev_image_id(struct rbd_device *rbd_dev)
 {
 	int ret;
-	size_t size;
 	char *object_name;
 	void *response;
 	char *image_id;
@@ -5854,17 +5853,16 @@  static int rbd_dev_image_id(struct rbd_device *rbd_dev)
 	 * First, see if the format 2 image id file exists, and if
 	 * so, get the image's persistent id from it.
 	 */
-	size = sizeof (RBD_ID_PREFIX) + strlen(rbd_dev->spec->image_name);
-	object_name = kmalloc(size, GFP_NOIO);
+	object_name = kmalloc(sizeof(RBD_ID_PREFIX)
+			      + strlen(rbd_dev->spec->image_name),
+			      GFP_NOIO);
 	if (!object_name)
 		return -ENOMEM;
 	sprintf(object_name, "%s%s", RBD_ID_PREFIX, rbd_dev->spec->image_name);
 	dout("rbd id object name is %s\n", object_name);
 
 	/* Response will be an encoded string, which includes a length */
-
-	size = sizeof (__le32) + RBD_IMAGE_ID_LEN_MAX;
-	response = kzalloc(size, GFP_NOIO);
+	response = kzalloc(sizeof(__le32) + RBD_IMAGE_ID_LEN_MAX, GFP_NOIO);
 	if (!response) {
 		ret = -ENOMEM;
 		goto out;