If a valid connection was updated and still valid, and then was
updated and become invalid, the connection would not be properly
removed from the keyfile plugin's priv->connections hash, and thus
would never be disposed.
This was due to using the direct pointer to the connection's UUID
as the key for the hash table. When a connection is updated and
its settings are replaced, the old UUID is freed and replaced with
a new pointer. But the keyfile plugin hash table still uses the
old (now freed) UUID pointer as the key. Thus when the connection
is updated and becomes invalid, looking up the UUID in the hash
table fails to find the connection, and the connection is not
removed from the hash.
This bug could cause a crash in some cases, if two keys of the
GHashTable hashed to the same value, in which case GLib would
call g_str_equal() on the freed pointer.
Since code other than in the keyfile plugin replaces settings,
we cannot be guaranteed that the pointer won't change. Avoid all
that and just strdup() the UUID when using it as a key.
(also collapses _internal_new_connection() into its only caller)
Some tests want to assert against the messages logged using g_test_expect_message().
In this mode, nmtst will not log anything itself.
Interpret the option no-expect-message which turns g_test_expect_message()
into a NOP and turns logging on. The use of this is for debugging such
tests, without asserting against the messages but printing them instead.
For tests that are not in the assert_message mode, the option has no
effect.
Example:
NMTST_DEBUG=debug,no-expect-message make -C src/settings/plugins/keyfile/tests/ check
Signed-off-by: Thomas Haller <thaller@redhat.com>
In this mode, nmtst itself will not log anything and not set the logging
level. Also, it will set g_log_set_always_fatal().
This is for tests that want to assert against all logged messages via
g_test_expect_message().
In this mode also setting the logging level via NMTST_DEBUG variable has
no effect. The test is expected to manage the logging level itself and
changing the logging level might interfere with the test.
As a showcase, move keyfile/tests/test-keyfile.c to nmtst.
Signed-off-by: Thomas Haller <thaller@redhat.com>
When reading a hardware address in keyfile plugin, check for the
expected length already in mac_address_parser().
Before, we would call the deprecated function nm_utils_hwaddr_type()
to see if it can be some kind of MAC address. In that case, the error
was caught later during NMSetting:verify().
Signed-off-by: Thomas Haller <thaller@redhat.com>
When converting the MAC address to keyfile value, simply accept
any given byte array and pass it to nm_utils_hwaddr_ntoa_len().
This no longer restricts the length of accepted addresses as known by
nm_utils_hwaddr_type(). It is up to the caller to perform any validation
of the MAC address.
Signed-off-by: Thomas Haller <thaller@redhat.com>
- nm_utils_hwaddr_len() and nm_utils_hwaddr_type() no longer assert
against known input types/lengths. Now they can be used to detect the
hwaddr type, returning -1 on unknown.
- more checking of input arguments in nm_utils_hwaddr_aton() and
related. Also note, that nm_utils_hwaddr_aton_len() has @len of type
gsize, so we cannot pass on the output of nm_utils_hwaddr_len()
without checking for -1.
Signed-off-by: Thomas Haller <thaller@redhat.com>
Don't call nm_utils_hwaddr_type () with random len, because it causes ugly
(NetworkManager:25325): libnm-util-CRITICAL **: file nm-utils.c: line 1989 (nm_utils_hwaddr_type): should not be reached
And add a testcase.
https://bugzilla.gnome.org/show_bug.cgi?id=730514
test_read_wired_aliases_bad() would succeed or fail depending on the
order that ifcfg-aliasem1:1 and ifcfg-aliasem1:2 got read from disk.
Fix this by splitting it into two separate tests, each with only a
single alias.
This results in some nice coloring. Only move the tests that are called
without arguments from check-local to TESTS.
Signed-off-by: Thomas Haller <thaller@redhat.com>
Use g_test_expect_message() in the various daemon-side test programs,
to avoid spewing error messages when (successfully) running "make
check".
The ifnet and ifupdown plugins are extremely verbose, so they were
partially "fixed" by turning down the logging level from INFO to WARN
in those tests.
test-dhcp-options needed to be converted to gtestutils so that the
newly-added check in nm-dbus-manager would recognize it as a test
program and not try to create a private bus.
Remove the PLUGIN_PRINT() and PLUGIN_WARN() macros and use the
standard NM logging functions instead.
Also changed PLUGIN_PRINT("error: ...") to nm_log_warn("...") in
places.
Rebasing the shvar changes to master added some new instances of
svNewFile() and svWriteFile() (in the aliases code) that needed to be
updated for the API changes.
shvar.c was assuming it could do a single read() to read in the ifcfg
file, without taking partial reads or EINTR into account. Fix that.
Also, it was keeping the raw contents of the ifcfg file in the
shvarFile even though it never looked at it after svOpenFile().
(Presumably lineList originally consisted of pointers into arena, but
that had to be changed to support readwrite.) Fix that.
It would simplify things further to use g_file_get_contents() and
g_file_set_contents(), but the current code is perhaps more resilient
to symlink attacks because it keeps the fd open?
Fix a bug when reading an invalid alias file, where the code meant to
skip the rest of the loop iteration, but failed.
Also fix a memory leak and remove an unused variable.
Bugs noticed by coverity.
Notes and changes by jklimes:
- fix reading TeamPort without TYPE=Ethernet
- fix tests
Ideally this should be solved on initscripts side. But teamd doesn't want to do
any changes to initscripts, so we make a workaround here.
https://bugzilla.redhat.com/show_bug.cgi?id=1074160
connection_parser.c: In function 'make_ip4_setting':
connection_parser.c:660:33: error: 'method' may be used uninitialized in this function [-Werror=maybe-uninitialized]
if (!is_static_block && strstr (method, "dhcp")) {
connections.c: In function ‘load_cmd_line_edit_lib’:
connections.c:5744:17: error: ‘module’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
g_module_close (module);
Signed-off-by: Thomas Haller <thaller@redhat.com>
