Hello,
> > starring at the same issue ATM :)
> > the proposed fix below gets rid of the potential leak, butÂ
> > (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.
in case old_value != 0 and value == 0, we'd log
key: (data) -> (unset)
but the correct logging IMHO would be
key: old_value -> (unset)
new suggestion:
if (pa_proplist_contains(o->proplist, key)) {
old_value = pa_xstrdup(pa_proplist_gets(o->proplist, key));
if (value && old_value && pa_streq(value, old_value))
goto finish;
if (!old_value)
old_value = pa_xstrdup("(data)");
} else {
> > (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).
right
--
Peter Meerwald-Stadler
+43-664-2444418 (mobile)
