All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc()
@ 2019-02-23 18:51 ` Gustavo A. R. Silva
  0 siblings, 0 replies; 16+ messages in thread
From: Gustavo A. R. Silva @ 2019-02-23 18:51 UTC (permalink / raw)


Update the code to use a zero-sized array instead of a pointer in
structure nvmet_fc_tgt_queue and use struct_size() in kzalloc().

Notice that one of the more common cases of allocation size calculations
is finding the size of a structure that has a zero-sized array at the end,
along with memory for some number of elements for that array. For example:

struct foo {
	int stuff;
	struct boo entry[];
};

instance = kzalloc(sizeof(struct foo) + sizeof(struct boo) * count, GFP_KERNEL);

Instead of leaving these open-coded and prone to type mistakes, we can now
use the new struct_size() helper:

instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL);

This code was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva <gustavo at embeddedor.com>
---
 drivers/nvme/target/fc.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
index 1e9654f04c60..23baec38f97e 100644
--- a/drivers/nvme/target/fc.c
+++ b/drivers/nvme/target/fc.c
@@ -128,12 +128,12 @@ struct nvmet_fc_tgt_queue {
 	struct nvmet_cq			nvme_cq;
 	struct nvmet_sq			nvme_sq;
 	struct nvmet_fc_tgt_assoc	*assoc;
-	struct nvmet_fc_fcp_iod		*fod;		/* array of fcp_iods */
 	struct list_head		fod_list;
 	struct list_head		pending_cmd_list;
 	struct list_head		avail_defer_list;
 	struct workqueue_struct		*work_q;
 	struct kref			ref;
+	struct nvmet_fc_fcp_iod		fod[];		/* array of fcp_iods */
 } __aligned(sizeof(unsigned long long));
 
 struct nvmet_fc_tgt_assoc {
@@ -588,9 +588,7 @@ nvmet_fc_alloc_target_queue(struct nvmet_fc_tgt_assoc *assoc,
 	if (qid > NVMET_NR_QUEUES)
 		return NULL;
 
-	queue = kzalloc((sizeof(*queue) +
-				(sizeof(struct nvmet_fc_fcp_iod) * sqsize)),
-				GFP_KERNEL);
+	queue = kzalloc(struct_size(queue, fod, sqsize), GFP_KERNEL);
 	if (!queue)
 		return NULL;
 
@@ -603,7 +601,6 @@ nvmet_fc_alloc_target_queue(struct nvmet_fc_tgt_assoc *assoc,
 	if (!queue->work_q)
 		goto out_a_put;
 
-	queue->fod = (struct nvmet_fc_fcp_iod *)&queue[1];
 	queue->qid = qid;
 	queue->sqsize = sqsize;
 	queue->assoc = assoc;
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc()
@ 2019-02-23 18:51 ` Gustavo A. R. Silva
  0 siblings, 0 replies; 16+ messages in thread
From: Gustavo A. R. Silva @ 2019-02-23 18:51 UTC (permalink / raw)
  To: James Smart, Christoph Hellwig, Sagi Grimberg
  Cc: linux-nvme, linux-kernel, Gustavo A. R. Silva

Update the code to use a zero-sized array instead of a pointer in
structure nvmet_fc_tgt_queue and use struct_size() in kzalloc().

Notice that one of the more common cases of allocation size calculations
is finding the size of a structure that has a zero-sized array at the end,
along with memory for some number of elements for that array. For example:

struct foo {
	int stuff;
	struct boo entry[];
};

instance = kzalloc(sizeof(struct foo) + sizeof(struct boo) * count, GFP_KERNEL);

Instead of leaving these open-coded and prone to type mistakes, we can now
use the new struct_size() helper:

instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL);

This code was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 drivers/nvme/target/fc.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
index 1e9654f04c60..23baec38f97e 100644
--- a/drivers/nvme/target/fc.c
+++ b/drivers/nvme/target/fc.c
@@ -128,12 +128,12 @@ struct nvmet_fc_tgt_queue {
 	struct nvmet_cq			nvme_cq;
 	struct nvmet_sq			nvme_sq;
 	struct nvmet_fc_tgt_assoc	*assoc;
-	struct nvmet_fc_fcp_iod		*fod;		/* array of fcp_iods */
 	struct list_head		fod_list;
 	struct list_head		pending_cmd_list;
 	struct list_head		avail_defer_list;
 	struct workqueue_struct		*work_q;
 	struct kref			ref;
+	struct nvmet_fc_fcp_iod		fod[];		/* array of fcp_iods */
 } __aligned(sizeof(unsigned long long));
 
 struct nvmet_fc_tgt_assoc {
@@ -588,9 +588,7 @@ nvmet_fc_alloc_target_queue(struct nvmet_fc_tgt_assoc *assoc,
 	if (qid > NVMET_NR_QUEUES)
 		return NULL;
 
-	queue = kzalloc((sizeof(*queue) +
-				(sizeof(struct nvmet_fc_fcp_iod) * sqsize)),
-				GFP_KERNEL);
+	queue = kzalloc(struct_size(queue, fod, sqsize), GFP_KERNEL);
 	if (!queue)
 		return NULL;
 
@@ -603,7 +601,6 @@ nvmet_fc_alloc_target_queue(struct nvmet_fc_tgt_assoc *assoc,
 	if (!queue->work_q)
 		goto out_a_put;
 
-	queue->fod = (struct nvmet_fc_fcp_iod *)&queue[1];
 	queue->qid = qid;
 	queue->sqsize = sqsize;
 	queue->assoc = assoc;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc()
  2019-02-23 18:51 ` Gustavo A. R. Silva
@ 2019-02-23 20:05   ` Joe Perches
  -1 siblings, 0 replies; 16+ messages in thread
From: Joe Perches @ 2019-02-23 20:05 UTC (permalink / raw)


On Sat, 2019-02-23@12:51 -0600, Gustavo A. R. Silva wrote:
> Update the code to use a zero-sized array instead of a pointer in
> structure nvmet_fc_tgt_queue and use struct_size() in kzalloc().
[]
> This code was detected with the help of Coccinelle.

Really?
Impressive script that found this one.

> diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
[]
> @@ -128,12 +128,12 @@ struct nvmet_fc_tgt_queue {
>  	struct nvmet_cq			nvme_cq;
>  	struct nvmet_sq			nvme_sq;
>  	struct nvmet_fc_tgt_assoc	*assoc;
> -	struct nvmet_fc_fcp_iod		*fod;		/* array of fcp_iods */
>  	struct list_head		fod_list;
>  	struct list_head		pending_cmd_list;
>  	struct list_head		avail_defer_list;
>  	struct workqueue_struct		*work_q;
>  	struct kref			ref;
> +	struct nvmet_fc_fcp_iod		fod[];		/* array of fcp_iods */
>  } __aligned(sizeof(unsigned long long));

Moving a pointer from the middle of a struct to
the end seems unusual for coccinelle.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc()
@ 2019-02-23 20:05   ` Joe Perches
  0 siblings, 0 replies; 16+ messages in thread
From: Joe Perches @ 2019-02-23 20:05 UTC (permalink / raw)
  To: Gustavo A. R. Silva, James Smart, Christoph Hellwig,
	Sagi Grimberg
  Cc: linux-nvme, linux-kernel

On Sat, 2019-02-23 at 12:51 -0600, Gustavo A. R. Silva wrote:
> Update the code to use a zero-sized array instead of a pointer in
> structure nvmet_fc_tgt_queue and use struct_size() in kzalloc().
[]
> This code was detected with the help of Coccinelle.

Really?
Impressive script that found this one.

> diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
[]
> @@ -128,12 +128,12 @@ struct nvmet_fc_tgt_queue {
>  	struct nvmet_cq			nvme_cq;
>  	struct nvmet_sq			nvme_sq;
>  	struct nvmet_fc_tgt_assoc	*assoc;
> -	struct nvmet_fc_fcp_iod		*fod;		/* array of fcp_iods */
>  	struct list_head		fod_list;
>  	struct list_head		pending_cmd_list;
>  	struct list_head		avail_defer_list;
>  	struct workqueue_struct		*work_q;
>  	struct kref			ref;
> +	struct nvmet_fc_fcp_iod		fod[];		/* array of fcp_iods */
>  } __aligned(sizeof(unsigned long long));

Moving a pointer from the middle of a struct to
the end seems unusual for coccinelle.



^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc()
  2019-02-23 20:05   ` Joe Perches
@ 2019-02-23 20:28     ` Gustavo A. R. Silva
  -1 siblings, 0 replies; 16+ messages in thread
From: Gustavo A. R. Silva @ 2019-02-23 20:28 UTC (permalink / raw)


Hey Joe,

On 2/23/19 2:05 PM, Joe Perches wrote:
> On Sat, 2019-02-23@12:51 -0600, Gustavo A. R. Silva wrote:
>> Update the code to use a zero-sized array instead of a pointer in
>> structure nvmet_fc_tgt_queue and use struct_size() in kzalloc().
> []
>> This code was detected with the help of Coccinelle.
> 
> Really?
> Impressive script that found this one.
> 

See my comments below.

>> diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
> []
>> @@ -128,12 +128,12 @@ struct nvmet_fc_tgt_queue {
>>  	struct nvmet_cq			nvme_cq;
>>  	struct nvmet_sq			nvme_sq;
>>  	struct nvmet_fc_tgt_assoc	*assoc;
>> -	struct nvmet_fc_fcp_iod		*fod;		/* array of fcp_iods */
>>  	struct list_head		fod_list;
>>  	struct list_head		pending_cmd_list;
>>  	struct list_head		avail_defer_list;
>>  	struct workqueue_struct		*work_q;
>>  	struct kref			ref;
>> +	struct nvmet_fc_fcp_iod		fod[];		/* array of fcp_iods */
>>  } __aligned(sizeof(unsigned long long));
> 
> Moving a pointer from the middle of a struct to
> the end seems unusual for coccinelle.
> 
> 

Notice that the commit log says "detected", which does not imply
the script made the transformation by itself. :)

And all the script detected was this piece of code:

	queue = kzalloc((sizeof(*queue) +
				(sizeof(struct nvmet_fc_fcp_iod) * sqsize)),
				GFP_KERNEL);


Thanks
--
Gustavo

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc()
@ 2019-02-23 20:28     ` Gustavo A. R. Silva
  0 siblings, 0 replies; 16+ messages in thread
From: Gustavo A. R. Silva @ 2019-02-23 20:28 UTC (permalink / raw)
  To: Joe Perches, James Smart, Christoph Hellwig, Sagi Grimberg
  Cc: linux-nvme, linux-kernel

Hey Joe,

On 2/23/19 2:05 PM, Joe Perches wrote:
> On Sat, 2019-02-23 at 12:51 -0600, Gustavo A. R. Silva wrote:
>> Update the code to use a zero-sized array instead of a pointer in
>> structure nvmet_fc_tgt_queue and use struct_size() in kzalloc().
> []
>> This code was detected with the help of Coccinelle.
> 
> Really?
> Impressive script that found this one.
> 

See my comments below.

>> diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
> []
>> @@ -128,12 +128,12 @@ struct nvmet_fc_tgt_queue {
>>  	struct nvmet_cq			nvme_cq;
>>  	struct nvmet_sq			nvme_sq;
>>  	struct nvmet_fc_tgt_assoc	*assoc;
>> -	struct nvmet_fc_fcp_iod		*fod;		/* array of fcp_iods */
>>  	struct list_head		fod_list;
>>  	struct list_head		pending_cmd_list;
>>  	struct list_head		avail_defer_list;
>>  	struct workqueue_struct		*work_q;
>>  	struct kref			ref;
>> +	struct nvmet_fc_fcp_iod		fod[];		/* array of fcp_iods */
>>  } __aligned(sizeof(unsigned long long));
> 
> Moving a pointer from the middle of a struct to
> the end seems unusual for coccinelle.
> 
> 

Notice that the commit log says "detected", which does not imply
the script made the transformation by itself. :)

And all the script detected was this piece of code:

	queue = kzalloc((sizeof(*queue) +
				(sizeof(struct nvmet_fc_fcp_iod) * sqsize)),
				GFP_KERNEL);


Thanks
--
Gustavo

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc()
  2019-02-23 20:28     ` Gustavo A. R. Silva
@ 2019-02-24  1:34       ` Gustavo A. R. Silva
  -1 siblings, 0 replies; 16+ messages in thread
From: Gustavo A. R. Silva @ 2019-02-24  1:34 UTC (permalink / raw)




On 2/23/19 2:28 PM, Gustavo A. R. Silva wrote:
> Hey Joe,
> 
> On 2/23/19 2:05 PM, Joe Perches wrote:
>> On Sat, 2019-02-23@12:51 -0600, Gustavo A. R. Silva wrote:
>>> Update the code to use a zero-sized array instead of a pointer in
>>> structure nvmet_fc_tgt_queue and use struct_size() in kzalloc().
>> []
>>> This code was detected with the help of Coccinelle.
>>
>> Really?
>> Impressive script that found this one.
>>
> 
> See my comments below.
> 
>>> diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
>> []
>>> @@ -128,12 +128,12 @@ struct nvmet_fc_tgt_queue {
>>>  	struct nvmet_cq			nvme_cq;
>>>  	struct nvmet_sq			nvme_sq;
>>>  	struct nvmet_fc_tgt_assoc	*assoc;
>>> -	struct nvmet_fc_fcp_iod		*fod;		/* array of fcp_iods */
>>>  	struct list_head		fod_list;
>>>  	struct list_head		pending_cmd_list;
>>>  	struct list_head		avail_defer_list;
>>>  	struct workqueue_struct		*work_q;
>>>  	struct kref			ref;
>>> +	struct nvmet_fc_fcp_iod		fod[];		/* array of fcp_iods */
>>>  } __aligned(sizeof(unsigned long long));
>>
>> Moving a pointer from the middle of a struct to
>> the end seems unusual for coccinelle.
>>
>>
> 
> Notice that the commit log says "detected", which does not imply
> the script made the transformation by itself. :)
> 
> And all the script detected was this piece of code:
> 
> 	queue = kzalloc((sizeof(*queue) +
> 				(sizeof(struct nvmet_fc_fcp_iod) * sqsize)),
> 				GFP_KERNEL);
> 
> 

Which is enough to mention the tool.

Thanks
--
Gustavo

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc()
@ 2019-02-24  1:34       ` Gustavo A. R. Silva
  0 siblings, 0 replies; 16+ messages in thread
From: Gustavo A. R. Silva @ 2019-02-24  1:34 UTC (permalink / raw)
  To: Joe Perches, James Smart, Christoph Hellwig, Sagi Grimberg
  Cc: linux-nvme, linux-kernel



On 2/23/19 2:28 PM, Gustavo A. R. Silva wrote:
> Hey Joe,
> 
> On 2/23/19 2:05 PM, Joe Perches wrote:
>> On Sat, 2019-02-23 at 12:51 -0600, Gustavo A. R. Silva wrote:
>>> Update the code to use a zero-sized array instead of a pointer in
>>> structure nvmet_fc_tgt_queue and use struct_size() in kzalloc().
>> []
>>> This code was detected with the help of Coccinelle.
>>
>> Really?
>> Impressive script that found this one.
>>
> 
> See my comments below.
> 
>>> diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
>> []
>>> @@ -128,12 +128,12 @@ struct nvmet_fc_tgt_queue {
>>>  	struct nvmet_cq			nvme_cq;
>>>  	struct nvmet_sq			nvme_sq;
>>>  	struct nvmet_fc_tgt_assoc	*assoc;
>>> -	struct nvmet_fc_fcp_iod		*fod;		/* array of fcp_iods */
>>>  	struct list_head		fod_list;
>>>  	struct list_head		pending_cmd_list;
>>>  	struct list_head		avail_defer_list;
>>>  	struct workqueue_struct		*work_q;
>>>  	struct kref			ref;
>>> +	struct nvmet_fc_fcp_iod		fod[];		/* array of fcp_iods */
>>>  } __aligned(sizeof(unsigned long long));
>>
>> Moving a pointer from the middle of a struct to
>> the end seems unusual for coccinelle.
>>
>>
> 
> Notice that the commit log says "detected", which does not imply
> the script made the transformation by itself. :)
> 
> And all the script detected was this piece of code:
> 
> 	queue = kzalloc((sizeof(*queue) +
> 				(sizeof(struct nvmet_fc_fcp_iod) * sqsize)),
> 				GFP_KERNEL);
> 
> 

Which is enough to mention the tool.

Thanks
--
Gustavo

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc()
  2019-02-23 18:51 ` Gustavo A. R. Silva
@ 2019-03-08 13:27   ` Christoph Hellwig
  -1 siblings, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2019-03-08 13:27 UTC (permalink / raw)


James, can you take a look at this one?

On Sat, Feb 23, 2019@12:51:08PM -0600, Gustavo A. R. Silva wrote:
> Update the code to use a zero-sized array instead of a pointer in
> structure nvmet_fc_tgt_queue and use struct_size() in kzalloc().
> 
> Notice that one of the more common cases of allocation size calculations
> is finding the size of a structure that has a zero-sized array at the end,
> along with memory for some number of elements for that array. For example:
> 
> struct foo {
> 	int stuff;
> 	struct boo entry[];
> };
> 
> instance = kzalloc(sizeof(struct foo) + sizeof(struct boo) * count, GFP_KERNEL);
> 
> Instead of leaving these open-coded and prone to type mistakes, we can now
> use the new struct_size() helper:
> 
> instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL);
> 
> This code was detected with the help of Coccinelle.
> 
> Signed-off-by: Gustavo A. R. Silva <gustavo at embeddedor.com>
> ---
>  drivers/nvme/target/fc.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
> index 1e9654f04c60..23baec38f97e 100644
> --- a/drivers/nvme/target/fc.c
> +++ b/drivers/nvme/target/fc.c
> @@ -128,12 +128,12 @@ struct nvmet_fc_tgt_queue {
>  	struct nvmet_cq			nvme_cq;
>  	struct nvmet_sq			nvme_sq;
>  	struct nvmet_fc_tgt_assoc	*assoc;
> -	struct nvmet_fc_fcp_iod		*fod;		/* array of fcp_iods */
>  	struct list_head		fod_list;
>  	struct list_head		pending_cmd_list;
>  	struct list_head		avail_defer_list;
>  	struct workqueue_struct		*work_q;
>  	struct kref			ref;
> +	struct nvmet_fc_fcp_iod		fod[];		/* array of fcp_iods */
>  } __aligned(sizeof(unsigned long long));
>  
>  struct nvmet_fc_tgt_assoc {
> @@ -588,9 +588,7 @@ nvmet_fc_alloc_target_queue(struct nvmet_fc_tgt_assoc *assoc,
>  	if (qid > NVMET_NR_QUEUES)
>  		return NULL;
>  
> -	queue = kzalloc((sizeof(*queue) +
> -				(sizeof(struct nvmet_fc_fcp_iod) * sqsize)),
> -				GFP_KERNEL);
> +	queue = kzalloc(struct_size(queue, fod, sqsize), GFP_KERNEL);
>  	if (!queue)
>  		return NULL;
>  
> @@ -603,7 +601,6 @@ nvmet_fc_alloc_target_queue(struct nvmet_fc_tgt_assoc *assoc,
>  	if (!queue->work_q)
>  		goto out_a_put;
>  
> -	queue->fod = (struct nvmet_fc_fcp_iod *)&queue[1];
>  	queue->qid = qid;
>  	queue->sqsize = sqsize;
>  	queue->assoc = assoc;
> -- 
> 2.20.1
---end quoted text---

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc()
@ 2019-03-08 13:27   ` Christoph Hellwig
  0 siblings, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2019-03-08 13:27 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: James Smart, Christoph Hellwig, Sagi Grimberg, linux-nvme,
	linux-kernel

James, can you take a look at this one?

On Sat, Feb 23, 2019 at 12:51:08PM -0600, Gustavo A. R. Silva wrote:
> Update the code to use a zero-sized array instead of a pointer in
> structure nvmet_fc_tgt_queue and use struct_size() in kzalloc().
> 
> Notice that one of the more common cases of allocation size calculations
> is finding the size of a structure that has a zero-sized array at the end,
> along with memory for some number of elements for that array. For example:
> 
> struct foo {
> 	int stuff;
> 	struct boo entry[];
> };
> 
> instance = kzalloc(sizeof(struct foo) + sizeof(struct boo) * count, GFP_KERNEL);
> 
> Instead of leaving these open-coded and prone to type mistakes, we can now
> use the new struct_size() helper:
> 
> instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL);
> 
> This code was detected with the help of Coccinelle.
> 
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
>  drivers/nvme/target/fc.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
> index 1e9654f04c60..23baec38f97e 100644
> --- a/drivers/nvme/target/fc.c
> +++ b/drivers/nvme/target/fc.c
> @@ -128,12 +128,12 @@ struct nvmet_fc_tgt_queue {
>  	struct nvmet_cq			nvme_cq;
>  	struct nvmet_sq			nvme_sq;
>  	struct nvmet_fc_tgt_assoc	*assoc;
> -	struct nvmet_fc_fcp_iod		*fod;		/* array of fcp_iods */
>  	struct list_head		fod_list;
>  	struct list_head		pending_cmd_list;
>  	struct list_head		avail_defer_list;
>  	struct workqueue_struct		*work_q;
>  	struct kref			ref;
> +	struct nvmet_fc_fcp_iod		fod[];		/* array of fcp_iods */
>  } __aligned(sizeof(unsigned long long));
>  
>  struct nvmet_fc_tgt_assoc {
> @@ -588,9 +588,7 @@ nvmet_fc_alloc_target_queue(struct nvmet_fc_tgt_assoc *assoc,
>  	if (qid > NVMET_NR_QUEUES)
>  		return NULL;
>  
> -	queue = kzalloc((sizeof(*queue) +
> -				(sizeof(struct nvmet_fc_fcp_iod) * sqsize)),
> -				GFP_KERNEL);
> +	queue = kzalloc(struct_size(queue, fod, sqsize), GFP_KERNEL);
>  	if (!queue)
>  		return NULL;
>  
> @@ -603,7 +601,6 @@ nvmet_fc_alloc_target_queue(struct nvmet_fc_tgt_assoc *assoc,
>  	if (!queue->work_q)
>  		goto out_a_put;
>  
> -	queue->fod = (struct nvmet_fc_fcp_iod *)&queue[1];
>  	queue->qid = qid;
>  	queue->sqsize = sqsize;
>  	queue->assoc = assoc;
> -- 
> 2.20.1
---end quoted text---

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc()
  2019-02-23 18:51 ` Gustavo A. R. Silva
@ 2019-03-08 19:15   ` James Smart
  -1 siblings, 0 replies; 16+ messages in thread
From: James Smart @ 2019-03-08 19:15 UTC (permalink / raw)


On 2/23/2019 10:51 AM, Gustavo A. R. Silva wrote:
> Update the code to use a zero-sized array instead of a pointer in
> structure nvmet_fc_tgt_queue and use struct_size() in kzalloc().
>
> Notice that one of the more common cases of allocation size calculations
> is finding the size of a structure that has a zero-sized array at the end,
> along with memory for some number of elements for that array. For example:
>
> struct foo {
> 	int stuff;
> 	struct boo entry[];
> };
>
> instance = kzalloc(sizeof(struct foo) + sizeof(struct boo) * count, GFP_KERNEL);
>
> Instead of leaving these open-coded and prone to type mistakes, we can now
> use the new struct_size() helper:
>
> instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL);
>
> This code was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo at embeddedor.com>
> ---
>   drivers/nvme/target/fc.c | 7 ++-----
>   1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
> index 1e9654f04c60..23baec38f97e 100644
> --- a/drivers/nvme/target/fc.c
> +++ b/drivers/nvme/target/fc.c
> @@ -128,12 +128,12 @@ struct nvmet_fc_tgt_queue {
>   	struct nvmet_cq			nvme_cq;
>   	struct nvmet_sq			nvme_sq;
>   	struct nvmet_fc_tgt_assoc	*assoc;
> -	struct nvmet_fc_fcp_iod		*fod;		/* array of fcp_iods */
>   	struct list_head		fod_list;
>   	struct list_head		pending_cmd_list;
>   	struct list_head		avail_defer_list;
>   	struct workqueue_struct		*work_q;
>   	struct kref			ref;
> +	struct nvmet_fc_fcp_iod		fod[];		/* array of fcp_iods */
>   } __aligned(sizeof(unsigned long long));
>   
>   struct nvmet_fc_tgt_assoc {
> @@ -588,9 +588,7 @@ nvmet_fc_alloc_target_queue(struct nvmet_fc_tgt_assoc *assoc,
>   	if (qid > NVMET_NR_QUEUES)
>   		return NULL;
>   
> -	queue = kzalloc((sizeof(*queue) +
> -				(sizeof(struct nvmet_fc_fcp_iod) * sqsize)),
> -				GFP_KERNEL);
> +	queue = kzalloc(struct_size(queue, fod, sqsize), GFP_KERNEL);
>   	if (!queue)
>   		return NULL;
>   
> @@ -603,7 +601,6 @@ nvmet_fc_alloc_target_queue(struct nvmet_fc_tgt_assoc *assoc,
>   	if (!queue->work_q)
>   		goto out_a_put;
>   
> -	queue->fod = (struct nvmet_fc_fcp_iod *)&queue[1];
>   	queue->qid = qid;
>   	queue->sqsize = sqsize;
>   	queue->assoc = assoc;

Reviewed-by:? James Smart <james.smart at broadcom.com>

I guess this is a better style.

-- james

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc()
@ 2019-03-08 19:15   ` James Smart
  0 siblings, 0 replies; 16+ messages in thread
From: James Smart @ 2019-03-08 19:15 UTC (permalink / raw)
  To: Gustavo A. R. Silva, Christoph Hellwig, Sagi Grimberg
  Cc: linux-nvme, linux-kernel

On 2/23/2019 10:51 AM, Gustavo A. R. Silva wrote:
> Update the code to use a zero-sized array instead of a pointer in
> structure nvmet_fc_tgt_queue and use struct_size() in kzalloc().
>
> Notice that one of the more common cases of allocation size calculations
> is finding the size of a structure that has a zero-sized array at the end,
> along with memory for some number of elements for that array. For example:
>
> struct foo {
> 	int stuff;
> 	struct boo entry[];
> };
>
> instance = kzalloc(sizeof(struct foo) + sizeof(struct boo) * count, GFP_KERNEL);
>
> Instead of leaving these open-coded and prone to type mistakes, we can now
> use the new struct_size() helper:
>
> instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL);
>
> This code was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
>   drivers/nvme/target/fc.c | 7 ++-----
>   1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
> index 1e9654f04c60..23baec38f97e 100644
> --- a/drivers/nvme/target/fc.c
> +++ b/drivers/nvme/target/fc.c
> @@ -128,12 +128,12 @@ struct nvmet_fc_tgt_queue {
>   	struct nvmet_cq			nvme_cq;
>   	struct nvmet_sq			nvme_sq;
>   	struct nvmet_fc_tgt_assoc	*assoc;
> -	struct nvmet_fc_fcp_iod		*fod;		/* array of fcp_iods */
>   	struct list_head		fod_list;
>   	struct list_head		pending_cmd_list;
>   	struct list_head		avail_defer_list;
>   	struct workqueue_struct		*work_q;
>   	struct kref			ref;
> +	struct nvmet_fc_fcp_iod		fod[];		/* array of fcp_iods */
>   } __aligned(sizeof(unsigned long long));
>   
>   struct nvmet_fc_tgt_assoc {
> @@ -588,9 +588,7 @@ nvmet_fc_alloc_target_queue(struct nvmet_fc_tgt_assoc *assoc,
>   	if (qid > NVMET_NR_QUEUES)
>   		return NULL;
>   
> -	queue = kzalloc((sizeof(*queue) +
> -				(sizeof(struct nvmet_fc_fcp_iod) * sqsize)),
> -				GFP_KERNEL);
> +	queue = kzalloc(struct_size(queue, fod, sqsize), GFP_KERNEL);
>   	if (!queue)
>   		return NULL;
>   
> @@ -603,7 +601,6 @@ nvmet_fc_alloc_target_queue(struct nvmet_fc_tgt_assoc *assoc,
>   	if (!queue->work_q)
>   		goto out_a_put;
>   
> -	queue->fod = (struct nvmet_fc_fcp_iod *)&queue[1];
>   	queue->qid = qid;
>   	queue->sqsize = sqsize;
>   	queue->assoc = assoc;

Reviewed-by:  James Smart <james.smart@broadcom.com>

I guess this is a better style.

-- james


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc()
  2019-02-23 18:51 ` Gustavo A. R. Silva
@ 2019-03-12 19:33   ` Christoph Hellwig
  -1 siblings, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2019-03-12 19:33 UTC (permalink / raw)


Thanks,

applied to nvme-5.2.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc()
@ 2019-03-12 19:33   ` Christoph Hellwig
  0 siblings, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2019-03-12 19:33 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: James Smart, Christoph Hellwig, Sagi Grimberg, linux-nvme,
	linux-kernel

Thanks,

applied to nvme-5.2.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc()
  2019-03-12 19:33   ` Christoph Hellwig
@ 2019-03-12 19:47     ` Gustavo A. R. Silva
  -1 siblings, 0 replies; 16+ messages in thread
From: Gustavo A. R. Silva @ 2019-03-12 19:47 UTC (permalink / raw)




On 3/12/19 2:33 PM, Christoph Hellwig wrote:
> Thanks,
> 
> applied to nvme-5.2.
> 

Great.

Thanks, Christoph.

--
Gustavo

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc()
@ 2019-03-12 19:47     ` Gustavo A. R. Silva
  0 siblings, 0 replies; 16+ messages in thread
From: Gustavo A. R. Silva @ 2019-03-12 19:47 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: James Smart, Sagi Grimberg, linux-nvme, linux-kernel



On 3/12/19 2:33 PM, Christoph Hellwig wrote:
> Thanks,
> 
> applied to nvme-5.2.
> 

Great.

Thanks, Christoph.

--
Gustavo

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-03-12 19:48 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-02-23 18:51 [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc() Gustavo A. R. Silva
2019-02-23 18:51 ` Gustavo A. R. Silva
2019-02-23 20:05 ` Joe Perches
2019-02-23 20:05   ` Joe Perches
2019-02-23 20:28   ` Gustavo A. R. Silva
2019-02-23 20:28     ` Gustavo A. R. Silva
2019-02-24  1:34     ` Gustavo A. R. Silva
2019-02-24  1:34       ` Gustavo A. R. Silva
2019-03-08 13:27 ` Christoph Hellwig
2019-03-08 13:27   ` Christoph Hellwig
2019-03-08 19:15 ` James Smart
2019-03-08 19:15   ` James Smart
2019-03-12 19:33 ` Christoph Hellwig
2019-03-12 19:33   ` Christoph Hellwig
2019-03-12 19:47   ` Gustavo A. R. Silva
2019-03-12 19:47     ` Gustavo A. R. Silva

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.