We can't pass the password obtained from
nm_setting_802_1x_get_*private_key_password() to
nm_setting_802_1x_set_*private_key() as the latter also frees the old
password.
Fixes: afd2811028
The DNS manager and other singletons have the problem that
they are not properly destroyed on exit, that is, we leak
most of the instances. That should be eventually fixed and
all resources/memory should be released.
Anyway, fix the shutdown procedure by adding an explict command
nm_dns_manager_shutdown(). We should not rely on cleanup actions
to take place when the last reference is dropped, because then
we get complex interactions where we must ensure that everybody
drops the references at the right pointer.
Since the previous shutdown action was effectively never performed,
it is not quite clear what we actually want to do on shutdown.
For now, move the code to nm_dns_manager_stop(). We will see if
that is the desired behavior.
Also, as time goes by it is less likely to encounter a user
where the kernel has no support. The most likely reason nowadays
is that the user booted with "ipv6.disabled=1".
https://bugzilla.redhat.com/show_bug.cgi?id=1421019
We want to have some guaranteed order when comparing different connections.
So, in case of equal timestamps, proceed with comparing more properties.
It makes sense to consider the autoconnect-priority next.
This is what get_existing_connection() needs, thus we no longer
need to pre-sort the list.
NMPolicy's auto_activate_device() wants to sort by autoconnect-priority,
nm_utils_cmp_connection_by_autoconnect_priority() but fallback to the default
nm_settings_connection_cmp_default(), which includes the timestamp.
Extend nm_settings_connection_cmp_default() to consider the
autoconnect-priority as well. Thus change behavior so that
nm_settings_connection_cmp_default() is the sort order that
auto_activate_device() wants. That makes sense, as
nm_settings_connection_cmp_default() already considered the
ability to autoconnect as first. Hence, it should also honor
the autoconnect priority.
When doing that, rename nm_settings_connection_cmp_default()
to nm_settings_connection_cmp_autoconnect_priority().
We call these functions a lot. A GSList is just the wrong tool for the
job. Refactor the code to use instead a sorted array everywhere.
This means, we malloc() one array for all connections instead
slice-allocate a GSList item for each. Also, sorting an array
is faster then sorting a GSList.
Technically, the GSList implementation had the same big-O runtime
complexity, but using an array is still faster. That is, sorting
an array and a GSList is both O(n*log(n)).
Actually, nm_settings_get_connections_sorted() used
g_slist_insert_sorted() instead of g_slist_sort(). That results
in O(n^2). That could have been fixed to have O(n*log(n)), but
instead refactor the code to use an array.
nm_settings_get_best_connections() has only one caller: to create
the hidden-SSID list.
Instead of having a highly specialised function (that accepts 3 ways for
filtering -- one of them broken, has one hard-coded way of sorting, and
a @max_requested argument), add a more generic nm_settings_get_connections_clone()
function.
Also invert nm_settings_sort_connections(). The two callers want
to sort descending, not ascending.
Have a proper cmp() function and a wrapper *_p_with_data() that can be
used for g_qsort_with_data().
Thus, establish a naming scheme (*_p_with_data()) for these compare
wrappers that we need all over the place. Note, we also have
nm_strcmp_p_with_data() for the same reason and later more such
functions will follow.
scan_request_cb() handles the answer from the D-Bus "Scan" method.
At that point, the scan is not yet done, it merely started. It is
wrong to already signal SCAN_DONE.
The only place where we want to signal SCAN_DONE is when we actually
receive the "ScanDone" D-Bus signal.
In the SCAN_DONE handler, NMDeviceWifi resets the flag that indicates
that a current scan request is pending. We need to first obtain the
new APs (NEW_BSS) before signalling SCAN_DONE.
Cache the value for accessing the GObject property
NM_DEVICE_WIFI_SCANNING.
Re-evaluating the property every time by checking the
supplicant interface is ugly because it might change
under the hood. It should only change if (and only if)
we emit a notify changed signal.
Also, avoid accessing
nm_supplicant_interface_get_scanning (priv->sup_iface)
without checking whether priv->sup_iface is not NULL.
When we dump a list of APs, determine one timestamp for "now",
instead of re-evaluating it every time.
This ensures that all APs are printed with the same understanding
of the current timestamp.
LOGD_WIFI_SCAN is there to avoid flodding the log with continous scan
results. It should not be used for messages related to scheduling scan
requests.
This is especially important, because LOGD_WIFI_SCAN domain is not
included in LOGD_DEFAULT.
The _LOGD() macros of NMDeviceWifi print a logging context for each
line, that is, they add a prefix with the device name.
Replace nm_wifi_ap_dump() by nm_wifi_ap_to_string() and let device
log a message about the AP.
Also, update the format for printing the AP. Now, all fields are
separated by space.
- no longer bother clearing .state and .reason when the .id
field is unset. The fields just don't matter and no user
accesses these fields when the glib source id is not set.
- unify logging and give them all a prefix "queue-state[%s, %s, %u]: ".
- drop nm_device_queued_state_peek(), it only had one caller,
thus inline the trivial check.
- make nm_device_queued_state_clear() a static function
queued_state_clear()
- rename queued_set_state() to queued_state_set().
Reorder code to be like in other source files:
- first includes and generic defines
- then various helper structs
- then GObject related declarations, with first signal and property
enums, then the private data, then the G_DEFINE_TYPE() itself.
- finally, forward declarations for functions.
We don't want to waste a full "int" size to store the @hw_addr_type
in NMDevicePrivate. Previously, that was hacked around by using guint8.
Now, instead use a bitfield which has the right type.
These two structs are only used at exactly one place: as the type
for a field in NMDevicePrivate.
Having additional structs (that are only used at one place) only
add noise. Also, there are already prior-acts of using unnamed
structs in NMDevicePrivate in case of structs that only serve
to group/namespace a set of fields.
All callers either use a static @action argument or keep a clone
of the string that lives as long as the action is pending. So,
save cloning the string.
While we still recheck-available, we want to queue a pending action to block
startup-complete. However, we have to queue that before removing the pending
action for "wait for supplicant".
<debug> [...] device[0x563abbcca400] (wlp2s0): remove_pending_action (0): 'waiting for supplicant'
<info> [...] manager: startup complete
<debug> [...] device[0x563abbcca400] (wlp2s0): add_pending_action (1): 'queued state change to disconnected'
Startup-complete means that all devices have settled in a state
and no further activation is pending. When we have a recheck-available
scheduled, we clearly should not yet declare startup-complete.
Add a new pending-action "recheck-available" to avoid:
<info> [1485520408.3920] device (wlp2s0): supplicant interface state: starting -> ready
<debug> [1485520408.3920] device[0x563abbcca400] (wlp2s0): remove_pending_action (0): 'waiting for supplicant'
<info> [1485520408.3920] manager: startup complete
<debug> [1485520408.3924] device[0x563abbcca400] (wlp2s0): add_pending_action (1): 'queued state change to disconnected'
grep-ing for '\<scanning\>' yields 42 hits under src. But only 2 are actual
references to the "scanning" GObject property of NMDeviceWifi.
Use a #define with a unique name where we mean NMDeviceWifi's property.
I think NM_CACHED_QUARK_FCN() is better because:
- the implementation is in our hand, meaning it is clear that
putting a "static" before NM_CACHED_QUARK_FCN() is guaranteed to
work -- without relying on G_DEFINE_QUARK() to be defined in a way
that this works (in fact, we currently never do that and instead
make all functions non-static).
- it does not construct function names by appending "_quark".
Thus you can grep for the entire function name and finding
the place where it is implemented.
- same with the stings, where the new macro doesn't stringify the
argument, which is less surpising. Again, now you can grep
for the string including the double quoting.
(yes, I really use grep to understand the source-code)
NM_CACHED_QUARK_FCN() is a replacement for G_DEFINE_QUARK().
G_DEFINE_QUARK() is mostly used to define GError quarks. As
such, it always appends _quark() to the function name, which
is unfavorable because it makes it harder to grep for the
definition of the function.
In general I think that macros that defined symbols by concatenating
something should be avoided because that makes it harder to locate
where the symbol was defined.
`nm` is used by "tools/create-exports-NetworkManager.sh" script.
Alloc configuring an explicit path during configure.
BINUTILS_NM=/usr/bin/nm ./configure
ip4_addr_subnets_is_secondary() should fill the list of addresses in
the same subnet also when returning FALSE, because
nm_platform_ip4_address_sync() uses it.
Fixes: 2f68a50041
