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)
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
- use gs_free attribute
- move printing the logging cache warning inside the place
where we actuall add a new item to the cache.
It's really a minor cleanup of stuff that come to my mind reviewing the
function.
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
./src/nm-config-data.h:163:1: error: 'const' type qualifier on return type has no effect [-Werror,-Wignored-qualifiers]
const guint nm_config_data_get_connectivity_interval (const NMConfigData *config_data);
^~~~~~
The -Wimplicit-fallthrough=3 warning is quite flexible of accepting
a fall-through warning.
Some comments were missing or not detected correctly.
Thereby, also change all other comments to follow the exact
same pattern.
It's not used anymore. Which is a good thing, because if it was used
we'd have to get rid of the uses.
It did accept a whitespace separated string for an argument, which is
never useful for us; it indicated error either on g_spawn_sync()
failure or an error status code of the program spawned, but only set the
error in the former case which had let to errors.
The would would be a bit nicer place without it.
(But not much)
nm_spawn_process() only sets error if the g_spawn_sync() itself fails,
not when the program ran returns a non-zero code.
<debug> [148 059915.1567] dns-mgr: update-dns: updating resolv.conf
<info> [148 059915.1568] dns-mgr: Removing DNS information from /usr/bin/resolvconf
No resolv.conf for interface NetworkManager
Thread 1 "NetworkManager" received signal SIGSEGV, Segmentation fault.
0x0000555555 7c325 in nm_dns_manager_end_updates
1532 _LOGW ("could not commit DNS changes: %s", error->message);
(gdb) bt full
#0 0x0000555555 7c325 in nm_dns_manager_end_updates
error = 0x0
With commit 4f6c91d696, we aimed to
enable mac-address-randomization by default for Wi-Fi. That however
is not possible by default because it breaks various scenarios.
Also, later wifi.mac-address-randomization was deprecated in favor
of wifi.cloned-mac-address setting.
Both wifi.mac-address-randomization and wifi.cloned-mac-address support
global default values, so it is wrong to read a missing
MAC_ADDRESS_RANDOMIZATION setting as "NEVER" -- which due to
normalization also results in cloned-mac-address=permanent.
See also commit 46d53e1101 which does
something similar for keyfile.
This bug also prevents a user from clearing the cloned-mac-address:
$ nmcli connection show "$CONN"
...
802-11-wireless.cloned-mac-address: permanent
802-11-wireless.mac-address-randomization: never
...
$ nmcli connection modify "$CONN: wifi.cloned-mac-address ''
# ^ takes no effect
As workaround, you also need to clear mac-address-randomization:
$ nmcli connection modify "$CONN: wifi.cloned-mac-address '' \
wifi.mac-address-randomization default
https://mail.gnome.org/archives/networkmanager-list/2017-January/msg00060.html
