| Message ID | 20250203080902.1864382-15-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:56PM +0530, Raag Jadav wrote: > Convert to use devm_kmemdup_array() which is more robust. ... > - channels = devm_kmemdup(dev, channel_templates, > - sizeof(channels[0]) * max_channels, GFP_KERNEL); > + channels = devm_kmemdup_array(dev, channel_templates, max_channels, > + sizeof(channels[0]), GFP_KERNEL); I would use more regular sizeof(*channels) > if (!channels) > return -ENOMEM; P.S. For all sizeof() replacements the changes would probably need the updated commit messages.
On Mon, Feb 03, 2025 at 11:54:42AM +0200, Andy Shevchenko wrote: > On Mon, Feb 03, 2025 at 01:38:56PM +0530, Raag Jadav wrote: > > Convert to use devm_kmemdup_array() which is more robust. > > ... > > > - channels = devm_kmemdup(dev, channel_templates, > > - sizeof(channels[0]) * max_channels, GFP_KERNEL); > > + channels = devm_kmemdup_array(dev, channel_templates, max_channels, > > + sizeof(channels[0]), GFP_KERNEL); > > I would use more regular sizeof(*channels) This might get confusing since we're assigning it back to channels. Raag
On Mon, 3 Feb 2025 12:42:36 +0200 Raag Jadav <raag.jadav@intel.com> wrote: > On Mon, Feb 03, 2025 at 11:54:42AM +0200, Andy Shevchenko wrote: > > On Mon, Feb 03, 2025 at 01:38:56PM +0530, Raag Jadav wrote: > > > Convert to use devm_kmemdup_array() which is more robust. > > > > ... > > > > > - channels = devm_kmemdup(dev, channel_templates, > > > - sizeof(channels[0]) * max_channels, GFP_KERNEL); > > > + channels = devm_kmemdup_array(dev, channel_templates, max_channels, > > > + sizeof(channels[0]), GFP_KERNEL); > > > > I would use more regular sizeof(*channels) > > This might get confusing since we're assigning it back to channels. > It looks like standard pattern. Assign X * the thing to the thing. So I'd prefer sizeof(*channels) here as well. > Raag > >
On Mon, Feb 03, 2025 at 12:42:36PM +0200, Raag Jadav wrote: > On Mon, Feb 03, 2025 at 11:54:42AM +0200, Andy Shevchenko wrote: > > On Mon, Feb 03, 2025 at 01:38:56PM +0530, Raag Jadav wrote: > > > Convert to use devm_kmemdup_array() which is more robust. > > > > ... > > > > > - channels = devm_kmemdup(dev, channel_templates, > > > - sizeof(channels[0]) * max_channels, GFP_KERNEL); > > > + channels = devm_kmemdup_array(dev, channel_templates, max_channels, > > > + sizeof(channels[0]), GFP_KERNEL); > > > > I would use more regular sizeof(*channels) > > This might get confusing since we're assigning it back to channels. Then it should be sizeof(*channel_templates) ? Now since you mention this, in all other suggested cases it may need to be carefully chosen.
On Mon, Feb 03, 2025 at 11:14:58AM +0000, Jonathan Cameron wrote: > On Mon, 3 Feb 2025 12:42:36 +0200 > Raag Jadav <raag.jadav@intel.com> wrote: > > On Mon, Feb 03, 2025 at 11:54:42AM +0200, Andy Shevchenko wrote: > > > On Mon, Feb 03, 2025 at 01:38:56PM +0530, Raag Jadav wrote: > > > > Convert to use devm_kmemdup_array() which is more robust. ... > > > > - channels = devm_kmemdup(dev, channel_templates, > > > > - sizeof(channels[0]) * max_channels, GFP_KERNEL); > > > > + channels = devm_kmemdup_array(dev, channel_templates, max_channels, > > > > + sizeof(channels[0]), GFP_KERNEL); > > > > > > I would use more regular sizeof(*channels) > > > > This might get confusing since we're assigning it back to channels. > > > It looks like standard pattern. Assign X * the thing to the thing. > > So I'd prefer sizeof(*channels) here as well. Yeah, but the problem with kmemdup() family of APIs is that we need to pass the source size if we want to have 1:1 copy. So, sizeof(*channel_templates) is more accurate in my opinion.
diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c index cfbfcaefec0f..4a47d1ded64a 100644 --- a/drivers/iio/adc/xilinx-xadc-core.c +++ b/drivers/iio/adc/xilinx-xadc-core.c @@ -1245,8 +1245,8 @@ static int xadc_parse_dt(struct iio_dev *indio_dev, unsigned int *conf, int irq) channel_templates = xadc_us_channels; max_channels = ARRAY_SIZE(xadc_us_channels); } - channels = devm_kmemdup(dev, channel_templates, - sizeof(channels[0]) * max_channels, GFP_KERNEL); + channels = devm_kmemdup_array(dev, channel_templates, max_channels, + sizeof(channels[0]), GFP_KERNEL); if (!channels) return -ENOMEM;
Convert to use devm_kmemdup_array() which is more robust. Signed-off-by: Raag Jadav <raag.jadav@intel.com> --- drivers/iio/adc/xilinx-xadc-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)