[pulseaudio-discuss] Revisiting raop2 patches

Anton Lundin glance at acc.umu.se
Mon Oct 31 06:59:06 UTC 2016


On 30 October, 2016 - Hajime Fujita wrote:
> 
> > On Oct 26, 2016, at 9:58 PM, Hajime Fujita <crisp.fujita at nifty.com> wrote:
> > 
> >> On Oct 25, 2016, at 3:47 PM, Anton Lundin <glance at acc.umu.se> wrote:
> > 
> >> Also, value is set to NULL in some parsing blocks and not in others.
> >> Some blocks strdup(value), and let avahi_free(value) free it, others
> >> "steal" value and set value = NULL. I'd feel more comfortable with all
> >> the code paths strdup'ing them selfs. Even, why not use pa_xstrdup ?
> > 
> > Will take a look.
> 
> So I took a look at this.
> 
> To me, this code makes some sense.
> Strdup is used when the raop module generates a new string based on information in a value given by Avahi.
> Value “stealing” happens when the raop module uses an Avahi string as-is. In this case strdup’ing is essentially a waste of memory and allocation/free cost. (Nobody probably cares about the cost here though)
> 
> “strdup” should be “pa_xstrdup”, I agree.
>

Yea, its some added cost in the discovery process, the thing that stands
out to me is that the code paths are different. Different means
complexity and complexity menas potential for bugs.

Yea, its a simple case, but I don't know the allocation rules for
avahi-strings. Its better to handle all of that logic right there and
then, and let PA own the strings from there and onwards.

It's also the case of keeping track of whats avahi-memory and whats
pa-memory. I'd guess pa_xfree() and avahi_free() are compatible wrappers
ontop of free(), but who knows. Some day some crazy person might change
one of them to be a completely different beast, and things will start to
behave strangely.

In such a code path as this one, I'd vote for clarity and robustness
every day of the week.


One can clearly see that module-raop-discover.c inherited alot of code
from module-zeroconf-discover.c, and this odity is there to. The same
goes for ex. that the hashmap that keeps track of all loaded raop
modules are called tunnels.


//Anton - who clearly hears the bikeshedding warning bells going off


-- 
Anton Lundin	+46702-161604


More information about the pulseaudio-discuss mailing list