Previously,the module was freed without detaching it from the list which could lead to memory leak
this patch uses list_del to safely remove the module from the list

Signed-off-by: Atharva Tiwari <evepolonium@gmail.com>
---
 drivers/staging/greybus/audio_manager_module.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/greybus/audio_manager_module.c b/drivers/staging/greybus/audio_manager_module.c
index 4a4dfb42f50f..7e52c3f95692 100644
--- a/drivers/staging/greybus/audio_manager_module.c
+++ b/drivers/staging/greybus/audio_manager_module.c
@@ -69,7 +69,8 @@ static void gb_audio_module_release(struct kobject *kobj)
 	struct gb_audio_manager_module *module = to_gb_audio_module(kobj);
 
 	pr_info("Destroying audio module #%d\n", module->id);
-	/* TODO -> delete from list */
+	if (module->list.prev && module->list.next)
+		list_del(&module->list);
 	kfree(module);
 }
 
-- 
2.39.5

On Mon, Dec 23, 2024 at 08:41:12PM +0530, Atharva Tiwari wrote:
> Previously,the module was freed without detaching it from the list which could lead to memory leak
> this patch uses list_del to safely remove the module from the list
> 
> Signed-off-by: Atharva Tiwari <evepolonium@gmail.com>
> ---
>  drivers/staging/greybus/audio_manager_module.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/greybus/audio_manager_module.c b/drivers/staging/greybus/audio_manager_module.c
> index 4a4dfb42f50f..7e52c3f95692 100644
> --- a/drivers/staging/greybus/audio_manager_module.c
> +++ b/drivers/staging/greybus/audio_manager_module.c
> @@ -69,7 +69,8 @@ static void gb_audio_module_release(struct kobject *kobj)
>  	struct gb_audio_manager_module *module = to_gb_audio_module(kobj);
>  
>  	pr_info("Destroying audio module #%d\n", module->id);
> -	/* TODO -> delete from list */
> +	if (module->list.prev && module->list.next)

For lists, you really shouldn't poke around in the prev and next
pointers, right?

Have you tested this?  This feels like an odd change, is it a real
system that is using this code anymore?

thanks,

greg k-h

Yes I have tested the patch with greybus drivers enabled on Debian 12.It works as expected, and i did not get any errors or warning during the compilation

Please let me know if further changes are required.

Best regards,
Atharva Tiwari

On Tue, Dec 24, 2024 at 10:49:17AM +0530, Atharva Tiwari wrote:
> Yes I have tested the patch with greybus drivers enabled on Debian 12.It works as expected, and i did not get any errors or warning during the compilation
> 
> Please let me know if further changes are required.
> 

You're trying to ensure that the list is initialized but instead of testing
for that, the correct way is to initialize it earlier in the process.

regards,
dan carpenter

On Tue, Dec 24, 2024 at 10:49:17AM +0530, Atharva Tiwari wrote:
> Yes I have tested the patch with greybus drivers enabled on Debian 12.It works as expected, and i did not get any errors or warning during the compilation

I have no context here, sorry.

Remember, some of use get over 1000 emails a day to do something with,
always respond with proper email quoting and context of what is going
on.

Also, just bulding code does not mean it works :)

thanks,

greg k-h