Failed default audio devices were removed from the list but not freed,
and that made LeakSanitizer sad. Free default audio devices as they are
consumed.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
V1 -> V2: Set s->dev NULL after free (Marc-André Lureau)

 audio/audio.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/audio/audio.c b/audio/audio.c
index f91e05b72c..8d1e4ad922 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1758,12 +1758,15 @@ static AudioState *audio_init(Audiodev *dev, Error **errp)
                 goto out;
             }
             s->dev = dev = e->dev;
+            QSIMPLEQ_REMOVE_HEAD(&default_audiodevs, next);
+            g_free(e);
             drvname = AudiodevDriver_str(dev->driver);
             driver = audio_driver_lookup(drvname);
             if (!audio_driver_init(s, driver, dev, NULL)) {
                 break;
             }
-            QSIMPLEQ_REMOVE_HEAD(&default_audiodevs, next);
+            qapi_free_Audiodev(dev);
+            s->dev = NULL;
         }
     }
 
-- 
2.42.1

Queued, thanks.

Paolo

Hi

On Mon, Nov 20, 2023 at 3:29 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> Failed default audio devices were removed from the list but not freed,
> and that made LeakSanitizer sad. Free default audio devices as they are
> consumed.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Acked-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
> V1 -> V2: Set s->dev NULL after free (Marc-André Lureau)
>
>  audio/audio.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/audio/audio.c b/audio/audio.c
> index f91e05b72c..8d1e4ad922 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -1758,12 +1758,15 @@ static AudioState *audio_init(Audiodev *dev, Error **errp)
>                  goto out;
>              }
>              s->dev = dev = e->dev;
> +            QSIMPLEQ_REMOVE_HEAD(&default_audiodevs, next);
> +            g_free(e);
>              drvname = AudiodevDriver_str(dev->driver);
>              driver = audio_driver_lookup(drvname);
>              if (!audio_driver_init(s, driver, dev, NULL)) {

Maybe it would be clearer/safer to set s->dev in audio_driver_init() on success.

>                  break;
>              }
> -            QSIMPLEQ_REMOVE_HEAD(&default_audiodevs, next);
> +            qapi_free_Audiodev(dev);
> +            s->dev = NULL;
>          }
>      }
>
> --
> 2.42.1
>
>


-- 
Marc-André Lureau

On 2023/11/20 20:35, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Nov 20, 2023 at 3:29 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>>
>> Failed default audio devices were removed from the list but not freed,
>> and that made LeakSanitizer sad. Free default audio devices as they are
>> consumed.
>>
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> 
> Acked-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
>> ---
>> V1 -> V2: Set s->dev NULL after free (Marc-André Lureau)
>>
>>   audio/audio.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/audio/audio.c b/audio/audio.c
>> index f91e05b72c..8d1e4ad922 100644
>> --- a/audio/audio.c
>> +++ b/audio/audio.c
>> @@ -1758,12 +1758,15 @@ static AudioState *audio_init(Audiodev *dev, Error **errp)
>>                   goto out;
>>               }
>>               s->dev = dev = e->dev;
>> +            QSIMPLEQ_REMOVE_HEAD(&default_audiodevs, next);
>> +            g_free(e);
>>               drvname = AudiodevDriver_str(dev->driver);
>>               driver = audio_driver_lookup(drvname);
>>               if (!audio_driver_init(s, driver, dev, NULL)) {
> 
> Maybe it would be clearer/safer to set s->dev in audio_driver_init() on success.

I considered that too, but I decided not to that because 
audio_driver_init() takes s and may expect s->dev is initialized.