Having synchronous API is wrong, or at least questionable.
Granted, libnm isn't currently very good about the exact order
of things to happen. However synchronous API by design delays events
while waiting for the response and hence messes up the ordering.
Maybe synchronous API should not be added to libnm.
Or at least, if we have synchronous API, we certainly need an asynchrnous
variant as well (which is still missing).
As synchronous API is not preferred, it should also be named
nm_some_thing_sync(), accompanied by nm_some_thing() and
nm_some_thing_finish(). The name for the synchronous method should be the
odd one and we shouldn't have an nm_some_thing_async(). Yes, libnm is not
consistend about that.
I am going to drop this API for the moment.
- fix leaking hw_address in finalize().
- reorder code.
- avoid double tabs in GObject property definitions.
- hide struct definitions from header.
- don't use signal slots in class structure.
- use NM_GOBJECT_PROPERTIES_DEFINE_BASE().
- add missing NM_AVAILABLE_IN_1_16 annotations.
Don't reallocate peers-array nor set it to %NULL. Instead,
just emit the signal for the peers and take them out one-by-one.
I am slightly surprised, that the peers array does not need to hold
a reference on the NMP2PPeer instances. But that seems intentional.
I think, the libnm code here should be significantly reworked, but
that is for another time.
Also, delay clearing the pointers until finalize() method. For
the most part, it shouldn't make a difference. Still avoid having
the instance in a badly defined state during dispose() (which
theoretically could be called multiple times).
We don't need to remember (and compare) all the bytes that we received.
We can just compare them right away, and remember how many good bytes
we received.
Since we only compare that the HTTP response starts with the expected
response, we need to handle the empty expected response specially
(because, every response has "" as prefix).
So now if connectivity.response is set to "" (empty) we accept:
- HTTP status code 204. We ignore and accept any extra data that we
might receive.
- HTTP status code 200 and an empty (or no) body.
Discovered by GCC 9:
src/ppp/nm-ppp-manager.c: In function ‘_ppp_manager_start’:
./src/nm-logging.h:59:9: error: ‘%s’ directive argument is null [-Werror=format-overflow=]
Fixes: 35d9169c3c
There are two callers that are concerned with disconnecting/releasing a
setting:
- _setting_release_hfr() (formerly _setting_release())
- _nm_connection_add_setting() for the @s_old setting
Compared to one caller that connects/adds a setting (_nm_connection_add_setting()).
Refactor the two callers to use the same helper function
(_setting_release()) so that the implementation of how to release a
setting is at one place.
This patch was originally done when adding another signal to NMSetting.
That did not happen (yet), but the refactoring still makes sense.
And merge it with the version that uses no flags.
Previously, clear_secrets(_with_flags()) was only implemented
by NMSettingVpn. All other settings would only consider GObject-based
properties.
As we will add secrets that have no GObject property, call the virtual
function always, so that the setting can hook into this (for WireGuard
peers).
The secret name should be the one that we can pass to nm_setting_get_secret_flags().
It's wrong to call the function repeatedly with secret-name "secrets".
Probably nobody cared anyway about the name. nm_connection_clear_secrets_with_func()
is used to clear secrets based on the flags, not the secret-name.
Fixes: 2b2404bbef
Add a hook so that we can overwrite the property info.
Yes, this is an API/ABI change for NMSettingClass, which is in a
header file. But this is not API that we want to support. Users must
not use this. Alternatively, I could hook the callback into
NMSettInfoSetting, but either works.
The caller may not wish to replace existing peers, but only update/add
the peers explicitly passed to nm_platform_link_wireguard_change().
I think that is in particular interesting, because for the most part
NetworkManager will configure the same set of peers over and over again
(whenever we resolve the DNS name of an IP endpoint of the WireGuard
peer).
At that point, it seems disruptive to drop all peers and re-add them
again. Setting @replace_peers to %FALSE allows to only update/add.
We still don't use getnameinfo(). This is used for logging,
where we want to see a string representation that is as close
as possible to the actual bytes (to spot differences). It should
not be obfuscated by a libc function out of our control.
Also fix the notation for the IPv6 scope ID to use the common '%'
character.
