dhcp: refactor nm_dhcp_dhclient_save_duid() to accept original DUID

There should be lower layers that are concerned with writing
and reading dhclient configuration files. It's wrong to
have a nm_dhcp_dhclient_save_duid() function which requires
the caller to pre-escape the string to write. The caller shouldn't
be concerned with the file format, that's why the function
is used in the first place.

4 files changed, 49 insertions, 38 deletions

diff --git a/src/dhcp/nm-dhcp-dhclient-utils.c b/src/dhcp/nm-dhcp-dhclient-utils.c
index fdfc2a408a..a6dad6fab8 100644
--- a/src/dhcp/nm-dhcp-dhclient-utils.c
+++ b/src/dhcp/nm-dhcp-dhclient-utils.c
@@ -584,9 +584,10 @@ nm_dhcp_dhclient_read_duid (const char *leasefile, GError **error)
 
 gboolean
 nm_dhcp_dhclient_save_duid (const char *leasefile,
-                            const char *escaped_duid,
+                            GBytes *duid,
                             GError **error)
 {
+	gs_free char *escaped_duid = NULL;
 	gs_strfreev char **lines = NULL;
 	char **iter, *l;
 	GString *s;
@@ -594,6 +595,14 @@ nm_dhcp_dhclient_save_duid (const char *leasefile,
 	gsize len = 0;
 
 	g_return_val_if_fail (leasefile != NULL, FALSE);
+
+	if (!duid) {
+		nm_utils_error_set_literal (error, NM_UTILS_ERROR_UNKNOWN,
+		                            "missing duid");
+		g_return_val_if_reached (FALSE);
+	}
+
+	escaped_duid = nm_dhcp_dhclient_escape_duid (duid);
 	g_return_val_if_fail (escaped_duid != NULL, FALSE);
 
 	if (g_file_test (leasefile, G_FILE_TEST_EXISTS)) {
diff --git a/src/dhcp/nm-dhcp-dhclient-utils.h b/src/dhcp/nm-dhcp-dhclient-utils.h
index 911fece488..57a711db14 100644
--- a/src/dhcp/nm-dhcp-dhclient-utils.h
+++ b/src/dhcp/nm-dhcp-dhclient-utils.h
@@ -40,7 +40,7 @@ GBytes *nm_dhcp_dhclient_unescape_duid (const char *duid);
 GBytes *nm_dhcp_dhclient_read_duid (const char *leasefile, GError **error);
 
 gboolean nm_dhcp_dhclient_save_duid (const char *leasefile,
-                                     const char *escaped_duid,
+                                     GBytes *duid,
                                      GError **error);
 
 #endif /* __NETWORKMANAGER_DHCP_DHCLIENT_UTILS_H__ */
diff --git a/src/dhcp/nm-dhcp-dhclient.c b/src/dhcp/nm-dhcp-dhclient.c
index bb8892f106..9e8dd6a5f3 100644
--- a/src/dhcp/nm-dhcp-dhclient.c
+++ b/src/dhcp/nm-dhcp-dhclient.c
@@ -418,10 +418,7 @@ dhclient_start (NMDhcpClient *client,
 
 	/* Save the DUID to the leasefile dhclient will actually use */
 	if (addr_family == AF_INET6) {
-		gs_free char *escaped = NULL;
-
-		escaped = nm_dhcp_dhclient_escape_duid (duid);
-		if (!nm_dhcp_dhclient_save_duid (priv->lease_file, escaped, &local)) {
+		if (!nm_dhcp_dhclient_save_duid (priv->lease_file, duid, &local)) {
 			nm_utils_error_set (error,
 			                    NM_UTILS_ERROR_UNKNOWN,
 			                    "failed to save DUID to '%s': %s",
diff --git a/src/dhcp/tests/test-dhcp-dhclient.c b/src/dhcp/tests/test-dhcp-dhclient.c
index edac42572d..ab1f5551d4 100644
--- a/src/dhcp/tests/test-dhcp-dhclient.c
+++ b/src/dhcp/tests/test-dhcp-dhclient.c
@@ -760,62 +760,74 @@ test_read_commented_duid_from_leasefile (void)
 	g_assert (duid == NULL);
 }
 
+/*****************************************************************************/
+
+static void
+_save_duid (const char *path,
+            const guint8 *duid_bin,
+            gsize duid_len)
+{
+	gs_unref_bytes GBytes *duid = NULL;
+	GError *error = NULL;
+	gboolean success;
+
+	g_assert (path);
+	g_assert (duid_bin);
+	g_assert (duid_len > 0);
+
+	duid = g_bytes_new (duid_bin, duid_len);
+	success = nm_dhcp_dhclient_save_duid (path, duid, &error);
+	nmtst_assert_success (success, error);
+}
+
 static void
 test_write_duid (void)
 {
-	const char *duid = "\\000\\001\\000\\001\\027X\\350X\\000#\\025\\010~\\254";
+	const guint8 duid[] = { 000, 001, 000, 001, 027, 'X', 0350, 'X', 0, '#', 025, 010, '~', 0254 };
 	const char *expected_contents = "default-duid \"\\000\\001\\000\\001\\027X\\350X\\000#\\025\\010~\\254\";\n";
 	GError *error = NULL;
-	char *contents = NULL;
+	gs_free char *contents = NULL;
 	gboolean success;
 	const char *path = "test-dhclient-write-duid.leases";
 
-	success = nm_dhcp_dhclient_save_duid (path, duid, &error);
-	g_assert_no_error (error);
-	g_assert (success);
+	_save_duid (path, duid, G_N_ELEMENTS (duid));
 
 	success = g_file_get_contents (path, &contents, NULL, &error);
-	g_assert_no_error (error);
-	g_assert (success);
+	nmtst_assert_success (success, error);
 
 	unlink (path);
-	g_assert_cmpstr (expected_contents, ==, contents);
 
-	g_free (contents);
+	g_assert_cmpstr (expected_contents, ==, contents);
 }
 
 static void
 test_write_existing_duid (void)
 {
-	const char *duid = "\\000\\001\\000\\001\\023o\\023n\\000\\\"\\372\\214\\326\\302";
+	const guint8 duid[] = { 000, 001, 000, 001, 023, 'o', 023, 'n', 000, '\"', 0372, 0214, 0326, 0302 };
 	const char *original_contents = "default-duid \"\\000\\001\\000\\001\\027X\\350X\\000#\\025\\010~\\254\";\n";
 	const char *expected_contents = "default-duid \"\\000\\001\\000\\001\\023o\\023n\\000\\\"\\372\\214\\326\\302\";\n";
 	GError *error = NULL;
-	char *contents = NULL;
+	gs_free char *contents = NULL;
 	gboolean success;
 	const char *path = "test-dhclient-write-existing-duid.leases";
 
 	success = g_file_set_contents (path, original_contents, -1, &error);
-	g_assert_no_error (error);
-	g_assert (success);
+	nmtst_assert_success (success, error);
 
 	/* Save other DUID; should be overwritten */
-	success = nm_dhcp_dhclient_save_duid (path, duid, &error);
-	g_assert_no_error (error);
-	g_assert (success);
+	_save_duid (path, duid, G_N_ELEMENTS (duid));
 
 	/* reread original contents */
 	success = g_file_get_contents (path, &contents, NULL, &error);
-	g_assert_no_error (error);
-	g_assert (success);
+	nmtst_assert_success (success, error);
 
 	unlink (path);
 	g_assert_cmpstr (expected_contents, ==, contents);
-
-	g_free (contents);
 }
 
+static const guint8 DUID_BIN[] = { 000, 001, 000, 001, 023, 'o', 023, 'n', 000, '\"', 0372, 0214, 0326, 0302 };
 #define DUID "\\000\\001\\000\\001\\023o\\023n\\000\\\"\\372\\214\\326\\302"
+
 static void
 test_write_existing_commented_duid (void)
 {
@@ -824,28 +836,22 @@ test_write_existing_commented_duid (void)
 	    "default-duid \"" DUID "\";\n"
 	    ORIG_CONTENTS;
 	GError *error = NULL;
-	char *contents = NULL;
+	gs_free char *contents = NULL;
 	gboolean success;
 	const char *path = "test-dhclient-write-existing-commented-duid.leases";
 
 	success = g_file_set_contents (path, ORIG_CONTENTS, -1, &error);
-	g_assert_no_error (error);
-	g_assert (success);
+	nmtst_assert_success (success, error);
 
 	/* Save other DUID; should be saved on top */
-	success = nm_dhcp_dhclient_save_duid (path, DUID, &error);
-	g_assert_no_error (error);
-	g_assert (success);
+	_save_duid (path, DUID_BIN, G_N_ELEMENTS (DUID_BIN));
 
 	/* reread original contents */
 	success = g_file_get_contents (path, &contents, NULL, &error);
-	g_assert_no_error (error);
-	g_assert (success);
+	nmtst_assert_success (success, error);
 
 	unlink (path);
 	g_assert_cmpstr (expected_contents, ==, contents);
-
-	g_free (contents);
 #undef ORIG_CONTENTS
 }
 
@@ -865,8 +871,7 @@ test_write_existing_multiline_duid (void)
 	success = g_file_set_contents (path, ORIG_CONTENTS, -1, &error);
 	nmtst_assert_success (success, error);
 
-	success = nm_dhcp_dhclient_save_duid (path, DUID, &error);
-	nmtst_assert_success (success, error);
+	_save_duid (path, DUID_BIN, G_N_ELEMENTS (DUID_BIN));
 
 	success = g_file_get_contents (path, &contents, NULL, &error);
 	nmtst_assert_success (success, error);