vfs_fruit: Time Machine/FULLSYNC: add mDNS/DNS-SD advertisement

Omri Mor omri50 at gmail.com
Mon Jul 24 01:30:20 UTC 2017


>> Can avahi_string_list_add() fail and return NULL?
>> Can avahi_string_list_add_printf() fail and return NULL?
> 
> The documentation doesn’t say; however, the Avahi source code reveals that it can.
> I’ll add in checks.

Looking further into it, the situation becomes more complex.
Idiomatically, when adding item to a list, you assign the new list pointer to the old list head.
However, in an out-of-memory situation, avahi_string_list_add() and its ilk return NULL.
So if you do

AvahiStringList *list = NULL;
list = avahi_string_list_add(list, “test”); /* succeeds, list != NULL */
list = avahi_string_list_add(list, “test2”); /* OOM, fails, list == NULL */

Then you lose the rest of the list and can no longer free it, resulting in a memory leak (which would exacerbate the OOM).
So you’d need to do something like:

AvahiStringList *list = NULL;
AvahiStringList *list_n = NULL;
list_n = avahi_string_list_add(list, “test”); /* succeeds, list_n != NULL */
if (list_n == NULL) {
	avahi_string_list_free(list);
} else {
	list = list_n; /* code goes here */
}
list_n = avahi_string_list_add(list, “test2”); /* OOM, fails, list_n == NULL */
if (list_n == NULL) {
	avahi_string_list_free(list); /* yay, free memory */
} else {
	list = list_n;
}

Which is much more complex and error-prone, as well as ugly.
Since the list functions returning NULL on failed memory allocation is undocumented, I’m not sure what to do.
Netatalk, for instance, doesn’t bother with error-checking the string list at all.
They also don’t free it after they use it, so I’m not sure that they are a good example of what to do.
Even Avahi itself gets this wrong: avahi_string_list_copy() calls avahi_string_list_free() on a null pointer, which of course does nothing.

Of course, I think that Avahi aborts on OOM (see oom() in avahi/avahi-common/malloc.c, which is called by their internal malloc/realloc/calloc wrappers, as well as g_malloc() from Glib, which also aborts on OOM).
So the entire point is moot: getting a null pointer from from avahi_string_list_*() would require setting a custom allocator that does not abort on OOM, which we do not currently do.
I suppose we *could* set it up with an talloc-based allocator, but that’s just increasing complexity without much benefit, I think, and is liable to exposing unknown bugs in Avahi.

Omri


More information about the samba-technical mailing list