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
