Comment 104 for bug 1754671

(In reply to Beniamino Galvani from comment #38)
> (In reply to Thomas Haller from comment #37)

> 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.

I mean, to handle "my..domain". Either normalize the double . away, or verify it and drop it as invalid. If you don't do either, then "~.." will end up being treated like "~." which is wrong.

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

Yes, after dropping the default domain becomes "". It's not that you currently use the domain "." as-is. You do:

  nm_streq (domain, ".") ? NULL : domain)

that could also be:

  nm_streq (domain, "") ? NULL : domain)

or

#define DEFAULT_DOMAIN ""
  nm_streq (domain, DEFAULT_DOMAIN) ? NULL : domain)

(maybe a define is in order either way).

> > > 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?

Maybe the hashing mechanism is anyway ugly, and should be dropped (in a future commit). Instead of implementing a SHA-hashing of ~some~ parameters, implement a cmp() function. It's more efficient, and easier to get right.

> > > 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.

you are right.