On Wed, 10 Aug 2016, at 10:15 PM, Tanu Kaskinen wrote: > 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. As Tanu points out, my commit message (which I've dropped now) is incorrect. old_value will never be NULL in that path. -- Arun