| Message ID | 20200917091008.2309158-1-liushixin2@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [-next] RDMA/mlx5: fix type warning of sizeof in __mlx5_ib_alloc_counters() | expand |
On Thu, Sep 17, 2020 at 05:10:08PM +0800, Liu Shixin wrote: > sizeof() when applied to a pointer typed expression should give the > size of the pointed data, even if the data is a pointer. > > Signed-off-by: Liu Shixin <liushixin2@huawei.com> > --- > drivers/infiniband/hw/mlx5/counters.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/infiniband/hw/mlx5/counters.c b/drivers/infiniband/hw/mlx5/counters.c > index 145f3cb40ccb..aeeb14ecb3ee 100644 > --- a/drivers/infiniband/hw/mlx5/counters.c > +++ b/drivers/infiniband/hw/mlx5/counters.c > @@ -456,12 +456,12 @@ static int __mlx5_ib_alloc_counters(struct mlx5_ib_dev *dev, > cnts->num_ext_ppcnt_counters = ARRAY_SIZE(ext_ppcnt_cnts); > num_counters += ARRAY_SIZE(ext_ppcnt_cnts); > } > - cnts->names = kcalloc(num_counters, sizeof(cnts->names), GFP_KERNEL); > + cnts->names = kcalloc(num_counters, sizeof(*cnts->names), GFP_KERNEL); This change is correct. > if (!cnts->names) > return -ENOMEM; > > cnts->offsets = kcalloc(num_counters, > - sizeof(cnts->offsets), GFP_KERNEL); > + sizeof(*cnts->offsets), GFP_KERNEL); This is not. > if (!cnts->offsets) > goto err_names; > > -- > 2.25.1 >
On Thu, Sep 17, 2020 at 12:08:10PM +0300, Leon Romanovsky wrote: > On Thu, Sep 17, 2020 at 05:10:08PM +0800, Liu Shixin wrote: > > sizeof() when applied to a pointer typed expression should give the > > size of the pointed data, even if the data is a pointer. > > > > Signed-off-by: Liu Shixin <liushixin2@huawei.com> Needs a fixes line > > if (!cnts->names) > > return -ENOMEM; > > > > cnts->offsets = kcalloc(num_counters, > > - sizeof(cnts->offsets), GFP_KERNEL); > > + sizeof(*cnts->offsets), GFP_KERNEL); > > This is not. Why not? Jason
On Thu, Sep 17, 2020 at 09:38:06AM -0300, Jason Gunthorpe wrote: > On Thu, Sep 17, 2020 at 12:08:10PM +0300, Leon Romanovsky wrote: > > On Thu, Sep 17, 2020 at 05:10:08PM +0800, Liu Shixin wrote: > > > sizeof() when applied to a pointer typed expression should give the > > > size of the pointed data, even if the data is a pointer. > > > > > > Signed-off-by: Liu Shixin <liushixin2@huawei.com> > > Needs a fixes line > > > > if (!cnts->names) > > > return -ENOMEM; > > > > > > cnts->offsets = kcalloc(num_counters, > > > - sizeof(cnts->offsets), GFP_KERNEL); > > > + sizeof(*cnts->offsets), GFP_KERNEL); > > > > This is not. > > Why not? cnts->offsets is array of pointers that we will set later. The "sizeof(*cnts->offsets)" will return the size of size_t, while we need to get "size_t *". Thanks > > Jason
On Thu, Sep 17, 2020 at 08:05:11PM +0300, Leon Romanovsky wrote: > On Thu, Sep 17, 2020 at 09:38:06AM -0300, Jason Gunthorpe wrote: > > On Thu, Sep 17, 2020 at 12:08:10PM +0300, Leon Romanovsky wrote: > > > On Thu, Sep 17, 2020 at 05:10:08PM +0800, Liu Shixin wrote: > > > > sizeof() when applied to a pointer typed expression should give the > > > > size of the pointed data, even if the data is a pointer. > > > > > > > > Signed-off-by: Liu Shixin <liushixin2@huawei.com> > > > > Needs a fixes line > > > > > > if (!cnts->names) > > > > return -ENOMEM; > > > > > > > > cnts->offsets = kcalloc(num_counters, > > > > - sizeof(cnts->offsets), GFP_KERNEL); > > > > + sizeof(*cnts->offsets), GFP_KERNEL); > > > > > > This is not. > > > > Why not? > > cnts->offsets is array of pointers that we will set later. > The "sizeof(*cnts->offsets)" will return the size of size_t, while we > need to get "size_t *". Then why isn't a pointer to size **? Something is rotten here Jason
On Thu, Sep 17, 2020 at 02:24:51PM -0300, Jason Gunthorpe wrote: > On Thu, Sep 17, 2020 at 08:05:11PM +0300, Leon Romanovsky wrote: > > On Thu, Sep 17, 2020 at 09:38:06AM -0300, Jason Gunthorpe wrote: > > > On Thu, Sep 17, 2020 at 12:08:10PM +0300, Leon Romanovsky wrote: > > > > On Thu, Sep 17, 2020 at 05:10:08PM +0800, Liu Shixin wrote: > > > > > sizeof() when applied to a pointer typed expression should give the > > > > > size of the pointed data, even if the data is a pointer. > > > > > > > > > > Signed-off-by: Liu Shixin <liushixin2@huawei.com> > > > > > > Needs a fixes line > > > > > > > > if (!cnts->names) > > > > > return -ENOMEM; > > > > > > > > > > cnts->offsets = kcalloc(num_counters, > > > > > - sizeof(cnts->offsets), GFP_KERNEL); > > > > > + sizeof(*cnts->offsets), GFP_KERNEL); > > > > > > > > This is not. > > > > > > Why not? > > > > cnts->offsets is array of pointers that we will set later. > > The "sizeof(*cnts->offsets)" will return the size of size_t, while we > > need to get "size_t *". > > Then why isn't a pointer to size **? > > Something is rotten here No problem, I'll check. > > Jason
On 2020/9/18 1:33, Leon Romanovsky wrote: > On Thu, Sep 17, 2020 at 02:24:51PM -0300, Jason Gunthorpe wrote: >> On Thu, Sep 17, 2020 at 08:05:11PM +0300, Leon Romanovsky wrote: >>> On Thu, Sep 17, 2020 at 09:38:06AM -0300, Jason Gunthorpe wrote: >>>> On Thu, Sep 17, 2020 at 12:08:10PM +0300, Leon Romanovsky wrote: >>>>> On Thu, Sep 17, 2020 at 05:10:08PM +0800, Liu Shixin wrote: >>>>>> sizeof() when applied to a pointer typed expression should give the >>>>>> size of the pointed data, even if the data is a pointer. >>>>>> >>>>>> Signed-off-by: Liu Shixin <liushixin2@huawei.com> >>>> Needs a fixes line >>>> >>>>>> if (!cnts->names) >>>>>> return -ENOMEM; >>>>>> >>>>>> cnts->offsets = kcalloc(num_counters, >>>>>> - sizeof(cnts->offsets), GFP_KERNEL); >>>>>> + sizeof(*cnts->offsets), GFP_KERNEL); >>>>> This is not. >>>> Why not? >>> cnts->offsets is array of pointers that we will set later. >>> The "sizeof(*cnts->offsets)" will return the size of size_t, while we >>> need to get "size_t *". >> Then why isn't a pointer to size **? >> >> Something is rotten here > No problem, I'll check. I think cnts->offsets is an array pointer whose element is size_t rathen than pointer, so the patch description does not correspond. And I think it should be modified to sizeof(*cnts->offsets) with other description. > >> Jason > . >
On Fri, Sep 18, 2020 at 11:23:18AM +0800, Liu Shixin wrote: > On 2020/9/18 1:33, Leon Romanovsky wrote: > > On Thu, Sep 17, 2020 at 02:24:51PM -0300, Jason Gunthorpe wrote: > >> On Thu, Sep 17, 2020 at 08:05:11PM +0300, Leon Romanovsky wrote: > >>> On Thu, Sep 17, 2020 at 09:38:06AM -0300, Jason Gunthorpe wrote: > >>>> On Thu, Sep 17, 2020 at 12:08:10PM +0300, Leon Romanovsky wrote: > >>>>> On Thu, Sep 17, 2020 at 05:10:08PM +0800, Liu Shixin wrote: > >>>>>> sizeof() when applied to a pointer typed expression should give the > >>>>>> size of the pointed data, even if the data is a pointer. > >>>>>> > >>>>>> Signed-off-by: Liu Shixin <liushixin2@huawei.com> > >>>> Needs a fixes line > >>>> > >>>>>> if (!cnts->names) > >>>>>> return -ENOMEM; > >>>>>> > >>>>>> cnts->offsets = kcalloc(num_counters, > >>>>>> - sizeof(cnts->offsets), GFP_KERNEL); > >>>>>> + sizeof(*cnts->offsets), GFP_KERNEL); > >>>>> This is not. > >>>> Why not? > >>> cnts->offsets is array of pointers that we will set later. > >>> The "sizeof(*cnts->offsets)" will return the size of size_t, while we > >>> need to get "size_t *". > >> Then why isn't a pointer to size **? > >> > >> Something is rotten here > > No problem, I'll check. > I think cnts->offsets is an array pointer whose element is size_t rathen than pointer, > so the patch description does not correspond. > And I think it should be modified to sizeof(*cnts->offsets) with other description. Sorry for me being wrong, you are right. Thanks > > > >> Jason > > . > > >
diff --git a/drivers/infiniband/hw/mlx5/counters.c b/drivers/infiniband/hw/mlx5/counters.c index 145f3cb40ccb..aeeb14ecb3ee 100644 --- a/drivers/infiniband/hw/mlx5/counters.c +++ b/drivers/infiniband/hw/mlx5/counters.c @@ -456,12 +456,12 @@ static int __mlx5_ib_alloc_counters(struct mlx5_ib_dev *dev, cnts->num_ext_ppcnt_counters = ARRAY_SIZE(ext_ppcnt_cnts); num_counters += ARRAY_SIZE(ext_ppcnt_cnts); } - cnts->names = kcalloc(num_counters, sizeof(cnts->names), GFP_KERNEL); + cnts->names = kcalloc(num_counters, sizeof(*cnts->names), GFP_KERNEL); if (!cnts->names) return -ENOMEM; cnts->offsets = kcalloc(num_counters, - sizeof(cnts->offsets), GFP_KERNEL); + sizeof(*cnts->offsets), GFP_KERNEL); if (!cnts->offsets) goto err_names;
sizeof() when applied to a pointer typed expression should give the size of the pointed data, even if the data is a pointer. Signed-off-by: Liu Shixin <liushixin2@huawei.com> --- drivers/infiniband/hw/mlx5/counters.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)