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

[Tanu Kaskinen]

On Wed, 2016-08-10 at 18:33 +0200, Peter Meerwald-Stadler wrote:
> 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?

pa_proplist_contains() returned true, so the hashmap contains something
for the key. old_data can only be NULL, if the old value was not an
utf8 string, hence "(data)" instead of the actual value.

> (2) in case value == 0, we'd still overwrite old_value, makes no
> sense

Why not? value is the new value, old_value is the old value. The new
value doesn't affect what we should print in the log message for the
old value.

> (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;

This doesn't handle correctly the transition from non-utf8 data to
unset (i.e. when both variables are NULL).

-- 
Tanu