Hook the signal handlers right before the main loop. Prior to that
the default handlers are good enough and our one crashes (due to
loop being instantialized).
Also, set the return value properly to indicate a termination by a
signal.
When pushing a warning disable with clang, always disable
-Wunknown-warning-option first -- it might be that clang wouldn't warn
of what we're trying to disable because it doesn't recognize it in the
first place. That is entierely okay.
With clang-5.0.0:
CC libnm/tests/libnm_tests_test_secret_agent-test-secret-agent.o
In file included from libnm/tests/test-secret-agent.c:29:
In file included from ./shared/nm-test-libnm-utils.h:23:
./shared/nm-utils/nm-test-utils.h:432:3: error: unknown warning group '-Wunused-but-set-variable', ignored [-Werror,-Wunknown-warning-option]
NM_PRAGMA_WARNING_DISABLE("-Wunused-but-set-variable")
^
./shared/nm-utils/nm-macros-internal.h:223:9: note: expanded from macro 'NM_PRAGMA_WARNING_DISABLE'
_Pragma(_NM_PRAGMA_WARNING_DO(warning))
^
<scratch space>:204:25: note: expanded from here
GCC diagnostic ignored "-Wunused-but-set-variable"
^
1 error generated.
Must always initialize cleanup variable, to be able to build with
"-fexceptions".
make[2]: Entering directory './contrib/fedora/rpm/NetworkManager.20180124-060444.C5tHCi/BUILD/NetworkManager-1.11.1'
CC src/devices/wifi/src_devices_wifi_libnm_device_plugin_wifi_la-nm-device-iwd.lo
In file included from ./shared/nm-utils/nm-glib.h:27:0,
from ./shared/nm-utils/nm-macros-internal.h:60,
from ./shared/nm-default.h:257,
from src/devices/wifi/nm-device-iwd.c:21:
src/devices/wifi/nm-device-iwd.c: In function ‘deactivate_async_finish’:
./shared/nm-utils/gsystem-local-alloc.h:37:8: error: ‘variant’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
if (*(Type*)v) \
^
src/devices/wifi/nm-device-iwd.c:405:29: note: ‘variant’ was declared here
gs_unref_variant GVariant *variant;
^~~~~~~
Fixes: d0c1e1a62a
XXX was used to either raise attention (NOTE) or to indicate
that this is ugly code that should be fixed (FIXME). The usage
was inconsistent.
Let's avoid XXX and use either NOTE or FIXME.
NM_API_VERSION is a better name. It's not the current stable
version, but the version number of the API which the current
NM_VERSION provides. In practice, NM_API_VERSION is either identical
to NM_VERSION (in case of a release) or NM_VERSION is a development
version leading up the the upcoming NM_API_VERSION.
For example, with the new name the check
#if NM_VERSION != NM_API_VERSION
# warning this is an development version
#endif
makes more sense.
We have a well defined versioning scheme and a defined way how
the version number indicates a stable version. We can simply
calculate NM_VERSION_CUR_STABLE based on the version numbers.
It already defaults to the right value. We only need to define
NM_VERSION_MIN_REQUIRED, so that parts of our internal build
can make use of deprecated API.
We don't need to have two version defines "CUR" and "NEXT".
The main purpose of these macros (if not their only), is to
make NM_AVAILABLE_IN_* and NM_DEPRECATED_IN_* macros work.
1) At the precise commit of a release, "CUR" and "NEXT" must be identical,
because whenever the user configures NM_VERSION_MIN_REQUIRED and
NM_VERSION_MAX_ALLOWED, then they both compare against the current
version, at which point "CUR" == "NEXT".
2) Every other commit aside the release, is a development version that leads
up the the next coming release. But as far as versioning is concerned, such
a development version should be treated like that future release. It's unstable
API and it may or may not be close to later API of the release. But
we shall treat it as that version. Hence, also in this case, we want to
set both "NM_VERSION_CUR_STABLE" and again NEXT to the future version.
This makes NM_VERSION_NEXT_STABLE redundant.
Previously, the separation between current and next version would for
example allow that NM_VERSION_CUR_STABLE is the previously release
stable API, and NM_VERSION_NEXT_STABLE is the version of the next upcoming
stable API. So, we could allow "examples" to make use of development
API, but other(?) internal code still restrict to unstable API. But it's
unclear which other code would want to avoid current development.
Also, the points 1) and 2) were badly understood. Note that for our
previousy releases, we usually didn't bump the macros at the stable
release (and if we did, we didn't set them to be the same). While using
two macros might be more powerful, it is hard to grok and easy to
forget to bump the macros a the right time. One macro shall suffice.
All this also means, that *immediately* after making a new release, we shall
bump the version number in `configure.ac` and "NM_VERSION_CUR_STABLE".
We can disable/enable configuration snippets per NetworkManager
version. But we must compare it against the current version
that we build, not the current API version.
In a previous patch I added deactivate_async to make sure that NM
auto re-connect waits for the IWD state to changed from "disconnecting"
to "disconnected" before starting a new activation when the user wants
to switch from one profile to another. This doesn't account for when
IWD itself goes into "disconnecting" because of a connect failure.
When IWD goes into the "disconnecting" state we call
nm_device_state_changed (NM_DEVICE_STATE_FAILED,
NM_DEVICE_STATE_REASON_SUPPLICANT_DISCONNECT)
immediately to give feedback to user as soon as possible. We will
return FALSE from get_autoconnect_allowed for the period the
"disconnecting" state.
Deactivating the connection translates to a Device.Disconnect dbus call
to IWD. The dbus call normally returns immediately because the
corresponding nl80211 command returns immediately but we can't rely on
that. Make sure that NMDevice waits for the IWD method return before
continuing with the new activation request when switching wifi networks.
The handler would until now check if nm_device_is_activating() was true
or the NMDevice state was "activated" to decide whether to report to
NMDevice that we'd been unexpectedly disconnected (i.e. connection
failed). However NM's "prepare" and "need auth" states correspond to
IWD's "disconnected" state because they don't involve Wifi
authentication/association.
Additionally nm_device_is_activating() returns TRUE even when NMDevice
state is "disconnected" but an activation request is pending. As a
result when switching networks, NMDevice would first save the activation
request and go into the "disconnected" state, we'd then call the IWD's
Disconnect method and when we received the IWD state change notification
to "disconnected", we'd cause the pending activation request to be
considered a failure. The handler shouldn't report a failed
connection when the NMDevice state is "disconnected".
We already avoid committing the IP configuration for external devices
(see commit 60334a2893). However, we still start DHCP/IPv6-autoconf
and, especially, we change sysctl values of the device.
To be sure that no action is taken on the device, return early from
the IP configuration phase, as in the method=disabled/ignore case.
https://bugzilla.redhat.com/show_bug.cgi?id=1530288
yes, this is not an issue in practice. Variadic arguments are always
propagated to at least int/unsigned type. And kernel and glib both require
sizeof(guint32) <= sizeof(guint). Hence, this was safe on any supported
architecture. Still, let's be explicit about the types.
Previouslly, the value of ieee80211w and key_mgmt field in
wpa_supplicant.conf was defined by the value of pmf.
NM_SETTING_WIRELESS_SECURITY_PMF_DISABLE
ieee80211w=0
key_mgmt=wpa-eap
NM_SETTING_WIRELESS_SECURITY_PMF_OPTIONAL
ieee80211w=1
key_mgmt=wpa-eap wpa-eap-sha256
NM_SETTING_WIRELESS_SECURITY_PMF_REQUIRED
ieee80211w=2
key_mgmt=wpa-eap-sha256
Though these works, these does not include whole combinations.
The key_mgmt could be set independent of ieee80211w value.
For example, management frame protection could be used with
wpa-eap.
ieee80211w=2
key_mgmt=wpa-eap
And wpa-eap-sha256 could be used without management frame
protection.
ieee80211w=0
key_mgmt=wpa-eap-sha256
So this patch uses always key_mgmt=wpa-psk wpa-psk-sha256 or
key_mgmt=wpa-eap wpa-eap-sha256. By this setting, when AP
supports both, stronger algorithm will be chosen (ex. when AP
supports both wpa-eap and wpa-eap-sha256, wpa-eap-sha256 will be
chosen).
Signed-off-by: Masashi Honma <masashi.honma@gmail.com>
We already define _GNU_SOURCE in "config.h", depending
on configure checks.
Also, we always should first include "config.h" (which means
to first include "nm-default.h").
Also, we don't need the entire <link.h>, <dlfcn.h> suffices.
"nm-utils/nm-jansson.h" and thus <jansson.h> must be included only
after "nm-json.h". Enforce that by never including them directly,
except from "nm-json.h" itself.
Otherwise, the only way to disconnect the NMVpnServicePlugin
instance is by completely unrefing it. However, often it is
not so easy to ensure that nobody else is still keeping the
instance alive, after the point where we no longer want to
handle D-Bus requests. nm_vpn_service_plugin_shutdown() to the
rescue.
