Comment 108 for bug 1754671

(In reply to Thomas Haller from comment #41)
> > core: allow '~.' domain in ip4 and ip6 configs
>
> commit bebafff7844228d018d17f0d11c5d6035dbf6a91,
> lgtm, but the commit message no longer seams to match, does it?

> the list head is accessed on every iteration. Could we pre-cache the value
> like:

> + int i, n, n_domains = 0;
>
> these variables are used to iterate over a number of guint values. Like
>
> n = nm_ip_config_get_num_searches (ip_config);
>
> where get_num_searches() returns a guint. Could we consistently use the
> correct type?

> add the wilcard domain to all non-VPN

> is "." still right? Should this be "" to mean the wildcard domain?

> could you add a
> nm_assert (!g_hash_table_contains (ht, domain));
> to domain_is_shadowed() -- because that one is already checked before
> calling domain_is_shadowed().

Fixed all the above.

> I always like avoiding unnecessary copies. But in this case, "search" array
> will point inside strings owned by NMIPxConfig. That seems quite fragile to
> me. Should we not clone them? In practice it's not necessary, but it feels
> fragile. What can we do about that?

Now the list is cleared just after nm_dns_plugin_update() returns to ensure
that elements in the list don't become stale. What do you think?

> »···/* Free the array and return NULL if the only element was the ending
> NULL */
> »···strv = (char **) g_ptr_array_free (domains, (domains->len == 1));
>
> »···return _nm_utils_strv_cleanup (strv, FALSE, FALSE, TRUE);
>
> skip_repeated=TRUE likely won't help much, because duplicated domains are
> probably not sorted after each other. Maybe sort them first? Maybe by
> cherry-picking "shared: add nm_utils_strv_sort() helper" from
> https://github.com/NetworkManager/NetworkManager/pull/91

I don't understand, duplicate domains don't have to be consecutive with
current _nm_utils_strv_cleanup() implementation.