| Message ID | 20190131003859.GA28539@embeddedor |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net-next] nfp: use struct_size() in kzalloc() | expand |
On Wed, 30 Jan 2019 18:38:59 -0600, Gustavo A. R. Silva 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 = kzalloc(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 = 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> Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
On 1/30/19 6:52 PM, Jakub Kicinski wrote: > On Wed, 30 Jan 2019 18:38:59 -0600, Gustavo A. R. Silva 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 = kzalloc(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 = 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> > > Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com> > Thanks, Jakub. -- Gustavo
On Wed, 2019-01-30 at 18:38 -0600, Gustavo A. R. Silva 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 = kzalloc(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 = kzalloc(struct_size(instance, entry, count), GFP_KERNEL); > > This code was detected with the help of Coccinelle. Might be useful to augment the script to include cases where the computed size is saved to a temporary and that temporary is used ala: https://patchwork.kernel.org/patch/10782453/ On Sat, 2019-01-26 at 20:42 +0800, YueHaibing wrote: > Use kmemdup rather than duplicating its implementation [] > diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c b/drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c [] > @@ -1196,13 +1196,9 @@ iwl_parse_nvm_mcc_info(struct device *dev, const struct iwl_cfg *cfg, > regd_to_copy = sizeof(struct ieee80211_regdomain) + > valid_rules * sizeof(struct ieee80211_reg_rule); > - copy_rd = kzalloc(regd_to_copy, GFP_KERNEL); > - if (!copy_rd) { > + copy_rd = kmemdup(regd, regd_to_copy, GFP_KERNEL); This should probably be copy_rd = kmemdup(regd, struct_size(regd, reg_rules, valid_rules), GFP_KERNEL);
Hi Joe, On 1/31/19 11:11 AM, Joe Perches wrote: > On Wed, 2019-01-30 at 18:38 -0600, Gustavo A. R. Silva 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 = kzalloc(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 = kzalloc(struct_size(instance, entry, count), GFP_KERNEL); >> >> This code was detected with the help of Coccinelle. > > Might be useful to augment the script to include cases > where the computed size is saved to a temporary and > that temporary is used ala: > Yep. That's already on my list. > https://patchwork.kernel.org/patch/10782453/ > > On Sat, 2019-01-26 at 20:42 +0800, YueHaibing wrote: >> Use kmemdup rather than duplicating its implementation > [] >> diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c b/drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c > [] >> @@ -1196,13 +1196,9 @@ iwl_parse_nvm_mcc_info(struct device *dev, const struct iwl_cfg *cfg, >> regd_to_copy = sizeof(struct ieee80211_regdomain) + >> valid_rules * sizeof(struct ieee80211_reg_rule); >> - copy_rd = kzalloc(regd_to_copy, GFP_KERNEL); >> - if (!copy_rd) { >> + copy_rd = kmemdup(regd, regd_to_copy, GFP_KERNEL); > > This should probably be > > copy_rd = kmemdup(regd, struct_size(regd, reg_rules, valid_rules), > GFP_KERNEL); > I agree. Thanks -- Gustavo
From: "Gustavo A. R. Silva" <gustavo@embeddedor.com> Date: Wed, 30 Jan 2019 18:38:59 -0600 > 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) + 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 = 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> Applied.
diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c index 802c9224bb32..f6f028fa5db9 100644 --- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c +++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c @@ -269,8 +269,7 @@ __nfp_eth_read_ports(struct nfp_cpp *cpp, struct nfp_nsp *nsp) goto err; } - table = kzalloc(sizeof(*table) + - sizeof(struct nfp_eth_table_port) * cnt, GFP_KERNEL); + table = kzalloc(struct_size(table, ports, cnt), GFP_KERNEL); if (!table) goto err;
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) + 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 = 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/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)