All of lore.kernel.org
 help / color / mirror / Atom feed
From: hch@lst.de (Christoph Hellwig)
Subject: [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc()
Date: Fri, 8 Mar 2019 14:27:27 +0100	[thread overview]
Message-ID: <20190308132727.GC18218@lst.de> (raw)
In-Reply-To: <20190223185108.GA6706@embeddedor>

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---

WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@lst.de>
To: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
Cc: James Smart <james.smart@broadcom.com>,
	Christoph Hellwig <hch@lst.de>, Sagi Grimberg <sagi@grimberg.me>,
	linux-nvme@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] nvmet-fc: use zero-sized array and struct_size() in kzalloc()
Date: Fri, 8 Mar 2019 14:27:27 +0100	[thread overview]
Message-ID: <20190308132727.GC18218@lst.de> (raw)
In-Reply-To: <20190223185108.GA6706@embeddedor>

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---

  parent reply	other threads:[~2019-03-08 13:27 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190308132727.GC18218@lst.de \
    --to=hch@lst.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.