Re: [PATCH] staging: greybus: power_supply: Use struct_size() helper
Johan, On 4/4/19 2:24 AM, Johan Hovold wrote: > On Thu, Apr 04, 2019 at 08:09:51AM +0100, Rui Miguel Silva wrote: >> Hi Gustavo, >> Thanks a lot for the patch. >> >> On Wed 03 Apr 2019 at 21:58, Gustavo A. R. Silva wrote: >>> Make use of the struct_size() helper instead of an open-coded >>> version >>> in order to avoid any potential type mistakes, in particular in >>> the >>> context in which this code is being used. >>> >>> So, replace code of the following form: >>> >>> sizeof(*resp) + props_count * sizeof(struct >>> gb_power_supply_props_desc) >>> >>> with: >>> >>> struct_size(resp, props, props_count) >>> >>> This code was detected with the help of Coccinelle. >>> >>> Signed-off-by: Gustavo A. R. Silva >> >> What are the odds of 2 people changing same code in greybus in >> the same day :). > > Well, I only noticed the bug in the current code, when reviewing > Gustavo's diff. ;) > Apparently, your patch hasn't been applied to any tree yet. So, I'll wait for it to be applied before sending v2. Thanks -- Gustavo ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: greybus: power_supply: Use struct_size() helper
Hi Johan, On 4/4/19 1:57 AM, Johan Hovold wrote: > > This patch looks good, but I noticed a bug here in the current code, > which should be fixed before applying this clean up. > > sizeof(req) should have been sizeof(*req) above. > Good catch. >> - sizeof(struct gb_power_supply_props_desc), >> + sizeof(req), >> + struct_size(resp, props, props_count), >> GFP_KERNEL); >> if (!op) >> return -ENOMEM; > > I've just submitted a fix (and CCed you as well). > > Would you mind respinning on top of that one? > Yep. I'll send v2 shortly. Thanks -- Gustavo ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: greybus: power_supply: Use struct_size() helper
On Thu, Apr 04, 2019 at 08:09:51AM +0100, Rui Miguel Silva wrote: > Hi Gustavo, > Thanks a lot for the patch. > > On Wed 03 Apr 2019 at 21:58, Gustavo A. R. Silva wrote: > > Make use of the struct_size() helper instead of an open-coded > > version > > in order to avoid any potential type mistakes, in particular in > > the > > context in which this code is being used. > > > > So, replace code of the following form: > > > > sizeof(*resp) + props_count * sizeof(struct > > gb_power_supply_props_desc) > > > > with: > > > > struct_size(resp, props, props_count) > > > > This code was detected with the help of Coccinelle. > > > > Signed-off-by: Gustavo A. R. Silva > > What are the odds of 2 people changing same code in greybus in > the same day :). Well, I only noticed the bug in the current code, when reviewing Gustavo's diff. ;) Johan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: greybus: power_supply: Use struct_size() helper
Hi Gustavo, Thanks a lot for the patch. On Wed 03 Apr 2019 at 21:58, Gustavo A. R. Silva wrote: Make use of the struct_size() helper instead of an open-coded version in order to avoid any potential type mistakes, in particular in the context in which this code is being used. So, replace code of the following form: sizeof(*resp) + props_count * sizeof(struct gb_power_supply_props_desc) with: struct_size(resp, props, props_count) This code was detected with the help of Coccinelle. Signed-off-by: Gustavo A. R. Silva What are the odds of 2 people changing same code in greybus in the same day :). But it happened, so as Johan asked please rebase on top of his patch. that would be great. --- Cheers, Rui --- drivers/staging/greybus/power_supply.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/greybus/power_supply.c b/drivers/staging/greybus/power_supply.c index 0529e5628c24..40cc2d462ba0 100644 --- a/drivers/staging/greybus/power_supply.c +++ b/drivers/staging/greybus/power_supply.c @@ -520,8 +520,8 @@ static int gb_power_supply_prop_descriptors_get(struct gb_power_supply *gbpsy) op = gb_operation_create(connection, GB_POWER_SUPPLY_TYPE_GET_PROP_DESCRIPTORS, - sizeof(req), sizeof(*resp) + props_count * - sizeof(struct gb_power_supply_props_desc), +sizeof(req), + struct_size(resp, props, props_count), GFP_KERNEL); if (!op) return -ENOMEM; ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: greybus: power_supply: Use struct_size() helper
On Wed, Apr 03, 2019 at 03:58:01PM -0500, Gustavo A. R. Silva wrote: > Make use of the struct_size() helper instead of an open-coded version > in order to avoid any potential type mistakes, in particular in the > context in which this code is being used. > > So, replace code of the following form: > > sizeof(*resp) + props_count * sizeof(struct gb_power_supply_props_desc) > > with: > > struct_size(resp, props, props_count) > > This code was detected with the help of Coccinelle. > > Signed-off-by: Gustavo A. R. Silva > --- > drivers/staging/greybus/power_supply.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/greybus/power_supply.c > b/drivers/staging/greybus/power_supply.c > index 0529e5628c24..40cc2d462ba0 100644 > --- a/drivers/staging/greybus/power_supply.c > +++ b/drivers/staging/greybus/power_supply.c > @@ -520,8 +520,8 @@ static int gb_power_supply_prop_descriptors_get(struct > gb_power_supply *gbpsy) > > op = gb_operation_create(connection, >GB_POWER_SUPPLY_TYPE_GET_PROP_DESCRIPTORS, > - sizeof(req), sizeof(*resp) + props_count * This patch looks good, but I noticed a bug here in the current code, which should be fixed before applying this clean up. sizeof(req) should have been sizeof(*req) above. > - sizeof(struct gb_power_supply_props_desc), > + sizeof(req), > + struct_size(resp, props, props_count), >GFP_KERNEL); > if (!op) > return -ENOMEM; I've just submitted a fix (and CCed you as well). Would you mind respinning on top of that one? Thanks, Johan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: greybus: power_supply: Use struct_size() helper
Make use of the struct_size() helper instead of an open-coded version in order to avoid any potential type mistakes, in particular in the context in which this code is being used. So, replace code of the following form: sizeof(*resp) + props_count * sizeof(struct gb_power_supply_props_desc) with: struct_size(resp, props, props_count) This code was detected with the help of Coccinelle. Signed-off-by: Gustavo A. R. Silva --- drivers/staging/greybus/power_supply.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/greybus/power_supply.c b/drivers/staging/greybus/power_supply.c index 0529e5628c24..40cc2d462ba0 100644 --- a/drivers/staging/greybus/power_supply.c +++ b/drivers/staging/greybus/power_supply.c @@ -520,8 +520,8 @@ static int gb_power_supply_prop_descriptors_get(struct gb_power_supply *gbpsy) op = gb_operation_create(connection, GB_POWER_SUPPLY_TYPE_GET_PROP_DESCRIPTORS, -sizeof(req), sizeof(*resp) + props_count * -sizeof(struct gb_power_supply_props_desc), +sizeof(req), +struct_size(resp, props, props_count), GFP_KERNEL); if (!op) return -ENOMEM; -- 2.21.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
