Re: [PATCH] drm/nouveau/iccsense:

[Karol Herbst]

On Thu, Jul 13, 2023 at 11:13 AM Karol Herbst  wrote:
>
> On Mon, Jul 10, 2023 at 11:10 AM Bagas Sanjaya  wrote:
> >
> > On Mon, Jul 10, 2023 at 03:06:47PM +0800, sunran...@208suo.com wrote:
> > > Fixed error: 'do not use assignment in if condition'
> > >
> > > This patch fixes error: 'do not use assignment in if condition'
> > > in drm/nouveau/iccsense
> >
> > I guess this is checkpatch fix, right?
> >
> > >
> > > Signed-off-by: Ran Sun 
> > > ---
> > >  drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
> > > b/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
> > > index 8f0ccd3664eb..2428f3d6e477 100644
> > > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
> > > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
> > > @@ -322,7 +322,8 @@ int
> > >  nvkm_iccsense_new_(struct nvkm_device *device, enum nvkm_subdev_type 
> > > type,
> > > int inst,
> > > struct nvkm_iccsense **iccsense)
> > >  {
> > > -if (!(*iccsense = kzalloc(sizeof(**iccsense), GFP_KERNEL)))
> > > +*iccsense = kzalloc(sizeof(**iccsense), GFP_KERNEL);
> > > +if (!*iccsense)
> > >  return -ENOMEM;
> > >  INIT_LIST_HEAD(&(*iccsense)->sensors);
> > >  INIT_LIST_HEAD(&(*iccsense)->rails);
> >
> > Your patch is corrupted (tabs converted to spaces) because you're using
> > Roundcube. Please use git-send-mail(1) instead.
> >
>
> oh right, I didn't notice with the patches I reviewed already here.
> Yeah, none of them apply, please use git to send those patches.
>

also please send them all together in a single series the next time.

> > Thanks.
> >
> > --
> > An old man doll... just what I always wanted! - Clara

Re: [PATCH] drm/nouveau/iccsense:

[Karol Herbst]

On Mon, Jul 10, 2023 at 11:10 AM Bagas Sanjaya  wrote:
>
> On Mon, Jul 10, 2023 at 03:06:47PM +0800, sunran...@208suo.com wrote:
> > Fixed error: 'do not use assignment in if condition'
> >
> > This patch fixes error: 'do not use assignment in if condition'
> > in drm/nouveau/iccsense
>
> I guess this is checkpatch fix, right?
>
> >
> > Signed-off-by: Ran Sun 
> > ---
> >  drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
> > b/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
> > index 8f0ccd3664eb..2428f3d6e477 100644
> > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
> > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
> > @@ -322,7 +322,8 @@ int
> >  nvkm_iccsense_new_(struct nvkm_device *device, enum nvkm_subdev_type type,
> > int inst,
> > struct nvkm_iccsense **iccsense)
> >  {
> > -if (!(*iccsense = kzalloc(sizeof(**iccsense), GFP_KERNEL)))
> > +*iccsense = kzalloc(sizeof(**iccsense), GFP_KERNEL);
> > +if (!*iccsense)
> >  return -ENOMEM;
> >  INIT_LIST_HEAD(&(*iccsense)->sensors);
> >  INIT_LIST_HEAD(&(*iccsense)->rails);
>
> Your patch is corrupted (tabs converted to spaces) because you're using
> Roundcube. Please use git-send-mail(1) instead.
>

oh right, I didn't notice with the patches I reviewed already here.
Yeah, none of them apply, please use git to send those patches.

> Thanks.
>
> --
> An old man doll... just what I always wanted! - Clara

[PATCH] drm/nouveau/iccsense: do not use assignment in if condition

[sunran001]

checkpatch.pl does not like assignment in if condition

Signed-off-by: Ran Sun 
---
 drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c 
b/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c

index 8f0ccd3664eb..2428f3d6e477 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
@@ -322,7 +322,8 @@ int
 nvkm_iccsense_new_(struct nvkm_device *device, enum nvkm_subdev_type 
type, int inst,

struct nvkm_iccsense **iccsense)
 {
-if (!(*iccsense = kzalloc(sizeof(**iccsense), GFP_KERNEL)))
+*iccsense = kzalloc(sizeof(**iccsense), GFP_KERNEL);
+if (!*iccsense)
 return -ENOMEM;
 INIT_LIST_HEAD(&(*iccsense)->sensors);
 INIT_LIST_HEAD(&(*iccsense)->rails);

Re: [PATCH] drm/nouveau/iccsense:

[Bagas Sanjaya]

On Mon, Jul 10, 2023 at 03:06:47PM +0800, sunran...@208suo.com wrote:
> Fixed error: 'do not use assignment in if condition'
> 
> This patch fixes error: 'do not use assignment in if condition'
> in drm/nouveau/iccsense

I guess this is checkpatch fix, right?

> 
> Signed-off-by: Ran Sun 
> ---
>  drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
> b/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
> index 8f0ccd3664eb..2428f3d6e477 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
> @@ -322,7 +322,8 @@ int
>  nvkm_iccsense_new_(struct nvkm_device *device, enum nvkm_subdev_type type,
> int inst,
> struct nvkm_iccsense **iccsense)
>  {
> -if (!(*iccsense = kzalloc(sizeof(**iccsense), GFP_KERNEL)))
> +*iccsense = kzalloc(sizeof(**iccsense), GFP_KERNEL);
> +if (!*iccsense)
>  return -ENOMEM;
>  INIT_LIST_HEAD(&(*iccsense)->sensors);
>  INIT_LIST_HEAD(&(*iccsense)->rails);

Your patch is corrupted (tabs converted to spaces) because you're using
Roundcube. Please use git-send-mail(1) instead.

Thanks.

-- 
An old man doll... just what I always wanted! - Clara


signature.asc
Description: PGP signature

[PATCH] drm/nouveau/iccsense:

[sunran001]

Fixed error: 'do not use assignment in if condition'

This patch fixes error: 'do not use assignment in if condition'
in drm/nouveau/iccsense

Signed-off-by: Ran Sun 
---
 drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c 
b/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c

index 8f0ccd3664eb..2428f3d6e477 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
@@ -322,7 +322,8 @@ int
 nvkm_iccsense_new_(struct nvkm_device *device, enum nvkm_subdev_type 
type, int inst,

struct nvkm_iccsense **iccsense)
 {
-if (!(*iccsense = kzalloc(sizeof(**iccsense), GFP_KERNEL)))
+*iccsense = kzalloc(sizeof(**iccsense), GFP_KERNEL);
+if (!*iccsense)
 return -ENOMEM;
 INIT_LIST_HEAD(&(*iccsense)->sensors);
 INIT_LIST_HEAD(&(*iccsense)->rails);

[PATCH] drm/nouveau/iccsense: fix memory leak

[Sudip Mukherjee]

In the for loop we are allocating the memory for rail everytime but
in some cases we use "continue" and in those cases the memory already
allocated for rail is leaked and we again allocate new memory for it.
Lets free the memory before continuing with the loop.

Signed-off-by: Sudip Mukherjee 
---
 drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c 
b/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
index 323c79a..756ff07 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
@@ -290,21 +290,28 @@ nvkm_iccsense_oneinit(struct nvkm_subdev *subdev)

switch (sensor->type) {
case NVBIOS_EXTDEV_INA209:
-   if (r->rail != 0)
+   if (r->rail != 0) {
+   kfree(rail);
continue;
+   }
rail->read = nvkm_iccsense_ina209_read;
break;
case NVBIOS_EXTDEV_INA219:
-   if (r->rail != 0)
+   if (r->rail != 0) {
+   kfree(rail);
continue;
+   }
rail->read = nvkm_iccsense_ina219_read;
break;
case NVBIOS_EXTDEV_INA3221:
-   if (r->rail >= 3)
+   if (r->rail >= 3) {
+   kfree(rail);
continue;
+   }
rail->read = nvkm_iccsense_ina3221_read;
break;
default:
+   kfree(rail);
continue;
}

-- 
1.9.1

[Nouveau] [PATCH] drm/nouveau/iccsense: fix memory leak on default sensor->type case

[karol herbst]

Hi Colin,

thanks for pointing this out, but I am quite sure this continue
statement won't ever be hit, ever.

check the nvkm_iccsense_create_sensor function. A sensor object will
only be created for one of those three types and others don't exist.
I've just added that default statement to shut GCC I think.

The thing is I reworked that code already and will check if this
situation still applies there.

2016-06-02 12:28 GMT+02:00 Colin King :
> From: Colin Ian King 
>
> The default sensor->type case leaks memory allocated to rail. Fix
> this by free'ing rail before we continue with the next loop iteration.
>
> Signed-off-by: Colin Ian King 
> ---
>  drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c 
> b/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
> index 323c79a..79b0eb5 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
> @@ -305,6 +305,7 @@ nvkm_iccsense_oneinit(struct nvkm_subdev *subdev)
> rail->read = nvkm_iccsense_ina3221_read;
> break;
> default:
> +   kfree(rail);
> continue;
> }
>
> --
> 2.8.1
>
> ___
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau

[Nouveau] [PATCH] drm/nouveau/iccsense: fix memory leak on default sensor->type case

[Colin Ian King]

On 03/06/16 10:55, karol herbst wrote:
> Hi Colin,
> 
> thanks for pointing this out, but I am quite sure this continue
> statement won't ever be hit, ever.
> 
> check the nvkm_iccsense_create_sensor function. A sensor object will
> only be created for one of those three types and others don't exist.
> I've just added that default statement to shut GCC I think.
> 
> The thing is I reworked that code already and will check if this
> situation still applies there.

OK, thanks for letting me know. Apologies if I burnt up some of your
valuable cycles on this.

Colin

> 
> 2016-06-02 12:28 GMT+02:00 Colin King :
>> From: Colin Ian King 
>>
>> The default sensor->type case leaks memory allocated to rail. Fix
>> this by free'ing rail before we continue with the next loop iteration.
>>
>> Signed-off-by: Colin Ian King 
>> ---
>>  drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c 
>> b/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
>> index 323c79a..79b0eb5 100644
>> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
>> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
>> @@ -305,6 +305,7 @@ nvkm_iccsense_oneinit(struct nvkm_subdev *subdev)
>> rail->read = nvkm_iccsense_ina3221_read;
>> break;
>> default:
>> +   kfree(rail);
>> continue;
>> }
>>
>> --
>> 2.8.1
>>
>> ___
>> Nouveau mailing list
>> Nouveau at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/nouveau

[PATCH] drm/nouveau/iccsense: fix memory leak on default sensor->type case

[Ben Skeggs]

On 06/02/2016 08:28 PM, Colin King wrote:
> From: Colin Ian King 
> 
> The default sensor->type case leaks memory allocated to rail. Fix
> this by free'ing rail before we continue with the next loop iteration.
This doesn't completely fix the issue, as there are continue statements
in other sections of the code that can leak also.

That said, I already have a patch fixing this issue in a branch I
haven't pushed as of yet, I'll split it out later today.

Thanks,
Ben.

> 
> Signed-off-by: Colin Ian King 
> ---
>  drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c 
> b/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
> index 323c79a..79b0eb5 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
> @@ -305,6 +305,7 @@ nvkm_iccsense_oneinit(struct nvkm_subdev *subdev)
>   rail->read = nvkm_iccsense_ina3221_read;
>   break;
>   default:
> + kfree(rail);
>   continue;
>   }
>  
> 

-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: 

[PATCH] drm/nouveau/iccsense: fix memory leak on default sensor->type case

[Colin King]

From: Colin Ian King 

The default sensor->type case leaks memory allocated to rail. Fix
this by free'ing rail before we continue with the next loop iteration.

Signed-off-by: Colin Ian King 
---
 drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c 
b/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
index 323c79a..79b0eb5 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
@@ -305,6 +305,7 @@ nvkm_iccsense_oneinit(struct nvkm_subdev *subdev)
rail->read = nvkm_iccsense_ina3221_read;
break;
default:
+   kfree(rail);
continue;
}

-- 
2.8.1

[PATCH] drm/nouveau/iccsense: fix ifnullfree.cocci warnings

[kbuild test robot]

drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c:133:2-7: WARNING: NULL 
check before freeing functions like kfree, debugfs_remove, 
debugfs_remove_recursive or usb_free_urb is not needed. Maybe consider 
reorganizing relevant code to avoid passing NULL values.

 NULL check before some freeing functions is not needed.

 Based on checkpatch warning
 "kfree(NULL) is safe this check is probably not required"
 and kfreeaddr.cocci by Julia Lawall.

Generated by: scripts/coccinelle/free/ifnullfree.cocci

CC: Karol Herbst 
Signed-off-by: Fengguang Wu 
---

 base.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

--- a/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c
@@ -129,8 +129,7 @@ nvkm_iccsense_dtor(struct nvkm_subdev *s
 {
struct nvkm_iccsense *iccsense = nvkm_iccsense(subdev);

-   if (iccsense->rails)
-   kfree(iccsense->rails);
+   kfree(iccsense->rails);

return iccsense;
 }

[PATCH] drm/nouveau/iccsense: fix ifnullfree.cocci warnings

[Karol Herbst]

> Martin Peres  hat am 13. April 2016 um 09:55
> geschrieben:
> 
> 
> On 13/04/16 10:07, kbuild test robot wrote:
> > drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c:133:2-7: WARNING: NULL
> > check before freeing functions like kfree, debugfs_remove,
> > debugfs_remove_recursive or usb_free_urb is not needed. Maybe consider
> > reorganizing relevant code to avoid passing NULL values.
> >
> >   NULL check before some freeing functions is not needed.
> >
> >   Based on checkpatch warning
> >   "kfree(NULL) is safe this check is probably not required"
> >   and kfreeaddr.cocci by Julia Lawall.
> >
> > Generated by: scripts/coccinelle/free/ifnullfree.cocci
> >
> > CC: Karol Herbst 
> > Signed-off-by: Fengguang Wu 
> Signed-off-by: Martin Peres 
Signed-off-by: Karol Herbst 

[PATCH] drm/nouveau/iccsense: fix ifnullfree.cocci warnings

[Martin Peres]

On 13/04/16 10:07, kbuild test robot wrote:
> drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c:133:2-7: WARNING: NULL 
> check before freeing functions like kfree, debugfs_remove, 
> debugfs_remove_recursive or usb_free_urb is not needed. Maybe consider 
> reorganizing relevant code to avoid passing NULL values.
>
>   NULL check before some freeing functions is not needed.
>
>   Based on checkpatch warning
>   "kfree(NULL) is safe this check is probably not required"
>   and kfreeaddr.cocci by Julia Lawall.
>
> Generated by: scripts/coccinelle/free/ifnullfree.cocci
>
> CC: Karol Herbst 
> Signed-off-by: Fengguang Wu 
Signed-off-by: Martin Peres