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 = kmalloc(sizeof(struct foo) + count * sizeof(struct boo), GFP_KERNEL);
Instead of leaving these open-coded and prone to type mistakes, we can
now use the new struct_size() helper:
instance = kmalloc(struct_size(instance, entry, count), GFP_KERNEL);
Signed-off-by: xiaolinkui <[email protected]>
---
block/bio.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/block/bio.c b/block/bio.c
index 683cbb4..847ac60 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -436,9 +436,7 @@ struct bio *bio_alloc_bioset(gfp_t gfp_mask, unsigned int nr_iovecs,
if (nr_iovecs > UIO_MAXIOV)
return NULL;
- p = kmalloc(sizeof(struct bio) +
- nr_iovecs * sizeof(struct bio_vec),
- gfp_mask);
+ p = kmalloc(struct_size(bio, bi_io_vec, nr_iovecs), gfp_mask);
front_pad = 0;
inline_vecs = nr_iovecs;
} else {
@@ -1120,8 +1118,7 @@ static struct bio_map_data *bio_alloc_map_data(struct iov_iter *data,
if (data->nr_segs > UIO_MAXIOV)
return NULL;
- bmd = kmalloc(sizeof(struct bio_map_data) +
- sizeof(struct iovec) * data->nr_segs, gfp_mask);
+ bmd = kmalloc(struct_size(bmd, iov, data->nr_segs), gfp_mask);
if (!bmd)
return NULL;
memcpy(bmd->iov, data->iov, sizeof(struct iovec) * data->nr_segs);
--
2.7.4
Looks good.
Reviewed-by: Chaitanya Kulkarni <[email protected]>
On 5/17/19 2:17 AM, xiaolinkui wrote:
> 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 = kmalloc(sizeof(struct foo) + count * sizeof(struct boo), GFP_KERNEL);
>
> Instead of leaving these open-coded and prone to type mistakes, we can
> now use the new struct_size() helper:
>
> instance = kmalloc(struct_size(instance, entry, count), GFP_KERNEL);
>
> Signed-off-by: xiaolinkui <[email protected]>
> ---
> block/bio.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/block/bio.c b/block/bio.c
> index 683cbb4..847ac60 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -436,9 +436,7 @@ struct bio *bio_alloc_bioset(gfp_t gfp_mask, unsigned int nr_iovecs,
> if (nr_iovecs > UIO_MAXIOV)
> return NULL;
>
> - p = kmalloc(sizeof(struct bio) +
> - nr_iovecs * sizeof(struct bio_vec),
> - gfp_mask);
> + p = kmalloc(struct_size(bio, bi_io_vec, nr_iovecs), gfp_mask);
> front_pad = 0;
> inline_vecs = nr_iovecs;
> } else {
> @@ -1120,8 +1118,7 @@ static struct bio_map_data *bio_alloc_map_data(struct iov_iter *data,
> if (data->nr_segs > UIO_MAXIOV)
> return NULL;
>
> - bmd = kmalloc(sizeof(struct bio_map_data) +
> - sizeof(struct iovec) * data->nr_segs, gfp_mask);
> + bmd = kmalloc(struct_size(bmd, iov, data->nr_segs), gfp_mask);
> if (!bmd)
> return NULL;
> memcpy(bmd->iov, data->iov, sizeof(struct iovec) * data->nr_segs);
On 5/17/19 3:12 AM, xiaolinkui wrote:
> 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 = kmalloc(sizeof(struct foo) + count * sizeof(struct boo), GFP_KERNEL);
>
> Instead of leaving these open-coded and prone to type mistakes, we can
> now use the new struct_size() helper:
>
> instance = kmalloc(struct_size(instance, entry, count), GFP_KERNEL);
Applied, thanks.
--
Jens Axboe
On 5/17/19 3:17 PM, Jens Axboe wrote:
> On 5/17/19 3:12 AM, xiaolinkui wrote:
>> 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 = kmalloc(sizeof(struct foo) + count * sizeof(struct boo), GFP_KERNEL);
>>
>> Instead of leaving these open-coded and prone to type mistakes, we can
>> now use the new struct_size() helper:
>>
>> instance = kmalloc(struct_size(instance, entry, count), GFP_KERNEL);
>
> Applied, thanks.
I take that back, you obviously didn't even compile this patch. Never
send untested crap, without explicitly saying so.
--
Jens Axboe
- [email protected] <[email protected]> to reduce
the noise.
I apologies Jens, I didn't apply and tested these patches before
submitting the
review and assumed that patches are compiled and tested, I'll do so for each
patch before submitting the review.
Xiaolinkui,
Please send compiled and tested patch only on the latest kernel on the
appropriate subsystem, otherwise mark the patch appropriately
[RFC/Compile only] so reviewer would know without such a tag
it is easy to assume that patch is compiled and tested.
You have also sent out the couple of more patches with this fix.
If they are not compiled and tested with right kernel branch for each
subsystem, please update the appropriate mail thread either to ignore those
patches (if they have compilation problem on appropriate branch) or mark
them compile test only (this needs to be avoided for these patches), in
either
case please send updated patches for this fix if needed.
Thanks.
On 5/17/19 3:59 PM, Jens Axboe wrote:
> On 5/17/19 3:17 PM, Jens Axboe wrote:
>> On 5/17/19 3:12 AM, xiaolinkui wrote:
>>> 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 = kmalloc(sizeof(struct foo) + count * sizeof(struct boo), GFP_KERNEL);
>>>
>>> Instead of leaving these open-coded and prone to type mistakes, we can
>>> now use the new struct_size() helper:
>>>
>>> instance = kmalloc(struct_size(instance, entry, count), GFP_KERNEL);
>> Applied, thanks.
> I take that back, you obviously didn't even compile this patch. Never
> send untested crap, without explicitly saying so.
>
On 5/17/19 6:43 PM, Chaitanya Kulkarni wrote:
> - [email protected] <[email protected]> to reduce
> the noise.
>
> I apologies Jens, I didn't apply and tested these patches before
> submitting the review and assumed that patches are compiled and
> tested, I'll do so for each patch before submitting the review.
Just to be clear, I'm not placing any blame on you. It's easy to miss
that kind of thing in a review. The onus is on the submitter to ensure
that anything he/she sends in has been both compile and runtime tested.
> Xiaolinkui,
>
> Please send compiled and tested patch only on the latest kernel on the
> appropriate subsystem, otherwise mark the patch appropriately
> [RFC/Compile only] so reviewer would know without such a tag
> it is easy to assume that patch is compiled and tested.
>
> You have also sent out the couple of more patches with this fix.
>
> If they are not compiled and tested with right kernel branch for each
> subsystem, please update the appropriate mail thread either to ignore those
> patches (if they have compilation problem on appropriate branch) or mark
> them compile test only (this needs to be avoided for these patches), in
> either
> case please send updated patches for this fix if needed.
This is solid advice. Sending out untested patches without EXPLICITLY
saying so is reckless and irresponsible, and causes harm to your
reputation as well. Trust is an important part of being successful in an
open source project.
--
Jens Axboe