[pulseaudio-discuss] [PATCH] sink-input, source-output: Fix a leak during property change logging

[Peter Meerwald-Stadler]

Hi,

> Using pa_xfree() irrespective of whether old_value is NULL or not to
> avoid potentially confusing nest of if statements.

starring at the same issue ATM :)
the proposed fix below gets rid of the potential leak, but 

(1) why do we set old_value to "(data)" in the first place?
wouldn't "(null)" make more sense?

(2) in case value == 0, we'd still overwrite old_value, makes no sense
 
(3) could use pa_safe_streq()

my suggestion would be

    if (pa_proplist_contains(o->proplist, key)) {
        old_value = pa_xstrdup(pa_proplist_gets(o->proplist, key));
        if (str_safe_streq(value, old_value))
            goto finish;
        if (!old_value)
            old_value = pa_xstrdup("(null)"); // or data?
    } else {
 
regards, p.

> CID: 1352052
> ---
>  src/pulsecore/sink-input.c    | 4 +++-
>  src/pulsecore/source-output.c | 4 +++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/src/pulsecore/sink-input.c b/src/pulsecore/sink-input.c
> index a7f6d55..1ed5dda 100644
> --- a/src/pulsecore/sink-input.c
> +++ b/src/pulsecore/sink-input.c
> @@ -1436,8 +1436,10 @@ void pa_sink_input_set_property(pa_sink_input *i, const char *key, const char *v
>          if (value && old_value) {
>              if (pa_streq(value, old_value))
>                  goto finish;
> -        } else
> +        } else {
> +            pa_xfree(old_value);
>              old_value = pa_xstrdup("(data)");
> +        }
>      } else {
>          if (!value)
>              goto finish;
> diff --git a/src/pulsecore/source-output.c b/src/pulsecore/source-output.c
> index 9e951ff..6c48bbc 100644
> --- a/src/pulsecore/source-output.c
> +++ b/src/pulsecore/source-output.c
> @@ -1089,8 +1089,10 @@ void pa_source_output_set_property(pa_source_output *o, const char *key, const c
>          if (value && old_value) {
>              if (pa_streq(value, old_value))
>                  goto finish;
> -        } else
> +        } else {
> +            pa_xfree(old_value);
>              old_value = pa_xstrdup("(data)");
> +        }
>      } else {
>          if (!value)
>              goto finish;
> -- 
> 2.7.4
> 
> _______________________________________________
> pulseaudio-discuss mailing list
> pulseaudio-discuss at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
> 

-- 

Peter Meerwald-Stadler
+43-664-2444418 (mobile)