On Tue, Jul 29, 2014 at 3:37 PM, Zbigniew Jędrzejewski-Szmek <zbys...@in.waw.pl> wrote: > On Tue, Jul 29, 2014 at 02:48:18PM -0700, Michael Marineau wrote: >> When the code for generating resolv.conf was moved from networkd to >> resolved the DHCP domain name code was dropped. >> --- >> >> This is a resend, rebased since some recent changes changed how this >> patch needed to be implemented. >> >> src/network/networkd-link.c | 13 +++++++++++++ >> src/network/sd-network.c | 24 ++++++++++++++++++++++++ >> src/resolve/resolved-link.c | 20 ++++++++++++++++++++ >> src/resolve/resolved-link.h | 2 ++ >> src/resolve/resolved-manager.c | 10 +++++++++- >> src/systemd/sd-network.h | 3 +++ >> 6 files changed, 71 insertions(+), 1 deletion(-) >> >> diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c >> index 3b8b7ed..827c428 100644 >> --- a/src/network/networkd-link.c >> +++ b/src/network/networkd-link.c >> @@ -2451,6 +2451,19 @@ int link_save(Link *link) { >> (address + 1 ? " " : "")); >> >> fputs("\n", f); >> + >> + fprintf(f, "DOMAINNAME="); >> + >> + if (link->network->dhcp_domainname && >> + link->dhcp_lease) { >> + const char *domainname; >> + >> + r = sd_dhcp_lease_get_domainname(link->dhcp_lease, >> &domainname); >> + if (r >= 0) >> + fputs(domainname, f); >> + } >> + >> + fputs("\n", f); > Is it really necessary to write anything if the name is not available? > Other parts of this function don't write anyting in similar cases.
I was just matching the above lines which may write DNS= or NTP= with blank values. I don't think it matters either way. Omitting DOMAINNAME= if it is blank certainly looks a little cleaner since the writes get squashed into a single fprintf. Will update. > >> >> if (link->dhcp_lease) { >> diff --git a/src/network/sd-network.c b/src/network/sd-network.c >> index bfb8321..a427a27 100644 >> --- a/src/network/sd-network.c >> +++ b/src/network/sd-network.c >> @@ -175,6 +175,30 @@ _public_ int sd_network_get_ntp(int ifindex, char >> ***ret) { >> return network_get_strv("NTP", ifindex, ret); >> } >> >> +_public_ int sd_network_get_domainname(int ifindex, char **domainname) { >> + _cleanup_free_ char *s = NULL, *p = NULL; >> + int r; >> + >> + assert_return(ifindex > 0, -EINVAL); >> + assert_return(domainname, -EINVAL); >> + >> + if (asprintf(&p, "/run/systemd/netif/links/%d", ifindex) < 0) >> + return -ENOMEM; > Not terribly important, but please spell that as: > > char p[sizeof("/run/systemd/netif/links/") + > DECIMAL_STRING_MAX(int)]; > snprintf(p, sizeof(p), "/run/systemd/netif/links/%d", ifindex); This was copied verbatim from similar functions in this file, should I update the style of the others to match your suggestion? Why the preference of manually calculating a buffer length than using asprintf? > >> + r = parse_env_file(p, NEWLINE, "DOMAINNAME", &s, NULL); >> + if (r == -ENOENT) >> + return -ENODATA; >> + else if (r < 0) >> + return r; >> + else if (!s) >> + return -EIO; >> + >> + *domainname = s; >> + s = NULL; >> + >> + return 0; >> +} >> + >> static inline int MONITOR_TO_FD(sd_network_monitor *m) { >> return (int) (unsigned long) m - 1; >> } >> diff --git a/src/resolve/resolved-link.c b/src/resolve/resolved-link.c >> index 6ac7c5b..f6b7f6a 100644 >> --- a/src/resolve/resolved-link.c >> +++ b/src/resolve/resolved-link.c >> @@ -77,6 +77,7 @@ Link *link_free(Link *l) { >> while (l->dns_servers) >> dns_server_free(l->dns_servers); >> >> + free(l->domainname); >> free(l); >> return NULL; >> } >> @@ -191,10 +192,29 @@ clear: >> return r; >> } >> >> +static int link_update_domainname(Link *l) { >> + char *domainname = NULL; >> + int r; >> + >> + assert(l); >> + >> + free(l->domainname); >> + l->domainname = NULL; >> + >> + r = sd_network_get_domainname(l->ifindex, &domainname); >> + if (r < 0) >> + return r; >> + >> + l->domainname = domainname; >> + >> + return 0; >> +} >> + >> int link_update_monitor(Link *l) { >> assert(l); >> >> link_update_dns_servers(l); >> + link_update_domainname(l); >> link_allocate_scopes(l); >> link_add_rrs(l); >> >> diff --git a/src/resolve/resolved-link.h b/src/resolve/resolved-link.h >> index f58bd54..9730aec 100644 >> --- a/src/resolve/resolved-link.h >> +++ b/src/resolve/resolved-link.h >> @@ -68,6 +68,8 @@ struct Link { >> >> RateLimit mdns_ratelimit; >> RateLimit llmnr_ratelimit; >> + >> + char *domainname; >> }; >> >> int link_new(Manager *m, Link **ret, int ifindex); >> diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c >> index a8715bd..253a97e 100644 >> --- a/src/resolve/resolved-manager.c >> +++ b/src/resolve/resolved-manager.c >> @@ -522,6 +522,7 @@ int manager_write_resolv_conf(Manager *m) { >> const char *path = "/run/systemd/resolve/resolv.conf"; >> _cleanup_free_ char *temp_path = NULL; >> _cleanup_fclose_ FILE *f = NULL; >> + const char *domainname = NULL; >> unsigned count = 0; >> DnsServer *s; >> Iterator i; >> @@ -542,13 +543,20 @@ int manager_write_resolv_conf(Manager *m) { >> "# resolv.conf(5) in a different way, replace the symlink by >> a\n" >> "# static file or a different symlink.\n\n", f); >> >> - HASHMAP_FOREACH(l, m->links, i) >> + HASHMAP_FOREACH(l, m->links, i) { >> LIST_FOREACH(servers, s, l->dns_servers) >> write_resolve_conf_server(s, f, &count); >> >> + if (!domainname && l->domainname) >> + domainname = l->domainname; >> + } >> + >> LIST_FOREACH(servers, s, m->dns_servers) >> write_resolve_conf_server(s, f, &count); >> >> + if (domainname) >> + fprintf(f, "domain %s\n", domainname); >> + > > How does this deal with empty domain names? Let's say the server is > misconfigure and returns "". Will this still be valid syntax? Also, > are the contents checked for special characters, etc, earlier? sd_network_get_domainname() returns NULL for empty strings following the behavior of parse_env_file() so simply checking for NULL elsewhere is sufficient. _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel