From 017a4b274f1fbb8fcc42e9926323ab78f584e5f4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 24 Aug 2019 13:31:19 +0200 Subject: [PATCH 1/3] ifupdown/tests: cleanup tests by freeing Expected variable with nm_auto() --- .../plugins/ifupdown/tests/test-ifupdown.c | 66 ++++++------------- 1 file changed, 21 insertions(+), 45 deletions(-) diff --git a/src/settings/plugins/ifupdown/tests/test-ifupdown.c b/src/settings/plugins/ifupdown/tests/test-ifupdown.c index 4adcf085e5..8e2c62f918 100644 --- a/src/settings/plugins/ifupdown/tests/test-ifupdown.c +++ b/src/settings/plugins/ifupdown/tests/test-ifupdown.c @@ -149,6 +149,9 @@ expected_free (Expected *e) g_free (e); } +NM_AUTO_DEFINE_FCN_VOID0 (Expected *, _nm_auto_free_expected, expected_free) +#define nm_auto_free_expected nm_auto(_nm_auto_free_expected) + static void compare_expected_to_ifparser (if_parser *parser, Expected *e) { @@ -226,7 +229,7 @@ init_ifparser_with_file (const char *file) static void test1_ignore_line_before_first_block (void) { - Expected *e; + nm_auto_free_expected Expected *e = NULL; ExpectedBlock *b; nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test1"); @@ -238,14 +241,12 @@ test1_ignore_line_before_first_block (void) expected_block_add_key (b, expected_key_new ("inet", "dhcp")); compare_expected_to_ifparser (parser, e); - - expected_free (e); } static void test2_wrapped_line (void) { - Expected *e; + nm_auto_free_expected Expected *e = NULL; ExpectedBlock *b; nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test2"); @@ -254,14 +255,12 @@ test2_wrapped_line (void) expected_add_block (e, b); compare_expected_to_ifparser (parser, e); - - expected_free (e); } static void test3_wrapped_multiline_multiarg (void) { - Expected *e; + nm_auto_free_expected Expected *e = NULL; ExpectedBlock *b; nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test3"); @@ -274,14 +273,12 @@ test3_wrapped_multiline_multiarg (void) expected_add_block (e, b); compare_expected_to_ifparser (parser, e); - - expected_free (e); } static void test4_allow_auto_is_auto (void) { - Expected *e; + nm_auto_free_expected Expected *e = NULL; ExpectedBlock *b; nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test4"); @@ -290,14 +287,12 @@ test4_allow_auto_is_auto (void) expected_add_block (e, b); compare_expected_to_ifparser (parser, e); - - expected_free (e); } static void test5_allow_auto_multiarg (void) { - Expected *e; + nm_auto_free_expected Expected *e = NULL; ExpectedBlock *b; nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test5"); @@ -308,14 +303,12 @@ test5_allow_auto_multiarg (void) expected_add_block (e, b); compare_expected_to_ifparser (parser, e); - - expected_free (e); } static void test6_mixed_whitespace (void) { - Expected *e; + nm_auto_free_expected Expected *e = NULL; ExpectedBlock *b; nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test6"); @@ -325,8 +318,6 @@ test6_mixed_whitespace (void) expected_add_block (e, b); compare_expected_to_ifparser (parser, e); - - expected_free (e); } static void @@ -348,7 +339,7 @@ test8_long_line_wrapped (void) static void test9_wrapped_lines_in_block (void) { - Expected *e; + nm_auto_free_expected Expected *e = NULL; ExpectedBlock *b; nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test9"); @@ -362,14 +353,12 @@ test9_wrapped_lines_in_block (void) expected_block_add_key (b, expected_key_new ("gateway", "10.250.2.50")); compare_expected_to_ifparser (parser, e); - - expected_free (e); } static void test11_complex_wrap (void) { - Expected *e; + nm_auto_free_expected Expected *e = NULL; ExpectedBlock *b; nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test11"); @@ -380,14 +369,12 @@ test11_complex_wrap (void) expected_block_add_key (b, expected_key_new ("pre-up", "/sbin/ifconfig eth0 up")); compare_expected_to_ifparser (parser, e); - - expected_free (e); } static void test12_complex_wrap_split_word (void) { - Expected *e; + nm_auto_free_expected Expected *e = NULL; ExpectedBlock *b; nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test12"); @@ -398,14 +385,12 @@ test12_complex_wrap_split_word (void) expected_block_add_key (b, expected_key_new ("up", "ifup ppp0=dsl")); compare_expected_to_ifparser (parser, e); - - expected_free (e); } static void test13_more_mixed_whitespace (void) { - Expected *e; + nm_auto_free_expected Expected *e = NULL; ExpectedBlock *b; nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test13"); @@ -415,14 +400,12 @@ test13_more_mixed_whitespace (void) expected_add_block (e, b); compare_expected_to_ifparser (parser, e); - - expected_free (e); } static void test14_mixed_whitespace_block_start (void) { - Expected *e; + nm_auto_free_expected Expected *e = NULL; ExpectedBlock *b; nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test14"); @@ -438,14 +421,12 @@ test14_mixed_whitespace_block_start (void) expected_add_block (e, b); compare_expected_to_ifparser (parser, e); - - expected_free (e); } static void test15_trailing_space (void) { - Expected *e; + nm_auto_free_expected Expected *e = NULL; ExpectedBlock *b; nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test15"); @@ -455,23 +436,20 @@ test15_trailing_space (void) expected_add_block (e, b); compare_expected_to_ifparser (parser, e); - - expected_free (e); } static void test16_missing_newline (void) { - Expected *e; + nm_auto_free_expected Expected *e = NULL; nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test16"); e = expected_new (); expected_add_block (e, expected_block_new ("mapping", "eth0")); compare_expected_to_ifparser (parser, e); - - expected_free (e); } + static void test17_read_static_ipv4 (void) { @@ -578,7 +556,7 @@ test19_read_static_ipv4_plen (void) static void test20_source_stanza (void) { - Expected *e; + nm_auto_free_expected Expected *e = NULL; ExpectedBlock *b; nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test20-source-stanza"); @@ -597,14 +575,12 @@ test20_source_stanza (void) expected_block_add_key (b, expected_key_new ("inet", "dhcp")); compare_expected_to_ifparser (parser, e); - - expected_free (e); } static void test21_source_dir_stanza (void) { - Expected *e; + nm_auto_free_expected Expected *e = NULL; ExpectedBlock *b; nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test21-source-dir-stanza"); @@ -617,10 +593,10 @@ test21_source_dir_stanza (void) expected_block_add_key (b, expected_key_new ("inet", "dhcp")); compare_expected_to_ifparser (parser, e); - - expected_free (e); } +/*****************************************************************************/ + NMTST_DEFINE (); int From a49027ab22aa8a6a24463e1baa3b0080d9536966 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 24 Aug 2019 13:38:19 +0200 Subject: [PATCH 2/3] ifupdown/tests: add test with duplicate interfaces This file causes a crash [1], add it to the tests. Note that the test only check parsing the file and the crash happens in the "upper" layers. So, it's not really a test for the crash. But at least have such a file in our repository. [1] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/235 --- Makefile.am | 1 + .../plugins/ifupdown/tests/test-ifupdown.c | 25 +++++++++++++++++++ .../ifupdown/tests/test22-duplicate-stanzas | 8 ++++++ 3 files changed, 34 insertions(+) create mode 100644 src/settings/plugins/ifupdown/tests/test22-duplicate-stanzas diff --git a/Makefile.am b/Makefile.am index 883cc0ea9d..03a35a617b 100644 --- a/Makefile.am +++ b/Makefile.am @@ -3215,6 +3215,7 @@ EXTRA_DIST += \ src/settings/plugins/ifupdown/tests/test20-source-stanza.eth1 \ src/settings/plugins/ifupdown/tests/test21-source-dir-stanza \ src/settings/plugins/ifupdown/tests/test21-source-dir-stanza.d \ + src/settings/plugins/ifupdown/tests/test22-duplicate-stanzas \ src/settings/plugins/ifupdown/meson.build \ src/settings/plugins/ifupdown/tests/meson.build diff --git a/src/settings/plugins/ifupdown/tests/test-ifupdown.c b/src/settings/plugins/ifupdown/tests/test-ifupdown.c index 8e2c62f918..1e4341e945 100644 --- a/src/settings/plugins/ifupdown/tests/test-ifupdown.c +++ b/src/settings/plugins/ifupdown/tests/test-ifupdown.c @@ -595,6 +595,30 @@ test21_source_dir_stanza (void) compare_expected_to_ifparser (parser, e); } +static void +test22_duplicate_stanzas (void) +{ + nm_auto_free_expected Expected *e = NULL; + ExpectedBlock *b; + nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test22-duplicate-stanzas"); + + e = expected_new (); + + b = expected_block_new ("iface", "br10"); + expected_add_block (e, b); + expected_block_add_key (b, expected_key_new ("inet", "manual")); + expected_block_add_key (b, expected_key_new ("bridge-ports", "enp6s0.15")); + expected_block_add_key (b, expected_key_new ("bridge-stp", "off")); + expected_block_add_key (b, expected_key_new ("bridge-maxwait", "0")); + expected_block_add_key (b, expected_key_new ("bridge-fd", "0")); + b = expected_block_new ("iface", "br10"); + expected_add_block (e, b); + expected_block_add_key (b, expected_key_new ("inet", "auto")); + expected_block_add_key (b, expected_key_new ("bridge-ports", "enp6s0.15")); + + compare_expected_to_ifparser (parser, e); +} + /*****************************************************************************/ NMTST_DEFINE (); @@ -626,6 +650,7 @@ main (int argc, char **argv) g_test_add_func ("/ifupdate/read_static_ipv4_plen", test19_read_static_ipv4_plen); g_test_add_func ("/ifupdate/source_stanza", test20_source_stanza); g_test_add_func ("/ifupdate/source_dir_stanza", test21_source_dir_stanza); + g_test_add_func ("/ifupdate/test22-duplicate-stanzas", test22_duplicate_stanzas); return g_test_run (); } diff --git a/src/settings/plugins/ifupdown/tests/test22-duplicate-stanzas b/src/settings/plugins/ifupdown/tests/test22-duplicate-stanzas new file mode 100644 index 0000000000..c13c2e7ebc --- /dev/null +++ b/src/settings/plugins/ifupdown/tests/test22-duplicate-stanzas @@ -0,0 +1,8 @@ +iface br10 inet manual + bridge_ports enp6s0.15 + bridge_stp off + bridge_maxwait 0 + bridge_fd 0 + +iface br10 inet auto + bridge_ports enp6s0.15 From e9ccc2da1926d969ffc95c469b8303877562302a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 24 Aug 2019 13:44:14 +0200 Subject: [PATCH 3/3] ifupdown: fix crash loading ifupdown settings with empty entries like bridge-ports and mappings Fixes: d35d3c468a30 ('settings: rework tracking settings connections and settings plugins') https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/235 --- src/settings/plugins/ifupdown/nms-ifupdown-plugin.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c index 475ecbb6d9..e663ab8a67 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c @@ -89,6 +89,8 @@ static GHashTable *load_eni_ifaces (NMSIfupdownPlugin *self); static void _storage_data_destroy (StorageData *sd) { + if (!sd) + return; nm_g_object_unref (sd->connection); nm_g_object_unref (sd->storage); g_slice_free (StorageData, sd);