Re: [PATCH v2 4/5] extcon: Use sizeof(*pointer) instead of sizeof(type)

From: Chanwoo Choi
Date: Thu Apr 06 2023 - 15:33:54 EST


On 23. 4. 6. 00:27, Andy Shevchenko wrote:
> It is preferred to use sizeof(*pointer) instead of sizeof(type).
> The type of the variable can change and one needs not change
> the former (unlike the latter). No functional change intended.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> Acked-by: Bumwoo Lee <bw365.lee@xxxxxxxxxxx>
> ---
> drivers/extcon/extcon.c | 15 ++++++---------
> 1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index daaded92cf80..50c5fd454488 100644
> --- a/drivers/extcon/extcon.c
> +++ b/drivers/extcon/extcon.c
> @@ -1098,8 +1098,7 @@ static int extcon_alloc_cables(struct extcon_dev *edev)
> if (!edev->max_supported)
> return 0;
>
> - edev->cables = kcalloc(edev->max_supported,
> - sizeof(struct extcon_cable),
> + edev->cables = kcalloc(edev->max_supported, sizeof(*edev->cables),
> GFP_KERNEL);
> if (!edev->cables)
> return -ENOMEM;
> @@ -1161,14 +1160,12 @@ static int extcon_alloc_muex(struct extcon_dev *edev)
> for (index = 0; edev->mutually_exclusive[index]; index++)
> ;
>
> - edev->attrs_muex = kcalloc(index + 1,
> - sizeof(struct attribute *),
> + edev->attrs_muex = kcalloc(index + 1, sizeof(*edev->attrs_muex),
> GFP_KERNEL);
> if (!edev->attrs_muex)
> return -ENOMEM;
>
> - edev->d_attrs_muex = kcalloc(index,
> - sizeof(struct device_attribute),
> + edev->d_attrs_muex = kcalloc(index, sizeof(*edev->d_attrs_muex),
> GFP_KERNEL);
> if (!edev->d_attrs_muex) {
> kfree(edev->attrs_muex);
> @@ -1214,8 +1211,8 @@ static int extcon_alloc_groups(struct extcon_dev *edev)
> return 0;
>
> edev->extcon_dev_type.groups = kcalloc(edev->max_supported + 2,
> - sizeof(struct attribute_group *),
> - GFP_KERNEL);
> + sizeof(*edev->extcon_dev_type.groups),
> + GFP_KERNEL);
> if (!edev->extcon_dev_type.groups)
> return -ENOMEM;
>
> @@ -1293,7 +1290,7 @@ int extcon_dev_register(struct extcon_dev *edev)
> spin_lock_init(&edev->lock);
> if (edev->max_supported) {
> edev->nh = kcalloc(edev->max_supported, sizeof(*edev->nh),
> - GFP_KERNEL);
> + GFP_KERNEL);

Looks good to me.

But, actually, this line change is not related to role of this patch.
If possible, we have to keep the only one role on one patch. Don't touch it.

--
Best Regards,
Samsung Electronics
Chanwoo Choi