GitLab

- g_ascii_strtoll() accepts leading spaces, but it leaves
  the end pointer at the first space after the digit. That means,
  we accepted "1: 0" but not "1 :0". We should either consistently
  accept spaces around the digits/colon or reject it.

- g_ascii_strtoll() accepts "\v" as a space (just like `man 3 isspace`
  comments that "\v" is a space in C and POSIX locale.
  For some reasons (unknown to me) g_ascii_isspace() does not treat
  "\v" as space. And neither does NM_ASCII_SPACES and
  nm_str_skip_leading_spaces().
  We should be consistent about what we consider spaces and what not.
  It's already odd to accept '\n' as spaces here, but well, lets do
  it for the sake of consistency (so that it matches with our
  understanding of ASCII spaces, albeit not POSIX's).

- don't use bogus error domains in "g_set_error (error, 1, 0, ..."
  That is a bug and we have NM_UTILS_ERROR exactly for error instances
  with unspecified domain and code.

- as before, accept a trailing ":" with omitted minor number.

- reject all unexpected characters. strtoll() accepts '+' / '-'
  and a "0x" prefix of the numbers (and leading POSIX spaces). Be
  strict here and only accepts NM_ASCII_SPACES, ':', and hexdigits.
  In particular, don't accept the "0x" prefix.

This parsing would be significantly simpler to implement, if we could
just strdup() the string, split the string at the colon delimiter and
use _nm_utils_ascii_str_to_int64() which gets leading/trailing spaces
right. But let's save the "overhead" of an additional alloc.

This is the preferred way of associating the connection with a
particualr modem.

!138

!134

Only read the keyfile databases once and cache them for the remainder of
the program.

- this avoids the overhead of opening the file over and over again.

- it also avoids the data changing without us expecting it. The state
  files are internal and we don't support changing it outside of
  NetworkManager. So in the base case we read the same data over
  and over. In the worst case, we read different data but are not
  interested in handling the changes.

- only write the file when the content changes or before exiting
  (normally).

- better log what is happening.

- our state files tend to grow as we don't garbage collect old entries.
  Keeping this all in memory might be problematic. However, the right
  solution for this is that we come up with some form of garbage
  collection so that the state files are reaonsably small to begin with.

It will be used for "/var/lib/NetworkManager/seen-bssids" and
"/var/lib/NetworkManager/timestamps" which currently is implemented
in NMSettingConnection.

We have it, so use it. Also, we use a similar macro for other singletons.

!133

ethtool/mii API is based on the ifname. As an interface can be renamed,
this API is inherently racy. We would prefer to use the ifindex instead.
The ifindex of a device cannot change (altough it can repeat, which opens a
different race *sigh*).

Anyway, we were already trying to minimize the race be resolving the
name from ifindex immediately before the call to ethtool/mii.

Do better than that. Now resolve the name before and after the call. If
the name changed in the meantime, we have an indication that a race
might have happend (but we cannot be sure).

Note that this can not catch every possible kind of rename race. If you are very
unlucky a swapping of names cannot be detected.

For getters this is relatively straight forward. Just retry when we
have an indication to fall victim to a race (up to a few times). Yes, we
still cannot be 100% sure, but this should be very reliable in practice.

For setters (that modify the device) we also retry. We do so under the
assumption that setting the same options multiple times has no bad effect.
Note that for setters the race of swapping interface names is particularly
bad. If we hit a very unlucky race condition, we might set the setting on
the wrong interface and there is nothing we can do about it. The retry only
ensures that eventually we will set it on the right interface.

Note that this involves one more if_indextoname() call for each operation (in
the common case when there is no renaming race). In cases where we make
multiple ioctl calls, we cache and reuse the information though. So, for such
calls the overhead is even smaller.

When we set the MTU on the link we remember its previous source
(ip-config, parent-device or connection profile) and don't change it
again afterwards to avoid interfering with user's manual changes. The
only exceptions when we change it again are (1) if the parent device
MTU changes and (2) if the new MTU has higher priority than the one
previously set.

To allow a live reapply of the MTU property we also need to clear the
saved source, or the checks described above will prevent setting the
new value.

Fixes: 2f891723 ('device: rework mtu priority handling')

https://bugzilla.redhat.com/show_bug.cgi?id=1702657

Don't print blob certificates unless the '--show-secrets' option is
passed to nmcli; plus other related changes.

https://bugzilla.redhat.com/show_bug.cgi?id=1702199
!122

This restores the behavior before commit 99711579.

Fixes: 99711579 ('cli: add property type for 802-1x certificate properties (pt2)').

Allow the completion function to indicate that the word should be
completed as a filename by the shell.

The 'bt-type' property alias accepts values provided by
gen_func_bt_type(); instead the 'bluetooth.type' property can only be
set to [dun, panu, nap] and therefore it doesn't need special
handling.

In this way it become possible to specify a filename that includes one
of the delimiters.

Fixes: fe390556 ('cli: add property type for 802-1x certificate properties (pt3)')

Fix the following assertion failure:

  g_object_ref: assertion 'G_IS_OBJECT (object)' failed.

nm_settings_add_connection() can return a NULL connection.

Fixes: f034f17f ('settings: keep the added connection alive for a bit longer')

!135

  make[3]: Entering directory 'NetworkManager/_build/sub'
    CC       clients/cli/nmcli-common.o
  cc1: error: ./clients/common: No such file or directory [-Werror=missing-include-dirs]
  cc1: all warnings being treated as errors

The only generated header in $builddir/clients/common is settings-docs.h
and only libnmc.la needs it. Include the directory on the command line
only when we know it exists.

Fixes: 2148d094 ('core/wireguard: add support for WireGuard peers')

On Ubuntu 14.04 kernel (4.4.0-146-generic, x86_64) this easily causes
test failures:

    make -j 8 src/platform/tests/test-route-linux \
    && while true; do \
        NMTST_SEED_RANDOM= ./tools/run-nm-test.sh src/platform/tests/test-route-linux -p /route/rule \
        || break; \
    done

outputs:

    ...
    /route/rule/1:
    nmtst: initialize nmtst_get_rand() with NMTST_SEED_RAND=22892021
    OK
    /route/rule/2: >>> failing...
    >>> no fuzzy match between: [routing-rule,0x205ab30,1,+alive,+visible; [6] 0: from all suppress_prefixlen 8 none]
    >>>                    and: [routing-rule,0x205c0c0,1,+alive,+visible; [6] 0: from all suppress_prefixlen -1579099242 none]
    **
    test:ERROR:src/platform/tests/test-route.c:1695:test_rule: code should not be reached

Changing "Token" translation from "identificador" to "testigo" as discussed
in the GNOME Spanish Translation Team's mailing list.

Special thanks to Daniel Mustieles our coordinator.

!130

NetworkManager/NetworkManager!128

!126

Merge the function pointer get_func() into to_dbus_fcn().

Previously, get_func() as handled separately from to_dbus_fnc()
(formerly synth_func()). The notion was that synth-func would syntetize
properties that are D-Bus only. But that distinction does not seem
very helpful to me.

Instaed, we want to convert a property to D-Bus. Period. The
implementation should be handled uniformly. Hence, now that is
all done by property_to_dbus().

Note that property_to_dbus() is also called as default implementation
for compare-property. At least, for properties that are backed by a
GObject property.

The naming was not very clear. How does get_func(), synth_func()
and to_dbus() relate? What does synth_func() do anyway?

Answers:

- get_func() and synth_func() do very similar. They should be merged
  in a next step.
  synth_func() has the notion of "synthetize" a property for
  D-Bus. As such, these properties are a bit unusual in that they
  don't have a backing GObject property in the setting. But it'd
  rather treat such properties like other properties. The step
  in that direction will be to merge the to-dbus functions.

- to_dbus() converts a GValue of the GObject property go GVariant.
  It's a simplified form of get_func()/synth_func() and a better name
  is gprop_to_dbus_fcn().
  The same for gprop_from_dbus_fcn().

For now, just rename.

We have certain artificial properties that not only depend on one
property alone or that depend on a property in another(!) setting.

For that, we have synth_func.

Other than that, synth_func and get_func are really fundamentally
similar and should be merged. That is because the distinction whether a
property value is "synthetized" or just based on a plain property is
minor. It's better to have the general concept of "convert property to
GVariant" in one form only.

Note that compare_property() is by default implemented based
on get_func. Hence, if get_func and synth_func get merged,
compare_property() will also require access to the NMConnection.

Also it makes some sense: some properties are artificial and actually
stored in "another" setting of the connection. But still, the property
descriptor for the property is in this setting. The example is the
"bond.interface-name" which only exists on D-Bus. It's stored as
"connection.interface-name".
I don't really like to say "exists on D-Bus only". It's still a valid
property, despite in NMSetting it's stored somehow differently (or not
at all). So, this is also just a regular property for which we have a
property-info vtable.
Does it make sense to compare such properties? Maybe. But the point is that
compare_property() function needs sometimes access to the entire
connection. So add the argument.

Always properly set NMSettInfoProperty.dbus_type, instead of leaving it
unspecified for GObject property based properties, and detect it each
time anew with variant_type_for_gtype().

Instead, autodetect and remember the dbus-type during _properties_override_add_struct().
For types that need special handling (GBytes, enums and flags) set a to_dbus() function.
This allows us to handle properties uniformly by either calling the to_dbus() function
or g_dbus_gvalue_to_gvariant().

- use cleanup attribute in get_property_for_dbus() and return early.

- use NM_FLAGS_HAS() macro in _nm_setting_to_dbus().

- in nm_setting_get_dbus_property_type() use g_return*() asserts
  instead of crash or hard asserts.

- return early from variant_type_for_gtype().

https://github.com/NetworkManager/NetworkManager/pull/338