The callback must be invoked, as also documented.
Otherwise, the tracked info gets leaked.
Let NMSecretAgentOld (the caller) be a bit resilient against
bugs in the client, and avoid a crash by prematurely remove
the request-info from the pending list. That does not fully
workaround the bug (it leads to a leak), but at least it does
not cause other "severe" issues.
The leak was present earlier as well.
NMSecretAgentOld's get_secrets_cb() gets this right and takes
a floating reference. So this was correct.
However, make this a bit more robust, and don't pass on
floating references. This was, we don't require the callee
to consume the reference.
Most of the times we actually need a NMSecretAgentSimple typed pointer.
This way, need need to cast less.
But even if we would need to cast more, it's better to have pointers
point to the actual type, not merely to avoid shortcomings of C.
No caller cared about the NM_SECRET_AGENT_ERROR_AGENT_CANCELED reason.
In particular, because previously the requests would keep the secret-agent
instance alive, and this never happend.
Also, NM_SECRET_AGENT_ERROR_AGENT_CANCELED precicley exists for
NMSecretAgentOld:cancel_get_secrets() (as documented). During finalize
we are not cancelled -- at least not the same way as
cancel_get_secrets(). Setting NM_SECRET_AGENT_ERROR_AGENT_CANCELED
is wrong.
Anyway, we have a default error for such cases already.
The code was correct. But it's hard to follow when and whether
the child-watch-id was destroyed at the right time.
Instead, always let _auth_dialog_data_free() clear the signal handlers.
Don't let RequestData keep the parent NMSecretAgentSimple instance
alive. Previously, the code in finalize() was never actually reached.
Also, move the final callback from finalize() to dispose(). It feels
wrong to invoke callbacks from finalize().
We must actually cancel the GCancellable. Otherwise, the pending async
operations are not cancelled. _auth_dialog_write_done() doesn't care
about that, but _auth_dialog_read_done() does. It must not touch the
destroyed data, after the operation is cancelled.
Note that previously the @requests hash took the request-id as key and
the RequestData as value. Likewise, the destroy functions of the head
would destroy the key and the value.
However, RequestData also had a field "request_id". But that pointer was
not owned (nor freed) by the RequestData structure. Instead, it was
relied that the hash kept the request-id alive long enough.
That is confusing. Let RequestData own the request-id.
Also, we don't need to track a separate key. Just move the request-id
as first filed in RequestData, and use compare/hash functions that
handle that correctly (nm_pstr_*()).
Code that is internal to a source file should not have a "nm" prefix.
That is what differenciates it from declarations in header files. It
makes it clearer that these names are only defined in the current file.
Also, our implementations of virtual functions shall have the same
name as the function pointer of the VTable (or at least, it shouldn't
have a "nm" prefix).
nmcli connection modify t ipv4.dns-options ndots:2
nmcli connection modify t +ipv4.dns-options ndots:4
should set dns-options to 'ndots:4', so we must remove other
occurences of the same option before adding it, otherwise the setting
refuses to set the same option again.
Appending to the ipvx.dns-options property:
nmcli connection modify con +ipv4.dns-options rotate
currently is buggy because it resets the list to contain only
'rotate'. The setter function should not clear the list.
https://bugzilla.redhat.com/show_bug.cgi?id=1665649
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
Describe how to specify multiple VFs and which attributes are
supported, so that this information is available in the nm-settings
manual page.
Also, clarify that SR-IOV parameters are managed only when the setting
is present.
https://bugzilla.redhat.com/show_bug.cgi?id=1651979
Add support for the teaming arp_ping link watcher 'vlanid' property.
Signed-off-by: Patrick Talbert <ptalbert@redhat.com>
[thaller@redhat.com: minor fixes to original patch]
https://bugzilla.redhat.com/show_bug.cgi?id=1652931
In most cases, we don't want the translated string (only marked
for translation, and then programatically the caller deciedes
whether to translate or not).
The few places that always call gettext() can do it explicitly.
Now, that our functions are all "no_l10n" by default, rename them.
The state handling in add_and_activate_cb() and connect_device_cb() is
redundant to connected_state_cb() and in fact executed only if the
activation is really really really quick (which it never is).
Also, the return_text those implementations provide is different from
what connected_state_cb(), potentially confusing the users and adding
extra work for translators.
Not to mention extra lines of code, reading whose wastes our precious
time on the planet we could spend doing heroin instead.
Try to locate an existing connection before creating a new one when
handling "nmcli device wifi connect". This allows WPA-Enterprise
networks to be activated this way, consistent with the comment that this
command is equivalent to clicking on an SSID in a GUI client.
VALUES_STATIC() was a macro to initialize the values_static pointer with
a (static) strv array.
For one, it lacked a "const" in "(const char *[])", which means
the data is not put in a read only section by the linker. That should
be fixed.
Anyway, we already have a macro for creating such constant strv arrays:
NM_MAKE_STRV().
I think it is good to the concept of "initializing values_static" a
name (VALUES_STATIC()). But it also hides (for better or worse), that
this is a strv array. Let's use NM_MAKE_STRV() instead. By looking at
the code, it's still clear that this initializes the "values_static"
array, but it also makes it clear that this is a plain strv array.
When nm_device_disconnect_async() returns, the device could be still
in DEACTIVATING state, and so we also register to device-state signal
notifications to know when the device state goes to DISCONNECTED.
Sometimes it happens that the device state goes to DISCONNECTED before
nm_device_disconnect_async() returns. In this case the signal handler
exits the main loop and then the callback for disconnect_async() is
executed anyway because it was already dispatched, leading to an
invalid memory access.
To avoid this we should cancel nm_device_disconnect_async() when we
are quitting the main loop.
Reproducer:
nmcli connection add type team ifname t1 con-name t1
nmcli connection up t1
nmcli device disconnect t1 & nmcli device delete t1
Crash example:
==14955==ERROR: AddressSanitizer: SEGV on unknown address 0xffffffff0000000b (pc 0x7f128c8ba3dd bp 0x0000004be080 sp 0x7ffcda7dc6e0 T0)
==14955==The signal is caused by a READ memory access.
0 0x7f128c8ba3dc in g_string_truncate (/lib64/libglib-2.0.so.0+0x713dc)
1 0x7f128c8bb4bb in g_string_printf (/lib64/libglib-2.0.so.0+0x724bb)
2 0x45bdfa in disconnect_device_cb clients/cli/devices.c:2321
3 0x7f128ca3d1a9 in g_simple_async_result_complete /usr/src/debug/glib2-2.58.1-1.fc29.x86_64/gio/gsimpleasyncresult.c:802
4 0x7f128cf85d0e in device_disconnect_cb libnm/nm-device.c:2354
5 0x7f128ca4ff73 in g_task_return_now /usr/src/debug/glib2-2.58.1-1.fc29.x86_64/gio/gtask.c:1148
6 0x7f128ca508d5 in g_task_return /usr/src/debug/glib2-2.58.1-1.fc29.x86_64/gio/gtask.c:1206
7 0x7f128ca8ecfc in reply_cb /usr/src/debug/glib2-2.58.1-1.fc29.x86_64/gio/gdbusproxy.c:2586
8 0x7f128ca4ff73 in g_task_return_now /usr/src/debug/glib2-2.58.1-1.fc29.x86_64/gio/gtask.c:1148
9 0x7f128ca508d5 in g_task_return /usr/src/debug/glib2-2.58.1-1.fc29.x86_64/gio/gtask.c:1206
10 0x7f128ca83440 in g_dbus_connection_call_done /usr/src/debug/glib2-2.58.1-1.fc29.x86_64/gio/gdbusconnection.c:5713
11 0x7f128ca4ff73 in g_task_return_now /usr/src/debug/glib2-2.58.1-1.fc29.x86_64/gio/gtask.c:1148
12 0x7f128ca4ffac in complete_in_idle_cb /usr/src/debug/glib2-2.58.1-1.fc29.x86_64/gio/gtask.c:1162
13 0x7f128c893b7a in g_idle_dispatch gmain.c:5620
14 0x7f128c89726c in g_main_dispatch gmain.c:3182
15 0x7f128c897637 in g_main_context_iterate gmain.c:3920
16 0x7f128c897961 in g_main_loop_run (/lib64/libglib-2.0.so.0+0x4e961)
17 0x473afb in main clients/cli/nmcli.c:1067
18 0x7f128c6a1412 in __libc_start_main (/lib64/libc.so.6+0x24412)
19 0x416c39 in _start (/usr/bin/nmcli+0x416c39)
https://github.com/NetworkManager/NetworkManager/pull/254https://bugzilla.redhat.com/show_bug.cgi?id=1546061
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.
