A function that accepts a floating variant must consume it.
Fixes: 7691fe5753 ('libnm: add new functions allowing passing options to RequestScan() D-Bus call')
Note that D-Bus is fundamentally asynchronous. Doing blocking calls
on top of D-Bus is odd, especially for libnm's NMClient. That is because
NMClient essentially is a client-side cache of the objects from the D-Bus
interface. This cache should be filled exclusively by (asynchronous) D-Bus
events (PropertiesChanged). So, making a blocking D-Bus call means to wait
for a response and return it, while queuing all messages that are received
in the meantime.
Basically there are three ways how a synchronous API on NMClient could behave:
1) the call just calls g_dbus_connection_call_sync(). This means
that libnm sends a D-Bus request via GDBusConnection, and blockingly
waits for the response. All D-Bus messages that get received in the
meantime are queued in the GMainContext that belongs to NMClient.
That means, none of these D-Bus events are processed until we
iterate the GMainContext after the call returns. The effect is,
that NMClient (and all cached objects in there) are unaffected by
the D-Bus request.
Most of the synchronous API calls in libnm are of this kind.
The problem is that the strict ordering of D-Bus events gets
violated.
For some API this is not an immediate problem. Take for example
nm_device_wifi_request_scan(). The call merely blockingly tells
NetworkManager to start scanning, but since NetworkManager's D-Bus
API does not directly expose any state that tells whether we are
currently scanning, this out of order processing of the D-Bus
request is a small issue.
The problem is more obvious for nm_client_networking_set_enabled().
After calling it, NM_CLIENT_NETWORKING_ENABLED is still unaffected
and unchanged, because the PropertiesChanged signal from D-Bus
is not yet processed.
This means, while you make such a blocking call, NMClient's state
does not change. But usually you perform the synchronous call
to change some state. In this form, the blocking call is not useful,
because NMClient only changes the state after iterating the GMainContext,
and not after the blocking call returns.
2) like 1), but after making the blocking g_dbus_connection_call_sync(),
update the NMClient cache artificially. This is what
nm_manager_check_connectivity() does, to "fix" bgo#784629.
This also has the problem of out-of-order events, but it kinda
solves the problem of not changing the state during the blocking
call. But it does so by hacking the state of the cache. I think
this is really wrong because the state should only be updated from
the ordered stream of D-Bus messages (PropertiesChanged signal and
similar). When libnm decides to modify the state, there may be already
D-Bus messages queued that affect this very state.
3) instead of calling g_dbus_connection_call_sync(), use the
asynchronous g_dbus_connection_call(). If we would use a sepaate
GMainContext for all D-Bus related calls, we could ensure that
while we block for the response, we iterate that internal main context.
This might be nice, because all events are processed in order and
after the blocking call returns, the NMClient state is up to date.
The are problems however: current blocking API does not do this,
so it's a significant change in behavior. Also, it might be
unexpected to the user that during the blocking call the entire
content of NMClient's cache might change and all pointers to the
cache might be invalidated. Also, of course NMClient would invoke
signals for all the changes that happen.
Another problem is that this would be more effort to implement
and it involves a small performance overhead for all D-Bus related
calls (because we have to serialize all events in an internal
GMainContext first and then invoke them on the caller's context).
Also, if the users wants this behavior, they could implement it themself
by running libnm in their own GMainContext. Note that libnm might
have bugs to make that really working, but that should be fixed
instead of adding such synchrnous API behavior.
Read also [1], for why blocking calls are wrong.
[1] https://smcv.pseudorandom.co.uk/2008/11/nonblocking/
So, all possible behaviors for synchronous API have severe behavioural
issues. Mark all this API as deprecated. Also, this serves the purpose of
identifying blocking D-Bus calls in libnm.
Note that "deprecated" here does not really mean that the API is going
to be removed. We don't break API. The user may:
- continue to use this API. It's deprecated, awkward and discouraged,
but if it works, by all means use it.
- use asynchronous API. That's the only sensible way to use D-Bus.
If libnm lacks a certain asynchronous counterpart, it should be
added.
- use GDBusConnection directly. There really isn't anything wrong
with D-Bus or GDBusConnection. This deprecated API is just a wrapper
around g_dbus_connection_call_sync(). You may call it directly
without feeling dirty.
---
The only other remainging API is the synchronous GInitable call for
NMClient. That is an entirely separate beast and not particularly
wrong (from an API point of view).
Note that synchronous API in NMSecretAgentOld, NMVpnPluginOld and
NMVpnServicePlugin as not deprecated here. These types are not part
of the D-Bus cache and while they have similar issues, it's less severe
because they have less state.
NM didn't support wpa-none for years because kernel drivers used to be
broken. Note that it wasn't even possible to *add* a connection with
wpa-none because it was rejected in nm_settings_add_connection_dbus().
Given that wpa-none is also deprecated in wpa_supplicant and is
considered insecure, drop altogether any reference to it.
We no longer add these. If you use Emacs, configure it yourself.
Also, due to our "smart-tab" usage the editor anyway does a subpar
job handling our tabs. However, on the upside every user can choose
whatever tab-width he/she prefers. If "smart-tabs" are used properly
(like we do), every tab-width will work.
No manual changes, just ran commands:
F=($(git grep -l -e '-\*-'))
sed '1 { /\/\* *-\*- *[mM]ode.*\*\/$/d }' -i "${F[@]}"
sed '1,4 { /^\(#\|--\|dnl\) *-\*- [mM]ode/d }' -i "${F[@]}"
Check remaining lines with:
git grep -e '-\*-'
The ultimate purpose of this is to cleanup our files and eventually use
SPDX license identifiers. For that, first get rid of the boilerplate lines.
If an operation is cancelled through the GCancellable, then the idiom is
that the operation is always cancelled, even if it has finished
successfully. To ensure this is the case, add calls to
g_simple_async_result_set_check_cancellable everywhere.
Without this, e.g. gnome-control-center will crash when switching away
from the power panel quickly, as the NMClient creation finishes
asynchronously and g-c-c assume that G_IO_ERROR_CANCELLED is returned to
ensure it doesn't access the now invalid user_data parameter.
https://bugzilla.gnome.org/show_bug.cgi?id=794088
This makes it easier to install the files with proper names.
Also, it makes the makefile rules slightly simpler.
Lastly, the documentation is now generated into docs/api, which makes it
possible to get rid of the awkward relative file names in docbook.
This speeds up the initial object tree load significantly. Also, it
reduces the object management complexity by shifting the duties to
GDBusObjectManager.
The lifetime of all NMObjects is now managed by the NMClient via the
object manager. The NMClient creates the NMObjects for GDBus objects,
triggers the initialization and serves as an object registry (replaces
the nm-cache).
The ObjectManager uses the o.fd.DBus.ObjectManager API to learn of the
object creation, removal and property changes. It takes care of the
property changes so that we don't have to and lets us always see a
consistent object state. Thus at the time we learn of a new object we
already know its properties.
The NMObject unfortunately can't be made synchronously initializable as
the NMRemoteConnection's settings are not managed with standard
o.fd.DBus Properties and ObjectManager APIs and thus are not known to
the ObjectManager. Thus most of the asynchronous object property
changing code in nm-object.c is preserved. The objects notify the
properties that reference them of their initialization in from their
init_finish() methods, thus the asynchronously created objects are not
allowed to fail creation (or the dependees would wait forever). Not a
problem -- if a connection can't get its Settings, it's either invisible
or being removed (presumably we'd learn of the removal from the object
manager soon).
The NMObjects can't be created by the object manager itself, since we
can't determine the resulting object type in proxy_type() yet (we can't
tell from the name and can't access the interface list). Therefore the
GDBusObject is coupled with a NMObject later on.
Lastly, now that all the objects are managed by the object manager, the
NMRemoteSettings and NMManager go away when the daemon is stopped. The
complexity of dealing with calls to NMClient that would require any of
the resources that these objects manage (connection or device lists,
etc.) had to be moved to NMClient. The bright side is that his allows
for removal all of the daemon presence tracking from NMObject.
On D-Bus level, string (s) or object paths (o) cannot be NULL.
Thus, whenver server exposes such an object, it gets automatically
coerced to "" or "/", respectively.
On client side, libnm should coerce certain properties back, for which
"" is just not a sensible value.
For example, an empty NM_DEVICE_ETHERNET_HW_ADDRESS should be instead
exposed as NULL.
Technically, this is an API change. However, all users were well advised
to expect both NULL and "" as possible return values and handle them
accordingly.
- All internal source files (except "examples", which are not internal)
should include "config.h" first. As also all internal source
files should include "nm-default.h", let "config.h" be included
by "nm-default.h" and include "nm-default.h" as first in every
source file.
We already wanted to include "nm-default.h" before other headers
because it might contains some fixes (like "nm-glib.h" compatibility)
that is required first.
- After including "nm-default.h", we optinally allow for including the
corresponding header file for the source file at hand. The idea
is to ensure that each header file is self contained.
- Don't include "config.h" or "nm-default.h" in any header file
(except "nm-sd-adapt.h"). Public headers anyway must not include
these headers, and internal headers are never included after
"nm-default.h", as of the first previous point.
- Include all internal headers with quotes instead of angle brackets.
In practice it doesn't matter, because in our public headers we must
include other headers with angle brackets. As we use our public
headers also to compile our interal source files, effectively the
result must be the same. Still do it for consistency.
- Except for <config.h> itself. Include it with angle brackets as suggested by
https://www.gnu.org/software/autoconf/manual/autoconf.html#Configuration-Headers
- "gsystem-local-alloc.h" and <gio/gio.h> are already included via
"nm-default.h". No need to include them separately.
- include "nm-macros-internal.h" via "nm-default.h" and drop all
explict includes.
- in the modified files, ensure that we always include "config.h"
and "nm-default.h" first. As second, include the header file
for the current source file (if applicable). Then follow external
includes and finally internal nm includes.
- include nm headers inside source code files with quotes
- internal header files don't need to include default headers.
They can savely assume that "nm-default.h" is already included
and with it glib, nm-glib.h, nm-macros-internal.h, etc.
API should be added with "Since:" of the next release on the same branch.
That means, new API on 1.1 branch (development), should be "Since: 1.2"
and new API on 1.0 branch (stable) will be "Since: 1.0.x". Similarly, new
API on master is NM_AVAILABLE_IN_1_2 and will be added with the linker
version libnl_1_2 -- never the versions of minor releases.
It is also strongly advised that for the 1.0 branch, we only add API
that was previously formerly added on master. IOW, that we only do true
backports of API that already exists on master.
API that gets backported, must also be added to master via NM_BACKPORT_SYMBOL().
That gives ABI compatibility and an application that was build against 1.0.x
will work with 1.y.z version (y > 0) without need for recompiling -- provided
that 1.y.z also contains that API.
There is one important caveat: if a major branch (e.g. current master) has a
linker section of backported APIs (e.g. libnm_1_0_6), we must do the minor release
(1.0.6) before the next major release (1.2). The reason is that after the major
release, the linker section (libnm_1_0_6) must not be extended and thus
the minor release (1.0.6) must be already released at that point.
In general, users should avoid using backported API because it limits
the ability to upgrade to arbitrary later versions. But together with the
previous point (that we only backport API to minor releases), a user that
uses backported API can be sure that a 1.y.z version is ABI compatible with
1.0.x, if the 1.y.z release date was after the release date of 1.0.x.
This reverts commit 02a136682c.
Backport to 1.0.6 the following symbols:
- nm_device_wifi_request_scan_options
- nm_device_wifi_request_scan_options_async
Backported by commit 91c0555afa
The localization headers are now included via "nm-default.h".
Also fixes several places, where we wrongly included <glib/gi18n-lib.h>
instead of <glib/gi18n.h>. For example under "clients/" directory.
Rather than randomly including one or more of <glib.h>,
<glib-object.h>, and <gio/gio.h> everywhere (and forgetting to include
"nm-glib-compat.h" most of the time), rename nm-glib-compat.h to
nm-glib.h, include <gio/gio.h> from there, and then change all .c
files in NM to include "nm-glib.h" rather than including the glib
headers directly.
(Public headers files still have to include the real glib headers,
since nm-glib.h isn't installed...)
Also, remove glib includes from header files that are already
including a base object header file (which must itself already include
the glib headers).
Libraries need to include <gi18n-lib.h>, not <gi18n.h>, so that _()
will get defined to "dgettext (GETTEXT_DOMAIN, string)" rather than
"gettext (string)" (which will use the program's default domain, which
works fine for programs in the NetworkManager tree, but not for
external users). Likewise, we need to call bindtextdomain() so that
gettext can find the translations if the library is installed in a
different prefix from the program using it (and
bind_textdomain_codeset(), so it will know the translations are in
UTF-8 even if the locale isn't).
(The fact that no one noticed this was broken before is because the
libraries didn't really start returning useful translated strings much
until 0.9.10, and none of the out-of-tree clients have been updated to
actually show those strings to users yet.)
config.h should be included from every .c file, and it should be
included before any other include. Fix that.
(As a side effect of how I did this, this also changes us to
consistently use "config.h" rather than <config.h>. To the extent that
it matters [which is not much], quotes are more correct anyway, since
we're talking about a file in our own build tree, not a system
include.)
As with the settings, each device type was defining its own error
type, containing either redundant or non-useful error codes. Drop all
of the subtype-specific errors, and reduce things to just
NM_DEVICE_ERROR_FAILED, NM_DEVICE_ERROR_INCOMPATIBLE_CONNECTION, and
NM_DEVICE_ERROR_INVALID_CONNECTION.
The device-type-specific errors were only returned from their
nm_device_connection_compatible() implementations, so this is also a
good opportunity to simplify those, by moving duplicated functionality
into the base NMDevice implementation, and then allowing the
subclasses to assume that the connection has already been validated in
their own code. Most of the implementations now just check that the
connection has the correct type for the device (which can't be done at
the NMDevice level since some device types (eg, Ethernet) support
multiple connection types.)
Also, make sure that all of the error messages are localized.
In some cases, code may look at the value of an array-valued property
during object initialization, before NMObject has set it to its actual
initial value. So ensure that we initialize all such properties to an
empty array, rather than leaving them NULL.
Also fix another bug in NMClient that could result in
priv->active_connections being NULL during certain signal emissions,
and fix nm_client_get_active_connections() to not return NULL when NM
was not running.
Make enum- and flags-valued properties use GParamSpecEnum and
GParamSpecFlags, for better introspectability/bindability.
This requires no changes outside libnm-core/libnm since the expected
data size is still the same with g_object_get()/g_object_set(), and
GLib will internally convert between int/uint and enum/flags GValues
when using g_object_get_property()/g_object_set_property().
Add the missing variant in most places in the API where previously
there was either only a synchronous version or only an asynchronous
version.
There is not yet a synchronous nm_client_activate_connection(),
nm_client_add_and_activate_connection(), or
nm_remote_settings_add_connection(), because the existing async code
depends on waiting for other asynchronous events, so making them run
synchronously is slightly more complicated. But these APIs can be
added later.
Make synchronous APIs take GCancellables, and make asynchronous APIs
use GAsyncReadyCallbacks and have names ending in "_async", with
"_finish" functions to retrieve the results.
Also, make nm_client_activate_connection_finish(),
nm_client_add_and_activate_finish(), and
nm_remote_settings_add_connection_finish() be (transfer full) rather
than (transfer none), because the refcounting semantics become
slightly confusing in some edge cases otherwise.
Port libnm-core/libnm to GDBus.
The NetworkManager daemon continues to use dbus-glib; the
previously-added connection hash/variant conversion methods are now
moved to NetworkManagerUtils (along with a few other utilities that
are now only needed by the daemon code).
Add _nm_object_class_add_interface(), for declaring that a class
implements a particular interface, and then have NMObject create the
corresponding proxies itself. (The subclass can get a copy with
_nm_object_get_proxy() if it needs it for something).
(In GDBus, creating a proxy is a heavier operation than in dbus-glib,
so we'll need to create the proxies asynchronously. Moving the
creation to NMObject makes that easier since we can do it as part
of the existing init/init_async.)
libnm functions that return GPtrArrays of objects had a rule that if
the array was empty, they would return NULL rather than a 0-length
array. As it turns out, this is just a nuisance to clients, since in
most places the code for the non-empty case would end up doing the
right thing for the empty case as well (and where it doesn't, we can
check "array->len == 0" just as easily as "array == NULL"). So just
return the 0-length array instead.
Use G_TYPE_PTR_ARRAY for GPtrArray-of-NMObject-valued properties,
because it has better introspection/bindings support.
As with the strdict change in libnm-core, we need to manually copy the
array in get_property() implementations, to preserve the standard
semantics that get_property() returns a copy, not the internal array.
(This patch also changes those properties so that they are always
non-NULL until dispose(); previously some of them could be either NULL
or 0-length at different times.)
Make all mac-address properties (including NMSettingBluetooth:bdaddr,
NMSettingOlpcMesh:dhcp-anycast-addr, and NMSettingWireless:bssid) be
strings, using _nm_setting_class_transform_property() to handle
translating to/from binary form when dealing with D-Bus.
Update everything accordingly for the change, and also add a test for
transformed setting properties to test-general.
Add nm_utils_hwaddr_matches(), for comparing hardware addresses for
equality, allowing either binary or ASCII hardware addresses to be
passed, and handling the special rules for InfiniBand hardware
addresses automatically. Update code to use it.
Include <linux/if_ether.h> and <linux/if_infiniband.h> from
nm-utils.h, to get ETH_ALEN and INFINIBAND_ALEN, and remove those
includes (as well as <net/ethernet.h> and <netinet/ether.h>, and
various headers that had been included to get the ARPHRD_* constants)
from other files where they're not needed now.
Lots of old code used struct ether_addr to store hardware addresses,
and ether_aton() to parse them, but more recent code generally uses
guint8 arrays, and the nm_utils_hwaddr_* methods, to be able to share
code between ETH_ALEN and INFINIBAND_ALEN cases. So update the old
code to match the new. (In many places, this ends up getting rid of
casts between struct ether_addr and guint8* anyway.)
(Also, in some places, variables were switched from struct ether_addr
to guint8[] a while back, but some code still used "&" when referring
to them even though that's unnecessary now. Clean that up.)
Remove _nm_object_ensure_inited(), etc; objects that implement
GInitable are now mandatory-to-init().
Remove constructor() implementations that sometimes return NULL; do
all the relevant checking in init() instead.
Make nm_client_new() and nm_remote_settings_new() take a GCancellable
and a GError**.
