Anton Lundin wrote: > 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. Okay, I understand your concern really is a valid point. I don't have strong preference for either side, so simply follow your suggestion. Thanks, Hajime > //Anton - who clearly hears the bikeshedding warning bells going off > >
