Originally, we used "nm-utils/siphash24.c", which was copied
from systemd's source tree. It was both used by our own NetworkManager
code, and by our internal systemd fork.
Then, we added "shared/c-siphash" as a dependency for n-acd.
Now, drop systemd's implementation and use c-siphash also
for our internal purpose. Also, let systemd code use c-siphash,
by patching "src/systemd/src/basic/siphash24.h".
This was only used for some extra assertions. It' is not essential.
If this would be for real usage, we should add a dependancy so that
nm-utils/nm-enum-utils.c requires nm-hash-utils.h. But as it is,
this is not necessary.
This fixes build for meson, which wrongly tries to build nm-enum-utils.c
for libnm-util, but then fails to include nm-hash-utils.c. That should
be fixed independently.
Fixes: 84a6eff106
For _nm_utils_enum_to_str_full(), we always first look whether we have
an alias/nick for the numeric value, and preferably use that. That makes a
lot of sense, as it allows the caller to provide better names (aliases),
which are preferred over the name from the GLib type. It renames the
numeric value.
For the reverse conversion, this makes less sense. A name should have a
unique numeric value. That is, we should not use one name that maps to
a different numeric value based on value_infos and GLib type. IOW, we
should not re-number names.
Add an assertion that we don't provide such a value_infos parameter,
that conflicts with names from GLib type.
Also, although the case where GLib type and value_infos disagree is now
forbidden by an assert, reorder the statements in _nm_utils_enum_from_str_full()
too. There is no difference in practice, but it mirros what we do in the
to-str case.
At various places we sort our D-Bus paths. For example,
server sorts them before exporting them on D-Bus.
Server knows well, that a lot of these paths are build
by attaching an incrementing number as last component.
It looks nicer to sort by this number, instead of strictly
lexical with strcmp().
Note that this handles the cases correctly where paths have
different prefixes, or where they don't end with a number.
Coccinelle:
@@
expression a, b;
@@
-a ? a : b
+a ?: b
Applied with:
spatch --sp-file ternary.cocci --in-place --smpl-spacing --dir .
With some manual adjustments on spots that Cocci didn't catch for
reasons unknown.
Thanks to the marvelous effort of the GNU compiler developer we can now
spare a couple of bits that could be used for more important things,
like this commit message. Standards commitees yet have to catch up.
It is meant to be rather similar in nature to isblank() or
g_ascii_isspace().
Sadly, isblank() is locale dependent while g_ascii_isspace() also considers
vertical whitespace as a space. That's no good for configuration files that
are strucutured into lines, which happens to be a pretty common case.
For one, these functions are not often needed. No need to define them in the
"nm-macros-internal.h" header, which is included everywhere. Move them to
"nm-shared-utils.h", which must be explicitly included.
Also, these functions are usually not called directly, but by passing their
function pointer to a sort function or similar. There is no point in having
defined in the header file.
If the main loop is quit before the timeout expires, we leave the
timeout source running on the main loop context. Since we usually
create the main loop using the default context, the source will fire
on the next main loop we create during the test.
Therefore, destroy the timeout source if it is still active.
Fixes: 766f31507b
Eventually, we should replace our uses of libgsystem's gsystem-local-alloc.h
by glib's g_auto*. As a first tiny step, add a compat implementation for g_autofree,
so that we could at least go ahead and use it instead of gs_free.
https://bugzilla.gnome.org/show_bug.cgi?id=794294
Previously, NM_PTRARRAY_LEN() would not work if the pointer type is
an opaque type, which is common. For example:
NMConnection *const*connections = ...;
Add an alternative to g_clear_pointer(). The differences are:
- nm_clear_pointer() is more type safe as it does not cast neither the
pointer nor the destroy function. Commonly, the types should be compatible
and not requiring a cast. Casting in the macro eliminates some of the
compilers type checking. For example, while
g_clear_pointer (&priv->hash_table, g_ptr_array_unref);
compiles, nm_clear_pointer() would prevent such an invalid use.
- also, clear the destination pointer *before* invoking the destroy
function. Destroy might emit signals (like weak-pointer callbacks
of GArray clear functions). Clear the destination first, so that
we don't leave a dangling pointer there.
- return TRUE/FALSE depending on whether there was a pointer to clear.
I tested that redefining g_clear_pointer()/g_clear_object() with our
more typesafe nm_* variants still compiles and indicates no bugs. So
that is good. It's not really expected that turning on more static checks
would yield a large number of bugs, because generally our code is in a good
shape already. We have few such bugs, because we already turn all all warnings
and extra checks that make sense. That however is not an argument for
not introducing (and using) a more resticted implementation.
It's slightly more correct to first clear the pointer location
before invoking the destroy function. The destroy function might
emit other callbacks, and at a certain point the pointer becomes
dangling. Avoid this danling pointer, by first clearing the
memory, and then destroing the instance.
Add macros that cast away the constness of a pointer, but
ensure that the type of the pointer is as expected.
Unfortunately, there is no way (AFAIK) to remove the constness of
a variable, without explicitly passing @type to the macro.
GCC 8.0's -Wcast-function-type objects casting function pointers to ones
with incompatible prototypes. Sometimes we do that on purpose though.
Notably, the g_source_set_callback()'s func argument can point to functions
of various prototypes. Also, libnm-glib/nm-remote-connection is perhaps
just not worth reworking, that would just be a waste of time.
A cast to void(*)(void) avoids the GCC warning, let's use it.
This makes its prototype compatible with GDestroyNotify so that GCC 8.0
won't warn.
The return value is not used anywhere and the unref() functions typically
don't return any.
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.
