to/from string functions are useful. We should be able to reuse them.
Move them to their own location.
Also, it moves independent code out of "clients/common/nm-meta-setting-desc.c"
which is already one of the largest source files we have.
Also, it makes the code unit-testable, which is obviously required
as the code has bugs.
"libnm-core" implements common functionality for "NetworkManager" and
"libnm".
Note that clients like "nmcli" cannot access the internal API provided
by "libnm-core". So, if nmcli wants to do something that is also done by
"libnm-core", , "libnm", or "NetworkManager", the code would have to be
duplicated.
Instead, such code can be in "libnm-libnm-core-{intern|aux}.la".
Note that:
0) "libnm-libnm-core-intern.la" is used by libnm-core itsself.
On the other hand, "libnm-libnm-core-aux.la" is not used by
libnm-core, but provides utilities on top of it.
1) they both extend "libnm-core" with utlities that are not public
API of libnm itself. Maybe part of the code should one day become
public API of libnm. On the other hand, this is code for which
we may not want to commit to a stable interface or which we
don't want to provide as part of the API.
2) "libnm-libnm-core-intern.la" is statically linked by "libnm-core"
and thus directly available to "libnm" and "NetworkManager".
On the other hand, "libnm-libnm-core-aux.la" may be used by "libnm"
and "NetworkManager".
Both libraries may be statically linked by libnm clients (like
nmcli).
3) it must only use glib, libnm-glib-aux.la, and the public API
of libnm-core.
This is important: it must not use "libnm-core/nm-core-internal.h"
nor "libnm-core/nm-utils-private.h" so the static library is usable
by nmcli which couldn't access these.
Note that "shared/nm-meta-setting.c" is an entirely different case,
because it behaves differently depending on whether linking against
"libnm-core" or the client programs. As such, this file must be compiled
twice.
From the files under "shared/nm-utils" we build an internal library
that provides glib-based helper utilities.
Move the files of that basic library to a new subdirectory
"shared/nm-glib-aux" and rename the helper library "libnm-core-base.la"
to "libnm-glib-aux.la".
Reasons:
- the name "utils" is overused in our code-base. Everything's an
"utils". Give this thing a more distinct name.
- there were additional files under "shared/nm-utils", which are not
part of this internal library "libnm-utils-base.la". All the files
that are part of this library should be together in the same
directory, but files that are not, should not be there.
- the new name should better convey what this library is and what is isn't:
it's a set of utilities and helper functions that extend glib with
funcitonality that we commonly need.
There are still some files left under "shared/nm-utils". They have less
a unifying propose to be in their own directory, so I leave them there
for now. But at least they are separate from "shared/nm-glib-aux",
which has a very clear purpose.
There should be little difference here, because the priority list is
(and was) never serialized with special characters like backslashes or
delimiters that require escaping.
Likewise, no working code actually tried to set such characters.
Still, drop the plain VALUE_STRSPLIT_MODE_STRIPPED and use
VALUE_STRSPLIT_MODE_ESCAPED_TOKENS_WITH_SPACES instead. We should have
a small set of modes that we use for splitting strings.
- merge the pointless helper function vlan_priorities_to_string()
into the only caller _get_fcn_vlan_xgress_priority_map().
- minor cleanups, like setting out-is-default if num==0, not
based on whether we have a non-empty string. There is not difference
in practice, because nm_setting_vlan_get_priority() never fails.
Hence they are identical. But nm_setting_vlan_get_priority() has
an API that allows it to fail, so we should declare the default
depending on the number of vlan priorities.
- don't allocate the temporary GString instance if we won't need it.
- only append the delimiter if needed, and not truncate it afterwards.
It might have even worse performance this way, but it feels more
correct to me.
- also cache the result of nm_setting_vlan_get_num_priorities().
NMSettingVlan's implementation is horrible and uses a GSList to
track the list of priorities. This makes it relatively expensive
to call get-num-priorities repeatedly (and pointless).
the only change in behaviour is for VALUE_STRSPLIT_MODE_MULTILIST.
Previously, we would split at " \t,", now we will also split at
the white space characters "\n\r\f".
We had %VALUE_STRSPLIT_MODE_MULTILIST_WITH_ESCAPE, which was used
by "match.interface-names". This uses nm_utils_strsplit_set_full()
with %NM_UTILS_STRSPLIT_SET_FLAGS_ALLOW_ESCAPING and
_nm_utils_unescape_plain().
We want eventually to use nm_utils_escaped_tokens_split() everywhere.
We already have %VALUE_STRSPLIT_MODE_ESCAPED_TOKENS, which splits the
list at ',' (and strips whitespaces at the around the delimiter). That
differs from what %VALUE_STRSPLIT_MODE_MULTILIST_WITH_ESCAPE did, which
also considered whitespace a delimiter.
So, we need a new mode %VALUE_STRSPLIT_MODE_ESCAPED_TOKENS_WITH_SPACES
which replaces the previous mode.
Note that the previous implementation did almost the same thing. In
fact, I cannot imagine examples where they behaved differently, but
my feeling is that there might be some edge cases where this changes
behavior.
The reminder of the function only does (something akin to) g_strstrip().
As we split the strings are spaces to begin with, there is nothing to
strip and we can return right away.
When splitting (and concatenating) list-typed properties,
we really should use nm_utils_escaped_tokens_split()
and nm_utils_escaped_tokens_escape*().
Make that the default, and mark all properties to opt-in to the
legacy behavior.
The modes VALUE_STRSPLIT_MODE_OBJLIST* and VALUE_STRSPLIT_MODE_MULTILIST* are
different. We must use the right mode.
For example, _get_fcn_match_interface_name() concatenates the interface-names
with space. So, the tokenizer of the setter must also use space as delimiter.
VALUE_STRSPLIT_MODE_MULTILIST_WITH_ESCAPE does that correctly,
VALUE_STRSPLIT_MODE_OBJLIST_WITH_ESCAPE does not.
In some cases it is convenient to specify ranges of bridge vlans, as
already supported by iproute2 and natively by kernel. With this commit
it becomes possible to add a range in this way:
nmcli connection modify eth0-slave +bridge-port.vlans "100-200 untagged"
vlan ranges can't be PVIDs because only one PVID vlan can exist.
https://bugzilla.redhat.com/show_bug.cgi?id=1652910
Optimally, all list types properties in nmcli support proper escaping.
For that, we should use nm_utils_escaped_tokens_*() API.
For now, just change that for policy routes. They were just added recently,
so no change in behavior of released API.
nmcli supports list options (optlist and multilist properties).
These commonly are individual items, concatenated by a delimiter.
It should be generally possibly to express every value. That means, we
need some for of escaping mechanism for delimiters.
Currently this is all inconsistent or no escaping is supported. I intend
to fix that (which will be a change in behavior).
For now, just add yet another style of tokenzing/concatenating list
items in nmcli. This is the style to replace all other styles.
Scripts should refer to connections by UUID. That means, whenever a
UUID matches, that is really what the user wants. The specifier "uuid"
must have precedence over "id" (and all others). That means, we must
search all connections.
For example:
$ UUIDS=($(nmcli -g TYPE,UUID connection show | sed -n 's/802-11-wireless://p'))
$ nmcli -f connection.id,connection.uuid connection show "${UUIDS[@]}"
in this case we must preferrably match by UUID, regardless of whether
a "connection.id" exists.
Note that if you have:
$ nmcli connection show | grep fdf7b2d2-2858-3938-9b14-7f1b514a9a00
b fdf7b2d2-2858-3938-9b14-7f1b514a9a00 ethernet --
fdf7b2d2-2858-3938-9b14-7f1b514a9a00 ab9f3891-3420-335e-89da-f14c1b94c540 ethernet --
then certain commands will still select all matching connections:
$ nmcli -f connection.id,connection.uuid --mode multiline connection show fdf7b2d2-2858-3938-9b14-7f1b514a9a00
connection.id: fdf7b2d2-2858-3938-9b14-7f1b514a9a00
connection.uuid: ab9f3891-3420-335e-89da-f14c1b94c540
connection.id: b
connection.uuid: fdf7b2d2-2858-3938-9b14-7f1b514a9a00
This only makes a difference for commands that must pick only one profile:
$ nmcli connection modify fdf7b2d2-2858-3938-9b14-7f1b514a9a00 con-name new-name
Since commit 62b939de4e ('cli: add nmc_complete_strv() which takes a
string array for completion that may contain NULL'), the sentinel is
no longer needed.
Before:
# nmcli c modify eth666 tc.qdiscs root
Error: failed to modify tc.qdiscs: '(null)' is not a valid kind The valid syntax is: '[root | parent <handle>] [handle <handle>] <qdisc>'.
After:
# nmcli c modify eth666 tc.qdiscs root
Error: failed to modify tc.qdiscs: kind is missing. The valid syntax is: '[root | parent <handle>] [handle <handle>] <kind>'.
Leak detection adds unhelpful messages to the stderr of nmcli, making
tests fail. For example:
=================================================================
==17156==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 256 byte(s) in 2 object(s) allocated from:
#0 0x7f08c7e27c88 in realloc (/lib64/libasan.so.5+0xefc88)
#1 0x7f08c7546e7d in g_realloc (/lib64/libglib-2.0.so.0+0x54e7d)
When nmcli needs secrets for a connection it asks them for every known
setting. nmtui is a bit smarter and asks them only for settings that
actually exist in the connection. Make a step further and let clients
ask secrets only for setting that exist *and* have any secret
property. This decreases the number of D-Bus calls when editing or
showing a connection with secrets.
https://bugzilla.redhat.com/show_bug.cgi?id=1506536https://github.com/NetworkManager/NetworkManager/pull/327
Call the correct _finish() function for
nm_client_add_and_activate_connection_async(). add_and_activate_cb()
somewhat confusingly alternates between two different ones depending on
whether info->create is set.
Fixes: 3593237527 ('cli: reuse connections in nmcli dev wifi con')
https://github.com/NetworkManager/NetworkManager/pull/326
I find it questionable, how nmcli likes to coerce the empty input to
NULL to indicate resetting the value. If nmcli would like to set a
default, it should use a different way of signalling that. Anyway, the
assertion was too strict.
$ nmcli connection modify "$PROFILE" +ipv4.addresses ''
