GError codes are only unique per domain, so logging the code without
also indicating the domain is not helpful. And anyway, if the error
messages are not distinctive enough to tell the whole story then we
should fix the error messages.
Based-on-patch-by: Dan Winship <danw@gnome.org>
- 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
This is a straightforward copy of the changes done in libnm. It is done to cope
with test failures due to redundant or misordered signals. See
commit 52ae28f6e5 for a detailed explanation.
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).
Add a file containing the defines like DBUS_INTERFACE_DBUS from
dbus-shared.h, and use it from the gdbus-using files.
Also, convert a bunch of other places that were previously hardcoding
the string values to use the defines instead, and fix the ifcfg-rh
plugin to properly namespace its own D-Bus-related defines.
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.)
NMObjectCache was assuming there would never be more than one object
with the same path, but since NMClient is an NMObject, it was getting
cached too, so if you created two clients and then unreffed one of
them, it's possible the wrong one could get left in the cache, causing
a crash the next time the other one called nm_object_cache_clear().
Fix this by only adding NMObjects to the cache in the codepaths where
we also check to see if the object was already in the cache.
(This also means we can remove the "except" argument to
nm_object_cache_clear(), since the NMClient won't be cached any more.)
Some libnm-glib object properties were only documented in the
GParamSpec strings, not via gtk-doc comments, so they became
undocumented when the paramspec strings went away. Fix that.
(Also fix incorrect gtk-doc syntax with several NMClient properties.)
- Remove list of authors from files that had them; these serve no
purpose except to quickly get out of date (and were only used in
libnm-util and not libnm-glib anyway).
- Just say "Copyright", not "(C) Copyright" or "Copyright (C)"
- Put copyright statement after the license, not before
- Remove "NetworkManager - Network link manager" from the few files
that contained it, and "libnm_glib -- Access network status &
information from glib applications" from the many files that
contained it.
- Remove vim modeline from nm-device-olpc-mesh.[ch], add emacs modeline
to files that were missing it.
Ensure that the @type_funcs and @type_async_funcs
hashes are initialized before running the class
init function.
libnm-glib-scan hits the following assertion:
GLib-CRITICAL **: g_hash_table_insert_internal: assertion 'hash_table != NULL' failed
#0 0x0000003370c504e9 in g_logv (log_domain=0x3370cb2f4e "GLib", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=args@entry=0x7fffd1578f70) at gmessages.c:989
#1 0x0000003370c5063f in g_log (log_domain=<optimized out>, log_level=<optimized out>, format=<optimized out>) at gmessages.c:1025
#2 0x00007f2169f42545 in _nm_object_register_type_func (base_type=base_type@entry=13597296, type_func=type_func@entry=0x7f2169f47ae9 <_nm_device_type_for_path>, type_async_func=type_async_func@entry=
0x7f2169f47880 <_nm_device_type_for_path_async>) at nm-object.c:551
#3 0x00007f2169f48664 in nm_device_get_type () at nm-device.c:62
#4 0x0000000000402577 in get_object_types () at libnm-glib-scan.c:46
#5 0x0000000000404b0b in main (argc=<optimized out>, argv=<optimized out>) at libnm-glib-scan.c:135
Signed-off-by: Thomas Haller <thaller@redhat.com>
Remove all remaining GParamSpec name and blurb strings (and fix
indentation while we're there), and add G_PARAM_STATIC_STRINGS to all
paramspecs that were lacking it.
Most of these warnings are things libnm-glib can't do anything
about, and they are pretty annoying when running nmcli or nmtui,
and libraries usually shouldn't print random warnings anyway.
So downgrade them to debug messages that can be enabled if we
need to see them.
With the addition of D-Bus properties for object-array properties in
NetworkManager core, libnm-glib can use these properties instead of
the pseudo-property stuff. However, we need to maintain API and
provide individual added/removed signals for these properties, and
that requires diff-ing the new and old object arrays. Add the
infrastructure for doing that.
deferred_notify_cb() needs to take a ref on the object around emitting
its deferred signals, since otherwise if a notify:: handler drops the
last reference on an object, a following g_object_notify() call would
crash.
When freeing one of the collections such as GArray, GPtrArray, GSList,
etc. it is common that the items inside the connections must be
freed/unrefed too.
The previous code often iterated over the collection first with
e.g. g_ptr_array_foreach and passing e.g. g_free as GFunc argument.
For one, this has the problem, that g_free has a different signature
GDestroyNotify then the expected GFunc. Moreover, this can be
simplified either by setting a clear function
(g_ptr_array_set_clear_func) or by passing the destroy function to the
free function (g_slist_free_full).
Signed-off-by: Thomas Haller <thaller@redhat.com>
All NMObjects created in response to property changes were getting
leaked, which in particular included all NMAccessPoint objects, which
meant that after disconnecting and reconnecting a wifi interface some
number of times (depending on how many access points were in the
area), gnome-shell would be watching so many D-Bus AccessPoint objects
(most of which didn't exist any more) that it would hit dbus-daemon's
limit on the number of match rules you can register, which meant that
NMActiveConnections created after that point wouldn't be able to
register for PropertiesChanged notifications, which meant that the
network status icon would get out of sync.
Use an environment variable LIBNM_GLIB_DEBUG=properties-changed to
indicate that properties-changed debugging messages should be printed.
Also cleans up the debug output formatting.
A common case where this warning was triggered, was the removal of
IP6Config objects. As this can happen as a regular event, do not warn in
this case.
Signed-off-by: Thomas Haller <thaller@redhat.com>
Though the client shouldn't be calling anything when NM isn't running
(because clients have nm_client_get_manager_running()), make sure
that NMClient never calls a NetworkManager method when NM isn't
on the bus.
Next, ensure NMObject doesn't try to refresh properties when NM isn't
running. Creating an NMClient may trigger a property refresh request,
but if NM isn't running, defer that until NM starts, to ensure that
we don't D-Bus autostart NM.
Third, ensure NMRemoteSettings doesn't attempt to list connections
unless NM is running.
This prevents service activation of NetworkManager in lieu of dbus-glib
learning about DBUS_HEADER_FLAG_NO_AUTO_START.
Use the D-Bus connection helper whenever we need a connection to
NM, which by default tries to use a private connection instead of
the shared bus connection whenever the user is root. Doing this
by default will not change the behavior of libnm-glib, and allows
tools like nmcli and libnm-glib-using clients to work in minimal
environments (those without a bus daemon) by default.
When using a private connection, we need to use dbus_g_proxy_new_for_peer()
because the bus isn't involved. Since many parts of libnm-glib create a
proxy for their corresponding remote object, consolidate the proxy creation
logic.
A later patch will add logic to use a private connection versus a bus-based
one.
Because object-valued properties (like ip4-config) get reloaded
asynchronously when they change, we will still have out-of-date values
for them cached at the point when we get the StateChanged signal from
the daemon. Work around this by manually reloading all properties
before emitting the client-side signal.
Also, fix a dumb bug in NMObject...
If a class implements init_async, it should implement init_finish too,
rather than assuming the default implementation will do the right
thing (which it briefly didn't in glib 2.33).
The signal is private for libnm-glib and should not be used externally.
It is emitted when there's an error while creating an object.
In addition, this commit makes use of the signal in NMClient to ensure
that the callbacks are always called for nm_client_activate_connection()
and nm_client_add_and_activate_connection().
In some situations, objects might get used after being disposed, so
clear out their various priv fields so we don't try to access unreffed
objects, freed strings, etc.
https://bugzilla.gnome.org/show_bug.cgi?id=674473
No idea *why* they're NULL, unless perhaps that dbus-glib can't demarshal
the variants for some reason, but until we know why at least log the
problem so we know what properties the issue might affect.
First: object creation requests get triggered each time a property
that refers to the object is read. That can happen from a couple
of places around the same time (like initialization) and to be
expected. But when those requests are processed (after we've
determined the type of object to create) a previous request may
have already created the object. If that's the case don't create
a duplicate.
Second: properties can also be updated from a few places which
don't know about each other (from both regular code and the
"pseudo property" signal handlers) so when adding objects to
array properties, make sure the object hasn't already been
added to that array.
Fix handle_object_array_property() to deal with receiving an empty
list correctly (rather than warning and leaving the property with its
previous value still set).
Also, add two more untracked properties that shouldn't be warned about
(NMDevice:device-type and NMActiveConnection:vpn, both of which are
only used at construct time).
The notifications for child objects (like NMClient's device-added
signal or NMDeviceWifi's access-point-added signal) could get emitted
before the child objects were actually constructed, because
object_created() decrements the properties-retrieved tracking
variable, which wasn't always incremented before calling that
function.
Need to initialize libnm-util to get GValue transforms registered
so the property values print out as strings. Then actually print
some debugging information about properties.
Some property updates (mainly those dealing with properties that
hold objects themsevles) weren't being done in a synchronous manner
when synchronicity was requested. Make sure that happens.
Implement GInitable and GAsyncInitable in NMObject, with
implementations that synchronously or asynchonously load all
properties, and change _nm_object_ensure_inited() to run
g_initable_init().
Update the object/object-array property handling to initialize the
objects after creating them (synchronously or asynchronously,
according to the situation), so that they will have all of their
properties preloaded before they are ever visible to the caller.
Move the non-blocking/non-failable parts of various objects'
constructor() methods to constructed(), and move the blocking/failable
parts to init(), and implement init_async() methods with non-blocking
versions of the blocking methods.
Make nm_device_new() and nm_client_new() call
_nm_object_ensure_inited(), to preserve the behaviour formerly
enforced by their construct() methods, that properties are guaranteed
to be initialized before any signals involving them are emitted.
Add generic handling for "properties" that consist of a "Get" method,
an "Added" signal, and a "Removed" signal, reusing some of the code
for handling object-array-valued properties. And load the values of
pseudo properties from _nm_object_reload/ensure_properties as well.
Add an "object_type" field to NMPropertiesInfo, and use that with
DBUS_TYPE_G_OBJECT_PATH and DBUS_TYPE_G_ARRAY_OF_OBJECT_PATH
properties so that we don't need custom marshallers for each one.
When creating an NMDevice or NMActiveConnection, we need to fetch an
extra property first to figure out the exact subclass to use, so add a
bit of infrastructure for that as well. Also, do that preprocessing
asynchronously when processing a property change notification, so that
it doesn't block the main loop.
