dhcp: cleanup static option list for internal client

- use proper data types "guint16" and "bool" in static
  option list. It saves a few bytes, but also it's the appropriate
  type. Well, at least, it's the appropriate type for DHCPv6,
  not for DHCPv4 (which is guint8).

- assert against failure of sd_dhcp_client_set_request_option() and
  sd_dhcp6_client_set_request_option().

1 files changed, 70 insertions, 56 deletions

diff --git a/src/dhcp/nm-dhcp-systemd.c b/src/dhcp/nm-dhcp-systemd.c
index 92152bba99..e31a526366 100644
--- a/src/dhcp/nm-dhcp-systemd.c
+++ b/src/dhcp/nm-dhcp-systemd.c
@@ -98,63 +98,72 @@ G_DEFINE_TYPE (NMDhcpSystemd, nm_dhcp_systemd, NM_TYPE_DHCP_CLIENT)
 #define DHCP6_OPTION_IAID            1034
 
 typedef struct {
-	guint num;
 	const char *name;
-	gboolean include;
+	uint16_t option_num;
+	bool include;
 } ReqOption;
 
 #define REQPREFIX "requested_"
 
+#define REQ(_num, _name, _include) \
+	{ \
+		.name = REQPREFIX""_name, \
+		.option_num = _num, \
+		.include = _include, \
+	}
+
 static const ReqOption dhcp4_requests[] = {
-	{ SD_DHCP_OPTION_SUBNET_MASK,                    REQPREFIX "subnet_mask",                     TRUE },
-	{ SD_DHCP_OPTION_TIME_OFFSET,                    REQPREFIX "time_offset",                     TRUE },
-	{ SD_DHCP_OPTION_ROUTER,                         REQPREFIX "routers",                         TRUE },
-	{ SD_DHCP_OPTION_DOMAIN_NAME_SERVER,             REQPREFIX "domain_name_servers",             TRUE },
-	{ SD_DHCP_OPTION_HOST_NAME,                      REQPREFIX "host_name",                       TRUE },
-	{ SD_DHCP_OPTION_DOMAIN_NAME,                    REQPREFIX "domain_name",                     TRUE },
-	{ SD_DHCP_OPTION_INTERFACE_MTU,                  REQPREFIX "interface_mtu",                   TRUE },
-	{ SD_DHCP_OPTION_BROADCAST,                      REQPREFIX "broadcast_address",               TRUE },
-	{ SD_DHCP_OPTION_STATIC_ROUTE,                   REQPREFIX "static_routes",                   TRUE },
-	{ DHCP_OPTION_NIS_DOMAIN,                        REQPREFIX "nis_domain",                      TRUE },
-	{ DHCP_OPTION_NIS_SERVERS,                       REQPREFIX "nis_servers",                     TRUE },
-	{ SD_DHCP_OPTION_NTP_SERVER,                     REQPREFIX "ntp_servers",                     TRUE },
-	{ SD_DHCP_OPTION_SERVER_IDENTIFIER,              REQPREFIX "dhcp_server_identifier",          TRUE },
-	{ SD_DHCP_OPTION_DOMAIN_SEARCH_LIST,             REQPREFIX "domain_search",                   TRUE },
-	{ SD_DHCP_OPTION_CLASSLESS_STATIC_ROUTE,         REQPREFIX "rfc3442_classless_static_routes", TRUE },
-	{ SD_DHCP_OPTION_PRIVATE_CLASSLESS_STATIC_ROUTE, REQPREFIX "ms_classless_static_routes",      TRUE },
-	{ SD_DHCP_OPTION_PRIVATE_PROXY_AUTODISCOVERY,    REQPREFIX "wpad",                            TRUE },
-	{ SD_DHCP_OPTION_ROOT_PATH,                      REQPREFIX "root_path",                       TRUE },
+	REQ (SD_DHCP_OPTION_SUBNET_MASK,                    "subnet_mask",                     TRUE ),
+	REQ (SD_DHCP_OPTION_TIME_OFFSET,                    "time_offset",                     TRUE ),
+	REQ (SD_DHCP_OPTION_ROUTER,                         "routers",                         TRUE ),
+	REQ (SD_DHCP_OPTION_DOMAIN_NAME_SERVER,             "domain_name_servers",             TRUE ),
+	REQ (SD_DHCP_OPTION_HOST_NAME,                      "host_name",                       TRUE ),
+	REQ (SD_DHCP_OPTION_DOMAIN_NAME,                    "domain_name",                     TRUE ),
+	REQ (SD_DHCP_OPTION_INTERFACE_MTU,                  "interface_mtu",                   TRUE ),
+	REQ (SD_DHCP_OPTION_BROADCAST,                      "broadcast_address",               TRUE ),
+	REQ (SD_DHCP_OPTION_STATIC_ROUTE,                   "static_routes",                   TRUE ),
+	REQ (DHCP_OPTION_NIS_DOMAIN,                        "nis_domain",                      TRUE ),
+	REQ (DHCP_OPTION_NIS_SERVERS,                       "nis_servers",                     TRUE ),
+	REQ (SD_DHCP_OPTION_NTP_SERVER,                     "ntp_servers",                     TRUE ),
+	REQ (SD_DHCP_OPTION_SERVER_IDENTIFIER,              "dhcp_server_identifier",          TRUE ),
+	REQ (SD_DHCP_OPTION_DOMAIN_SEARCH_LIST,             "domain_search",                   TRUE ),
+	REQ (SD_DHCP_OPTION_CLASSLESS_STATIC_ROUTE,         "rfc3442_classless_static_routes", TRUE ),
+	REQ (SD_DHCP_OPTION_PRIVATE_CLASSLESS_STATIC_ROUTE, "ms_classless_static_routes",      TRUE ),
+	REQ (SD_DHCP_OPTION_PRIVATE_PROXY_AUTODISCOVERY,    "wpad",                            TRUE ),
+	REQ (SD_DHCP_OPTION_ROOT_PATH,                      "root_path",                       TRUE ),
 
 	/* Internal values */
-	{ SD_DHCP_OPTION_IP_ADDRESS_LEASE_TIME,          REQPREFIX "expiry",                          FALSE },
-	{ SD_DHCP_OPTION_CLIENT_IDENTIFIER,              REQPREFIX "dhcp_client_identifier",          FALSE },
-	{ DHCP_OPTION_IP_ADDRESS,                        REQPREFIX "ip_address",                      FALSE },
-	{ 0, NULL, FALSE }
+	REQ (SD_DHCP_OPTION_IP_ADDRESS_LEASE_TIME,          "expiry",                          FALSE ),
+	REQ (SD_DHCP_OPTION_CLIENT_IDENTIFIER,              "dhcp_client_identifier",          FALSE ),
+	REQ (DHCP_OPTION_IP_ADDRESS,                        "ip_address",                      FALSE ),
+
+	{ 0 }
 };
 
 static const ReqOption dhcp6_requests[] = {
-	{ SD_DHCP6_OPTION_CLIENTID,                      REQPREFIX "dhcp6_client_id",     FALSE },
+	REQ (SD_DHCP6_OPTION_CLIENTID,                      "dhcp6_client_id",     FALSE ),
 
 	/* Don't request server ID by default; some servers don't reply to
 	 * Information Requests that request the Server ID.
 	 */
-	{ SD_DHCP6_OPTION_SERVERID,                      REQPREFIX "dhcp6_server_id",     FALSE },
+	REQ (SD_DHCP6_OPTION_SERVERID,                      "dhcp6_server_id",     FALSE ),
 
-	{ SD_DHCP6_OPTION_DNS_SERVERS,                   REQPREFIX "dhcp6_name_servers",  TRUE },
-	{ SD_DHCP6_OPTION_DOMAIN_LIST,                   REQPREFIX "dhcp6_domain_search", TRUE },
-	{ SD_DHCP6_OPTION_SNTP_SERVERS,                  REQPREFIX "dhcp6_sntp_servers",  TRUE },
+	REQ (SD_DHCP6_OPTION_DNS_SERVERS,                   "dhcp6_name_servers",  TRUE ),
+	REQ (SD_DHCP6_OPTION_DOMAIN_LIST,                   "dhcp6_domain_search", TRUE ),
+	REQ (SD_DHCP6_OPTION_SNTP_SERVERS,                  "dhcp6_sntp_servers",  TRUE ),
 
 	/* Internal values */
-	{ DHCP6_OPTION_IP_ADDRESS,                       REQPREFIX "ip6_address",         FALSE },
-	{ DHCP6_OPTION_PREFIXLEN,                        REQPREFIX "ip6_prefixlen",       FALSE },
-	{ DHCP6_OPTION_PREFERRED_LIFE,                   REQPREFIX "preferred_life",      FALSE },
-	{ DHCP6_OPTION_MAX_LIFE,                         REQPREFIX "max_life",            FALSE },
-	{ DHCP6_OPTION_STARTS,                           REQPREFIX "starts",              FALSE },
-	{ DHCP6_OPTION_LIFE_STARTS,                      REQPREFIX "life_starts",         FALSE },
-	{ DHCP6_OPTION_RENEW,                            REQPREFIX "renew",               FALSE },
-	{ DHCP6_OPTION_REBIND,                           REQPREFIX "rebind",              FALSE },
-	{ DHCP6_OPTION_IAID,                             REQPREFIX "iaid",                FALSE },
-	{ 0, NULL, FALSE }
+	REQ (DHCP6_OPTION_IP_ADDRESS,                       "ip6_address",         FALSE ),
+	REQ (DHCP6_OPTION_PREFIXLEN,                        "ip6_prefixlen",       FALSE ),
+	REQ (DHCP6_OPTION_PREFERRED_LIFE,                   "preferred_life",      FALSE ),
+	REQ (DHCP6_OPTION_MAX_LIFE,                         "max_life",            FALSE ),
+	REQ (DHCP6_OPTION_STARTS,                           "starts",              FALSE ),
+	REQ (DHCP6_OPTION_LIFE_STARTS,                      "life_starts",         FALSE ),
+	REQ (DHCP6_OPTION_RENEW,                            "renew",               FALSE ),
+	REQ (DHCP6_OPTION_REBIND,                           "rebind",              FALSE ),
+	REQ (DHCP6_OPTION_IAID,                             "iaid",                FALSE ),
+
+	{ 0 }
 };
 
 static void
@@ -165,18 +174,22 @@ take_option (GHashTable *options,
 {
 	guint i;
 
-	g_return_if_fail (value != NULL);
+	nm_assert (options);
+	nm_assert (requests);
+	nm_assert (value);
 
 	for (i = 0; requests[i].name; i++) {
-		if (requests[i].num == option) {
+		nm_assert (g_str_has_prefix (requests[i].name, REQPREFIX));
+		if (requests[i].option_num == option) {
 			g_hash_table_insert (options,
 			                     (gpointer) (requests[i].name + NM_STRLEN (REQPREFIX)),
 			                     value);
-			break;
+			return;
 		}
 	}
+
 	/* Option should always be found */
-	g_assert (requests[i].name);
+	nm_assert_not_reached ();
 }
 
 static void
@@ -187,13 +200,6 @@ add_option (GHashTable *options, const ReqOption *requests, guint option, const
 }
 
 static void
-add_option_u32 (GHashTable *options, const ReqOption *requests, guint option, guint32 value)
-{
-	if (options)
-		take_option (options, requests, option, g_strdup_printf ("%u", value));
-}
-
-static void
 add_option_u64 (GHashTable *options, const ReqOption *requests, guint option, guint64 value)
 {
 	if (options)
@@ -205,7 +211,10 @@ add_requests_to_options (GHashTable *options, const ReqOption *requests)
 {
 	guint i;
 
-	for (i = 0; options && requests[i].name; i++) {
+	if (!options)
+		return;
+
+	for (i = 0; requests[i].name; i++) {
 		if (requests[i].include)
 			g_hash_table_insert (options, (gpointer) requests[i].name, g_strdup ("1"));
 	}
@@ -420,7 +429,7 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx,
 	r = sd_dhcp_lease_get_mtu (lease, &mtu);
 	if (r == 0 && mtu) {
 		nm_ip4_config_set_mtu (ip4_config, mtu, NM_IP_CONFIG_SOURCE_DHCP);
-		add_option_u32 (options, dhcp4_requests, SD_DHCP_OPTION_INTERFACE_MTU, mtu);
+		add_option_u64 (options, dhcp4_requests, SD_DHCP_OPTION_INTERFACE_MTU, mtu);
 		LOG_LEASE (LOGD_DHCP4, "mtu %u", mtu);
 	}
 
@@ -671,8 +680,11 @@ ip4_start (NMDhcpClient *client,
 
 	/* Add requested options */
 	for (i = 0; dhcp4_requests[i].name; i++) {
-		if (dhcp4_requests[i].include)
-			sd_dhcp_client_set_request_option (sd_client, dhcp4_requests[i].num);
+		if (dhcp4_requests[i].include) {
+			nm_assert (dhcp4_requests[i].option_num <= 255);
+			r = sd_dhcp_client_set_request_option (sd_client, dhcp4_requests[i].option_num);
+			nm_assert (r >= 0 || r == -EEXIST);
+		}
 	}
 
 	hostname = nm_dhcp_client_get_hostname (client);
@@ -973,8 +985,10 @@ ip6_start (NMDhcpClient *client,
 
 	/* Add requested options */
 	for (i = 0; dhcp6_requests[i].name; i++) {
-		if (dhcp6_requests[i].include)
-			sd_dhcp6_client_set_request_option (sd_client, dhcp6_requests[i].num);
+		if (dhcp6_requests[i].include) {
+			r = sd_dhcp6_client_set_request_option (sd_client, dhcp6_requests[i].option_num);
+			nm_assert (r >= 0 || r == -EEXIST);
+		}
 	}
 
 	r = sd_dhcp6_client_set_local_address (sd_client, ll_addr);