"nm-macros-interal.h" already includes <errno.h> and <string.h>.
No need to include it everywhere else too.

It's not necessary.

Validate them, like we do on a connection addition.

- use cleanup attribute to free memory

- return floating reference from _nm_connection_for_each_secret().
  It's more idiomatic that a function that constructs a variant and
  returns it, returns a floating variant.

_nm_connection_for_each_secret() (formerly for_each_secret()) and
_nm_connection_find_secret() (formerly find_secret()) operate on a
GVariant of secrets. For that, they implement certain assumptions
of how to handle secrets. For example, it must special-case VPN settings,
because there is no generic abstraction to handle regular secret and VPN
secrets the same.

Such special casing should only be done in libnm-core, at one place.

Move the code to libnm-core as internal API.

nm_setting_get_secret_flags() looks whether we have a suitable "-flags"
data value, or whether we have a secret with that name.

In fact, we know this is a valid secret-name. Even if there are no secret-flags
and the secret (currently) does not exists. We shall not care about the
return value.

Note that nm_setting_get_secret_flags() also for non-secrets will set
the flags to "NONE", which is just what we need.

We cannot just blindly assume that the variant is of the right type
to iterate over it.

Fixes: df670681

    ../src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c:126:19: warning: duplicate 'const' declaration specifier [-Wduplicate-decl-specifier]
     static const char const NO_EXPECTED[1];
                       ^~~~~

Fixes: f04bf45e

NetworkManager/NetworkManager!64

We should no longer use nm_connection_for_each_setting_value() and
nm_setting_for_each_value(). It's fundamentally broken as it does
not work with properties that are not backed by a GObject property
and it cannot be fixed because it is public API.

Add an internal function _nm_connection_aggregate() to replace it.

Compare the implementation of the aggregation functionality inside
libnm with the previous two checks for secret-flags that it replaces:

- previous approach broke abstraction and require detailed knowledge of
  secret flags. Meaning, they must special case NMSettingVpn and
  GObject-property based secrets.
  If we implement a new way for implementing secrets (like we will need
  for WireGuard), then this the new way should only affect libnm-core,
  not require changes elsewhere.

- it's very inefficient to itereate over all settings. It involves
  cloning and sorting the list of settings, and retrieve and clone all
  GObject properties. Only to look at secret properties alone.

_nm_connection_aggregate() is supposed to be more flexible then just
the two new aggregate types that perform a "find-any" search. The
@arg argument and boolean return value can suffice to implement
different aggregation types in the future.

Also fixes the check of NMAgentManager for secret flags for VPNs
(NM_CONNECTION_AGGREGATE_ANY_SYSTEM_SECRET_FLAGS). A secret for VPNs
is a property that either has a secret or a secret-flag. The previous
implementation would only look at present secrets and
check their flags. It wouldn't check secret-flags that are
NM_SETTING_SECRET_FLAG_NONE, but have no secret.

Fixes: 6e54057b

Add missing trailing commas that avoids getting noise when another
file/parameter is added and eases reviewing changes[0].

[0] https://gitlab.gnome.org/GNOME/dconf/merge_requests/11#note_291585

While nm_utils_inet*_ntop() accepts a %NULL buffer to fallback
to a static buffer, don't do that.

I find the possibility of using a static buffer here error prone
and something that should be avoided. There is of course the downside,
that in some cases it requires an additional line of code to allocate
the buffer on the stack as auto-variable.

The 'number' property in GSM settings is a legacy thing that comes
from when ModemManager used user-provided numbers, if any, to connect
3GPP modems.

Since ModemManager 1.0, this property is completely unused for 3GPP
modems, and so it doesn't make sense to use it in the NetworkManager
settings. Ofono does not use it either.

For AT+PPP-based 3GPP modems, the 'number' to call to establish the
data connection is decided by ModemManager itself, e.g. for standard
GSM/UMTS/LTE modems it will connect a given predefined PDP context,
and for other modems like Iridium it will have the number to call
hardcoded in the plugin itself.

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

Fixes: d48f389c

When updating a connection passing agent-owned secret, they are lost
from @reread_connection after the settings-plugin persists the
connection. Therefore we need to cache and reapply them separately to
the connection so that they can be saved to secret agents later.

#82

The writer should write all properties of the sriov setting when the
setting exists without additional logic. Likewise, the reader should
instantiate a sriov setting when any sriov key is present and blindly
set properties from keys.

The old code did not always preserve the presence of a sriov setting
after a write/read cycle.

Fixes: c02d1c48

Report an error when the user tries to add an unknown attribute
instead of silently accepting (and ignoring) it.

Note that this commit also changes the behavior of public API
nm_utils_sriov_vf_from_str() to return an error when an unknown
attribute is found. I think the previous behavior was buggy as wrong
attributes were simply ignored without any way for the user to know.

Fixes: a9b4532f

This code will be used later.

We want to remember which keyfiles are currently loaded (or hidden).

With the addition or multiple keyfile directories (soon), there are
two cases where this matters:

 - if there are multiple keyfiles which reference the same UUID,
   we can only load one of them. That is already a problem today
   with only one keyfile directory, where multiple files can reference
   the same UUID.
   The implementation will pick the file based on priorities (like
   the file modification date). However, the user may call explicitly
   call `nmcli connection load`. In that case, we cannot reload
   all files to find out whether the to be loaded file is hidden
   according to the defined priorities. We cannot do that, because we
   must not make decisions based on files on disk, which we are not told
   to reload. So, during a `nmcli connection load` we must look at
   unrelated files, to determine how to load the file.
   Instead, we do allow the user to load any file, even if it would be
   shadowed by other files. When we do that, we may want to persist which
   file is currently loaded, so that a service restart and a `nmcli connection
   reload` does not undo the load again. This can be later later be solved by
   writing a symlink

       "/var/run/NetworkManager/system-connections/.loaded-$UUID.nmkeyfile"

   which targets the currently active file.

 - if a profile was loaded from read-only persistant storage, the user
   may still delete the profile. We also need to remember the deletion
   of the file. That will be achieved by symlinking "/dev/null" as
   "/etc/NetworkManager/system-connections/.loaded-$UUID.nmkeyfile".

Add helper functions to read and write these symlinks.

In particular, have a full path (with slashes), and a filename
with trailing slash (a directory).

Correct the spelling across the *entire* tree, including translations,
comments, etc. It's easier that way.

Even the places where it's not exposed to the user, such as tests, so
that we learn how is it spelled correctly.

In commit f0938948 a typo creeped in and
"block->name" got replaced by "block_name". Variable block_name is used
for a different purpose and not initialized at this point.
As a result g_str_has_prefix crashes with a segfault.

Spotted by Bernhard Übelacker <bernhardu@mailbox.org>

Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=911621
Fixes: f0938948

!31

For upstream, we changed behavior here. However, I think certain
downstream don't want to do that, and revert patch "d37ad15f keyfile:
also add ".nmconnection" extension when writing keyfiles in /etc".

For that to make easier, keep the upstream sources closer to what
was. Revert.

This reverts commit e93d8cdb.

For non-test code, we always passed TRUE.

For test code, setting with_extension is fine too, so drop the
argument.

NM_CONFIG_KEYFILE_PATH_IN_MEMORY is now called NMS_KEYFILE_PATH_NAME_RUN.
This name seems odd in the current context, it will be more suitable
when we also have NMS_KEYFILE_PATH_NAME_LIB (for /usr/lib).

These utilities are concerned with valid file names (as NetworkManager
daemon requires it). This is relevant for everybody who wants to write
keyfile files directly. Hence, move it to libnm-core. Still as internal
API.

For one, re-use the helper function instead of re-implementing
the check at multiple places.

Also, with this duplicate path separators are accepted.

During gitlab-ci, some tests may take a long time. Increase
the default timeout.

This is a change in behavior regarding the filename that we choose when
writing files to "/etc/NetworkManager/system-connections/".

For profiles in "/etc/NetworkManager/system-connections", we did not enforce
that the keyfiles have a special suffix, nor did we generate the
filenames in such a manner. In hindsight, I think that was a mistake.

Recently we added "/run/NetworkManager/system-connections" as additional
keyfile directory. Enforce a suffix and write keyfiles with such a name.

In principle, we could also start writing keyfiles in /etc with the
same suffix. But let's not do that, because we anyway cannot enforce
it.

An ugly part is, that during `nmcli connection load` we need to
determine whether the to-be-loaded connection is under /etc or /run.
Preferably, we would allow any kind of symlinking as what matters
is the file object (inode) and not the path. Anyway, we don't do
that but compare plain paths. That means, paths which are not
in an expected form, will be rejected. In particular, the paths
starting with "/run/..." and "/var/run/..." will be treated differently,
and one of them will be rejected.

Note that ifcfg-rh plugin strictly enforces that the path
starts with IFCFG_DIR as well. So, while this is a breaking
change for keyfile, I think it's reasonable.

The extra newlines look bad when logging to the console.

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

Fixes: e886e536