| Message ID | 20250203080902.1864382-13-raag.jadav@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Split devres APIs to device/devres.h and introduce devm_kmemdup_array() | expand |
On Mon, Feb 03, 2025 at 01:38:54PM +0530, Raag Jadav wrote: > Convert to use devm_kmemdup_array() which is more robust. ... > data->voltages_mV = > - devm_kmemdup(dev, resp.voltages_mv, > - sizeof(u16) * data->num_voltages, GFP_KERNEL); > + devm_kmemdup_array(dev, resp.voltages_mv, data->num_voltages, > + sizeof(u16), GFP_KERNEL); Wondering if this can be sizeof(*data->voltages_mV) that makes code robust against type changes.
On Mon, Feb 03, 2025 at 11:51:10AM +0200, Andy Shevchenko wrote: > On Mon, Feb 03, 2025 at 01:38:54PM +0530, Raag Jadav wrote: > > Convert to use devm_kmemdup_array() which is more robust. > > ... > > > data->voltages_mV = > > - devm_kmemdup(dev, resp.voltages_mv, > > - sizeof(u16) * data->num_voltages, GFP_KERNEL); > > + devm_kmemdup_array(dev, resp.voltages_mv, data->num_voltages, > > + sizeof(u16), GFP_KERNEL); > > Wondering if this can be sizeof(*data->voltages_mV) that makes code robust > against type changes. True, but I opted for a blind treewide conversion that is consistent with existing driver conventions. Perhaps a better place for it is a separate filewide series? Raag
On Mon, Feb 03, 2025 at 12:40:23PM +0200, Raag Jadav wrote: > On Mon, Feb 03, 2025 at 11:51:10AM +0200, Andy Shevchenko wrote: > > On Mon, Feb 03, 2025 at 01:38:54PM +0530, Raag Jadav wrote: > > > Convert to use devm_kmemdup_array() which is more robust. ... > > > data->voltages_mV = > > > - devm_kmemdup(dev, resp.voltages_mv, > > > - sizeof(u16) * data->num_voltages, GFP_KERNEL); > > > + devm_kmemdup_array(dev, resp.voltages_mv, data->num_voltages, > > > + sizeof(u16), GFP_KERNEL); > > > > Wondering if this can be sizeof(*data->voltages_mV) that makes code robust > > against type changes. > > True, but I opted for a blind treewide conversion that is consistent with > existing driver conventions. Perhaps a better place for it is a separate > filewide series? I think an additional series is just an increased churn. In this you are changing an API in use, it's completely fine to update a parameter in accordance with new API. I.o.w. I consider these two are coupled enough to be semantically and logically in a single change along with the fact that in all such case you are touching the same line(s).
diff --git a/drivers/regulator/cros-ec-regulator.c b/drivers/regulator/cros-ec-regulator.c index fb9643ed7a49..38afd4b3d2d0 100644 --- a/drivers/regulator/cros-ec-regulator.c +++ b/drivers/regulator/cros-ec-regulator.c @@ -138,8 +138,8 @@ static int cros_ec_regulator_init_info(struct device *dev, data->num_voltages = min_t(u16, ARRAY_SIZE(resp.voltages_mv), resp.num_voltages); data->voltages_mV = - devm_kmemdup(dev, resp.voltages_mv, - sizeof(u16) * data->num_voltages, GFP_KERNEL); + devm_kmemdup_array(dev, resp.voltages_mv, data->num_voltages, + sizeof(u16), GFP_KERNEL); if (!data->voltages_mV) return -ENOMEM;
Convert to use devm_kmemdup_array() which is more robust. Signed-off-by: Raag Jadav <raag.jadav@intel.com> --- drivers/regulator/cros-ec-regulator.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)