Run:
./contrib/scripts/nm-code-format.sh -i
./contrib/scripts/nm-code-format.sh -i
Yes, it needs to run twice because the first run doesn't yet produce the
final result.
Signed-off-by: Antonio Cardace <acardace@redhat.com>
Callbacks might reference the main loop when destroying the NMClient
instance. Unref the main loop later.
# G_DEBUG=fatal-warnings valgrind --num-callers=100 nmcli device wifi connect home
^C
Error: nmcli terminated by signal Interrupt (2)
Error: Connection activation failed: (0) No reason given.
==11050== Invalid read of size 4
==11050== at 0x4C90D3D: g_main_loop_quit (in /usr/lib64/libglib-2.0.so.0.6200.6)
==11050== by 0x431435: quit (devices.c:934)
==11050== by 0x43272C: connected_state_cb (devices.c:1919)
==11050== by 0x4BF6741: g_closure_invoke (in /usr/lib64/libgobject-2.0.so.0.6200.6)
==11050== by 0x4C0A603: ??? (in /usr/lib64/libgobject-2.0.so.0.6200.6)
==11050== by 0x4C133AD: g_signal_emit_valist (in /usr/lib64/libgobject-2.0.so.0.6200.6)
==11050== by 0x4C139D2: g_signal_emit (in /usr/lib64/libgobject-2.0.so.0.6200.6)
==11050== by 0x4BFB1C3: ??? (in /usr/lib64/libgobject-2.0.so.0.6200.6)
==11050== by 0x4BFAAEC: ??? (in /usr/lib64/libgobject-2.0.so.0.6200.6)
==11050== by 0x4BFD86A: g_object_thaw_notify (in /usr/lib64/libgobject-2.0.so.0.6200.6)
==11050== by 0x48BA040: _nm_client_notify_event_emit (nm-client.c:937)
==11050== by 0x48CA01F: _dbus_handle_changes_commit (nm-client.c:2850)
==11050== by 0x48CC221: _dbus_handle_changes (nm-client.c:2864)
==11050== by 0x48CC833: _init_release_all (nm-client.c:6969)
==11050== by 0x48D2818: dispose (nm-client.c:7826)
==11050== by 0x4BFBC27: g_object_unref (in /usr/lib64/libgobject-2.0.so.0.6200.6)
==11050== by 0x43FF93: nmc_cleanup (nmcli.c:941)
==11050== by 0x4410AD: main (nmcli.c:1005)
==11050== Address 0x54738fc is 12 bytes inside a block of size 16 free'd
==11050== at 0x4839A0C: free (vg_replace_malloc.c:540)
==11050== by 0x4C9649C: g_free (in /usr/lib64/libglib-2.0.so.0.6200.6)
==11050== by 0x4410A3: main (nmcli.c:1004)
==11050== Block was alloc'd at
==11050== at 0x483AB1A: calloc (vg_replace_malloc.c:762)
==11050== by 0x4C96400: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.6200.6)
==11050== by 0x4C90A45: g_main_loop_new (in /usr/lib64/libglib-2.0.so.0.6200.6)
==11050== by 0x441020: main (nmcli.c:987)
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/501
It's bad style to pass the argv argument around and mutate it.
We shouldn't mutate it, and not assume that it stays around after
the function returns to the caller (meaning, we should clone the
array if we intend to use it later).
Add const specifier.
- move the main func declarations to nmcli.h and give them a common
prefix "nmc_command_func_" prefix.
- remove some of the header files that are now empty. In fact, these
headers did not really declare some well separated module. While we
probably should structure the code in nmcli better with better layering,
it was not and still is not. Having these dummy headers don't mean that
the code is well structured and they serve little purpose.
- move the static NMCommand lists variables into the function scope
where they are used.
We should not use global variables, and we should minimize the state
that we pass around. Instead of requiring the full NmCli struct in
nm_cli_spawn_pager(), pass only the necessary data.
This reduces our use of global variables.
We should try to avoid access to global variables. For libreadline
callbacks we still need a global variable.
Introduce a global variable nm_cli_global_readline, specially for this
use. It makes the places clear where we use it, and discourages
the use at other places, where we better avoid global variables.
The only remaining GValue transform function was from GHashTable (of (str,str) type)
to string. Drop that too, and implement the conversion in _get_fcn_gobject_impl().
Note that there are few GObject properties of type GHashTable and most
of them implement their own logic. This only applies to
"802-3-ethernet.s390-options".
Also, always sort the keys. Otherwise, the output is not stable.
The g_value_register_transform_func() for handling GBytes was not actually used.
All properties of type G_TYPE_BYTES have their explit handler how to convert bytes
to string. That is good, because the implementation there was very bad (it did not
honor pretty/parsable get-type).
Also, merge _get_fcn_gobject_bytes() into _get_fcn_gobject_impl(). We
already have a generic handler that handles properties solely based on
the GObject type: _get_fcn_gobject_impl(). Just let it also handle
bytes. It's better to have fewer handlers, if they don't need special
context.
It's ugly to modify the global behavior of glib to convert between
types. Instead, _get_fcn_gobject_impl() is perfectly capable to implement
converting a strv array to string.
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.
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.
We support all of these:
nmcli -v
nmcli -version
nmcli --version
Change the help output to display the first and last versions for
options, since they are the most common ones for command line tools.
With --color=auto, coloring is enabled based on the .enable/.disable
termcolors files.
Likewise, when we enable coloring, we parse the color palette from the
.schem termcolors files.
The termcolors files are searched by finding the best match depending
on the terminal and application name. Note, that if we find a matching
file like "nmcli@xterm.enable" we still allow loading the palette from
a less specific file like "nmcli.schem" and vice versa. That was already
done before.
Previously, the search was done by calling several layers of functions, and having
in/out arguments "color_option" and "p_palette_buffer". in/out paramters
here seems confusing to me, as they are state that gets modified and carried
along.
Instead, rework the functions to clearly separate between input
and output arguments.
Also, in the auto-case, check_colors() now first determines whether
coloring is enabled, before even starting loading the palette.
This avoids loading the palette until we are sure that we need it.
The NmCli variables is essentially a global variable of *everything*.
The set_color() function and its helpers only need a particular
part of it. Instead, of passing the entire global state to them,
only pass what they need.
It makes it clearer which parts are actually relevant. Turns out,
it only actually touches a resonable small part of the global state.
use nmc_print() for the job.
Also, localize non-terse output.
Also, fix bug with
$ nmcli c s /org/freedesktop/NetworkManager/ActiveConnection/1
if active connection #1 is invisible to the user.
Also, previously, fill_output_active_connection() wrongly tries to
write to a field that doesn't exist:
set_val_strc (arr, 13-idx_start, s_con ? nm_setting_connection_get_slave_type (s_con) : NULL);
