Comment 103 for bug 1754671

(In reply to Thomas Haller from comment #37)
> > dns: dnsmasq: fix adding multiple domains
>
> can you add a "Fixes" comment?

Done.

> > core: allow '~.' domain in ip4 and ip6 configs
>
> + if (!nm_streq (search, "~.")) {
> len = strlen (search);
> if (search[len - 1] == '.')
> search[len - 1] = 0;
> + }
>
> this seems wrong, for example if "search" is "~..".
> Either, do a full normalization and drop such entires (or clean up the
> duplicate dots). Or: normalize "~." to "~" too. It's odd that the special
> domain is treated entirely different regardingt the trailing dot.

What do you mean by "full normalization"?

We can convert 'domain.' into 'domain' because it's equivalent, but
'domain..' or 'my..domain' are invalid and they should be dropped. I
think this would be the best approach.

If we normalize '~.' to '~', once we strip the ~ prefix the domain
would become empty, which is not desirable in my opinion.

> > dns: use dns-priority to provide a preprocessed domain list to plugins
>
> _LOGD ("update-dns: updating plugin %s", plugin_name);
> + rebuild_domain_lists (self);
> if (!nm_dns_plugin_update (plugin,
>
> do we need to rebuild the list every time? We know when an update changes
> something. Can we cache the result, generate it when needed, and clear it
> when something changes?

If nothing changes, the DNS configuration hash will be the same and
update_dns() won't be called at all by nm_dns_manager_end_updates().

Ok, nm_dns_manager_set_hostname() calls update_dns() without checking
the hash, but do we need another caching mechanism different from the
existing one just for this case?

> + struct {
> + const char **search;
> + char **reverse;
> + } domains;
> } NMDnsIPConfigData;
>
> I think these are leaked.

Ops, fixed.

> > dns: dnsmasq: honor dns-priority
>
> + int addr_family, i, j, num;
>
> let's use guint for index variables of arrays? Also matches
> num = nm_ip_config_get_num_nameservers (ip_config);

Ok.

> > dns: sd-resolved: honor dns-priority
>
> - NMIPConfig *ip_config = elem->data;
> + NMDnsIPConfigData *data = elem->data;
> + NMIPConfig *ip_config = data->ip_config;
>
> this seems wrong.

Why? Looks fine to me.