The g_clear_pointer() macro already contains a cast to GDestroyNotify. No
need to do it ourself. In fact, with the cast, this only works with the
particular g_clear_pointer() implementation, that first assigns the
destroy function to a local variable.
See-also: https://bugzilla.gnome.org/show_bug.cgi?id=674634#c52
Replace the usage of g_str_hash() with our own nm_str_hash().
GLib's g_str_hash() uses djb2 hashing function, just like we
do at the moment. The only difference is, that we use a diffrent
seed value.
Note, that we initialize the hash seed with random data (by calling
getrandom() or reading /dev/urandom). That is a change compared to
before.
This change of the hashing function and accessing the random pool
might be undesired for libnm/libnm-core. Hence, the change is not
done there as it possibly changes behavior for public API. Maybe
we should do that later though.
At this point, there isn't much of a change. This patch becomes
interesting, if we decide to use a different hashing algorithm.
Just because the user requested a scan doesn't mean the supplicant should
use the result of that scan to jump to an AP that's slightly better than
the current one. Let the supplicant handle when it's supposed to roam
based on it's own logic, not random scans from users or NM clients.
Don't have pending asynchronous requests in parallel, like setting
"ProcessCredentials" and "Start", or "Cancel" and "Start".
Instead, "Start" is only scheduled after "ProcessCredentials" completed
and "ProcessCredentials" is only scheduled after "Cancel" completed.
Also, handle the async response of these requests. For one, to achive the
chaining mentioned above and to log what happens and possible errors.
Upon new enrollment, a previously created GDBusProxy is now reused,
where the first operation is to Cancel the previous action.
Also, consistently <trace> log what is happening.
Not doing all of this is less lines of code. It's also simpler, and
faster. But in my opinion, it is (usually) better to check and wait for
return values, instead of firing off async requests uncontrolled. It
allows us to better know where we are and to log about each individual
step. This also makes all operations cancellable.
Undoubtedly, correctness and handling failures conflicts with simplicity
in this case -- or at least: what I think is "correctness" conflicts.
NetworkManager-1.8.0/src/supplicant/nm-supplicant-interface.c:232: check_return: Calling "g_async_initable_init_finish" without checking return value (as is done elsewhere 7 out of 8 times).
Now that we have a PMF connection property, get rid of the previous
code to globally enable/disable PMF and use the 'ieee80211w'
configuration option for each configured network when the supplicant
supports it.
We initialize the BSS asyncronously. Don't declare SCAN_DONE
until all BSS are up.
Otherwise, especially during the very first scan we declare SCAN_DONE
when having no BSS yet. This wrongly removes the pending action
"wifi-scan", while "autoconnect" cannot happen as there are not BSS
yet. Thus we declare "startup-complete" too early.
Another issue is that we may start autoconnecting with an incomplete
scan list, and thus pick a non-preferred connections.
https://bugzilla.gnome.org/show_bug.cgi?id=777831
Before, the NEW_BSS signal was not careful to emit the signal only when the BSS
is seen for the first time. Consequently, supplicant_iface_new_bss_cb() checked
whether it already knows about the new BSS.
Merge NEW_BSS and BSS_UPDATED. Now we emit BSS_UPDATED when either the
BSS is new or changed.
Also, in supplicant_iface_new_bss_cb() (now supplicant_iface_bss_updated_cb())
no longer constructs an @ap instance if we have a @found_ap.
In some situations there can be a value of having a separate ADD signal.
But only when there the consumers care, and if the consumers can trust that
ADD is not just an UPDATE. The only consumer doesn't care and it not not be
trusted, so merge the signals.
If the assoc-request is cancelled while an "AddNetwork" request is
pending, we must cleanup the added network when the request succeeds.
The issue can also happen when NetworkManager shuts down and exits
the mainloop. This scenario is unsolved as the cleanup action
"RemoveNetwork" has no chance to run.
"AddBlob" works differently in that blogs are added with a specific name,
not like "AddNetwork", where a new D-Bus path is created. Maybe we should
also cleanup blobs that were added by us, but currently we don't.
Instead of having a NM_SUPPLICANT_INTERFACE_CONNECTION_ERROR signal to notify
about failures during AddNetwork/SelectNetwork, accept a callback to report
success/failure.
Thereby, rename nm_supplicant_interface_set_config() to
nm_supplicant_interface_assoc().
The async callback is guaranteed to:
- be invoked exactly once, signalling success or failure
- always being invoked asyncronously.
The pending request can be (synchronously) cancelled via
nm_supplicant_interface_disconnect() or by disposing the
interface instance. In those cases the callback will be invoked
too, with error code cancelled/disposing.
Also change the signature of the NM_SUPPLICANT_INTERFACE_STATE signal,
to have three "int" type arguments. Thereby also fix the subscribers
to this signal that wrongly had type guint32, instead of guint
(which happens to be the same underlying type, so no real problem).
https://mail.gnome.org/archives/networkmanager-list/2017-February/msg00021.html
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.
We no longer use wpa_supplicant for MAC address randomization. Instead, NetworkManager
handles it on it's own. It is actually important that supplicant does not interfere
when setting the MAC address of the device.
The code was only in effect when supplicant has a PreassocMacAddr property.
As this is a recent feature, the left-over code wasn't noticed until now.
https://mail.gnome.org/archives/networkmanager-list/2017-February/msg00003.html
Fixes: 767abfa690
As the fast-supported flag changes, update the existing supplicant
interfaces with the new information.
Also, by default assume it is supported.
(cherry picked from commit 872b9ec5ea)
