A NMRefString tracks the length seprately, it thus may not be a NUL terminated
string (although, there is always a NUL character at the end of the buffer).
As such, the previous implementation did not work correctly in when comparing
for example NMRefString("a\0b") with "a". There was even a comment hinting
to that fact. Instead of making obscure comments, fix the implementation to
behave always correctly.
This helper is useful to get a dummy GSource instance that can be
refed, unrefed and destroyed. It can act as a replacement for
a timeout source with infinite timeout.
However, don't also use the NM_DEPRECATED_IN_1_32 macro, because that
causes annoying compiler warnings.
There is no replacement for the function in libnm, nor is it planned
to add one. So users may still call it, but they are now warned by
documentation that it may not be a good idea.
In the past, the UUID was only loosely validate and would accept
forms that are not valid. This was fixed by commit 207cf3d5d4 ('libnm:
normalize "connection.uuid"'). Now the UUID is always strictly valid
and lower case.
Thus, don't use the fuzzy nm_utils_is_uuid() from libnm but the exact
check nm_uuid_is_valid_full().
Note that this is only used for assertions in the header file. We thus
don't want to drag in "libnm-glib-aux/nm-uuid.h". Instead, we forward
declare the function.
lgtm.com warns about declarations are block scope, so fix that too by
moving the declaration at file scope.
lgtm.com warns about function declarations inside blocks.
*sigh*. I think it's well understood what this code means, and it is not
done by accident. Still, let's make the tool happy in this case.
lgtm.com warns about these uses. They are correct though. Maybe the code should
not use alloca() simply to suppress the warning. Instead, add a comment pointing
out that this is in fact correct.
For one, "src_n_map" must always be greater than zero at this point.
lgtm.com warns about that, and the point of this patch is to avoid
that warning.
Still, the check really isn't needed, also because nm_memdup() explicitly
handles buffers sizes of zero.
First of all, the "os.setuid()" call is never reached. lgtm.com warns about
that, and warnings are annoying because they require investigation.
This code is only in the example, so that the user would understand that
they should edit the source and set the desired UID. But you can only
call setuid() if you have CAP_SETUID, so commonly this anyway doesn't
work -- unless you are root already, and then you actually don't need
it either.
Let's drop this code from the example.
Maybe this example really should be dropped. Does the "dbus" python
module even still work? Doesn't for me...
...
File "/usr/local/lib/python3.9/site-packages/jinja2/environment.py", line 1361, in generate
yield self.environment.handle_exception()
File "/usr/local/lib/python3.9/site-packages/jinja2/environment.py", line 925, in handle_exception
raise rewrite_traceback_stack(source=source)
File ".gitlab-ci/ci.template", line 178, in top-level template code
{% if not version in distro.always and (distro.name != pages_build.name or version != pages_build.version) %}
jinja2.exceptions.UndefinedError: 'dict object' has no attribute 'always'
Ups, we actually still require libuuid. Actually, we only need to
to build the example script `examples/C/glib/add-connection-gdbus.c`.
The proper solution would be to make this an optional dependency.
So far this was not yet done. Also, libuuid is really an ubiquitous
dependency on Linux, so it's not really a problem to have this build
dependency, even if it's just to build the examples.
This reverts commit c0a3947ff9.
lgtm.com has some interesting scripts that try to automatically
build an autotools project and detect dependencies.
It currently doesn't work, because gobject-introspection is detected,
but pgobject is not installed. That causes a configure failure
configure: error: "--enable-introspection aims to build the settings documentation. This requires GObject introspection for python (pygobject)
I guess we could make the configure script smarter, to ignore
introspection by default if pygobject is not workable.
On the other hand, let's simply help the project to build NetworkManager by
adding a .lgtm.yml configuration.
input() in Python2 evaluated the string and was thus unsafe. On
Python2, the right choice is raw_input. In Python3, input does
what raw_input did.
Work around this.
The main "problem" is that lgtm.com flags this as error. The fix
in the example is not important, but getting a low number of warnings
is.
Since the [main].iwd-config-path functionality, where NM watches for
NMSettingsConnection changes and update IWD network config files with
new settings, has proven to work without issues so far, enable it by
default. Instead of hardcoding /var/lib/iwd as the value, and since the
value can't be probed at NM compile time, query it from IWD's recently-
added D-Bus interface for settings when [main].iwd-config-path is either
missing or set to the new value "auto".
Coverity has issues with functions that handle ownership like
g_strstrip(). Thus the scan is full of false positives like:
Error: RESOURCE_LEAK (CWE-772): [#def45] [important]
NetworkManager-1.31.5/src/core/devices/wwan/nm-service-providers.c:134: alloc_fn: Storage is returned from allocation function "g_strdup".
NetworkManager-1.31.5/src/core/devices/wwan/nm-service-providers.c:134: noescape: Resource "g_strdup(attribute_values[i])" is not freed or pointed-to in "g_strchug".
NetworkManager-1.31.5/src/core/devices/wwan/nm-service-providers.c:134: leaked_storage: Failing to save or free storage allocated by "g_strdup(attribute_values[i])" leaks it.
# 132| if (strcmp(attribute_names[i], "value") == 0) {
# 133| parse_context->state = PARSER_METHOD_GSM_APN;
# 134|-> parse_context->apn = g_strstrip(g_strdup(attribute_values[i]));
# 135| break;
# 136| }
Add a workaround for that.
There are other functions that have the same problem, but the usage
g_strstrip(g_strdup(...)) is common to warrant a special workaround.
It's a false positive. Still avoid it.
Error: FORWARD_NULL (CWE-476): [#def479]
NetworkManager-1.31.5/src/libnm-glib-aux/nm-shared-utils.c:5558: var_compare_op: Comparing "strbuf->_priv_str" to null implies that "strbuf->_priv_str" might be null.
NetworkManager-1.31.5/src/libnm-glib-aux/nm-shared-utils.c:5575: var_deref_model: Passing "strbuf" to "nm_str_buf_maybe_expand", which dereferences null "strbuf->_priv_str".
# 5573| l2 = ((gsize) l) + 1u;
# 5574|
# 5575|-> nm_str_buf_maybe_expand(strbuf, l2, FALSE);
# 5576|
# 5577| va_start(args, format);
Error: FORWARD_NULL (CWE-476): [#def480]
NetworkManager-1.31.5/src/libnm-glib-aux/nm-shared-utils.c:5558: var_compare_op: Comparing "strbuf->_priv_str" to null implies that "strbuf->_priv_str" might be null.
NetworkManager-1.31.5/src/libnm-glib-aux/nm-shared-utils.c:5575: no_write_call: Although "nm_str_buf_maybe_expand" does overwrite "strbuf->_priv_str" on some paths, it also contains at least one feasible path which does not overwrite it.
NetworkManager-1.31.5/src/libnm-glib-aux/nm-shared-utils.c:5578: var_deref_op: Dereferencing null pointer "strbuf->_priv_str".
# 5576|
# 5577| va_start(args, format);
# 5578|-> l = g_vsnprintf(&strbuf->_priv_str[strbuf->_priv_len], l2, format, args);
# 5579| va_end(args);
# 5580|
Also Coverity found that something is wrong here:
Error: FORWARD_NULL (CWE-476): [#def361]
NetworkManager-1.31.5/src/libnm-client-impl/nm-vpn-plugin-old.c:441: var_compare_op: Comparing "connection" to null implies that "connection" might be null.
NetworkManager-1.31.5/src/libnm-client-impl/nm-vpn-plugin-old.c:489: var_deref_model: Passing null pointer "connection" to "g_object_unref", which dereferences it.
# 487| }
# 488|
# 489|-> g_object_unref(connection);
# 490| }
# 491|
Fixes: 6793a32a8c ('libnm: port to GDBus')
Previously, we would allocate a buffer of the worst case, that is,
4 times the number of bytes, in case all of them require octal escaping.
Coverity doesn't like _escape_ansic() for another reason:
Error: NULL_RETURNS (CWE-476): [#def298]
NetworkManager-1.31.5/src/core/settings/plugins/ifcfg-rh/shvar.c:161: returned_null: "g_malloc" returns "NULL".
NetworkManager-1.31.5/src/core/settings/plugins/ifcfg-rh/shvar.c:161: alias: Assigning: "q" = "dest = g_malloc(strlen(source) * 4UL + 1UL + 3UL)". Both pointers are now "NULL".
NetworkManager-1.31.5/src/core/settings/plugins/ifcfg-rh/shvar.c:163: dereference: Incrementing a pointer which might be null: "q".
# 161| q = dest = g_malloc(strlen(source) * 4 + 1 + 3);
# 162|
# 163|-> *q++ = '$';
# 164| *q++ = '\'';
# 165|
It doesn't recognize that g_malloc() shouldn't return NULL (because
we never request zero bytes).
I am not sure how to avoid that, but let's rework the code to first count
how many characters we exactly need. It think that should also help with
the coverity warning.
Doing exact allocation requires first to count the number of required
bytes. It still might be worth it, because we might keep the allocated
strings a bit longer around.
It seems very ugly to read one byte at a time. Use a naive buffered
reader, so that we can read multiple bytes at a time, and return them
one by one.
Also, this now keeps state of any error/EOF. Once we reach EOF, we won't
read again. The previous code did that too, but I think this code is
easier to read.
g_warning() and printing to stdout/stderr are not suitable actions
for a library. If there is something important, find a way to report the
condition to the caller. If it's not important, stay quiet.
