Returning TRUE for zero makes no sense. Obviously, zero is not a power
of two.
Also, the function is used to check whether a number has only one bit
(flag) set, so, an alternative name would be "has-one-bit-set", which
also should return FALSE for zero. All callers didn't really care for
the previous meaning "has-at-most-one-bit-set".
This also avoids the issue of checking (x >= 0), which causes
-Wtype-limits warnings for unsigned types. Which was avoided
by doing (x == 0 || x > 0), which caused -Wlogical-op warning,
which then was avoided (x == 0 || (x > 0 && 1)). Just don't.
We recently added -Wlogical-op in our build process
(commit #41e7fca59762dc928c9d67b555b1409c3477b2b0).
Seems that old versions of gcc (4.8.x) will hit that warning with our
implementation of our "nm_utils_is_power_of_two" and
"test_nm_utils_is_power_of_two_do" macros.
Fool it just adding an always TRUE check.
Use C-style backslash escaping to sanitize non-UTF-8 strings.
The functions are compatible with glib's g_strcompress() and
g_strescape().
The difference is only that g_strescape() escapes all non-printable,
non ASCII character as well, while nm_utils_str_utf8safe_escape()
-- depending on the flags -- preserves valid UTF-8 sequence except
backslash.
The flags allow to optionally escape ASCII control characters and
all non-ASCII (valid UTF-8) characters. But the option to preserve
valid UTF-8 (non-ASCII) characters verbatim, is what distinguishes
from g_strescape().
UDev never creates such invalid escape sequences. Anyway,
we cannot accept a NUL character at this point. Just take
the ill escape verbatim -- it should never happen anyway.
Include the circular, doubly-linked list implementation from
c-util/c-list [1], commit 864051de6e7e1c93c782064fbe3a86b4c17ac466.
[1] https://github.com/c-util/c-list
CC shared/nm-utils/libnm_core_libnm_core_la-nm-udev-utils.lo
In file included from ./shared/nm-utils/nm-glib.h:27:0,
from ./shared/nm-utils/nm-macros-internal.h:29,
from ./shared/nm-default.h:178,
from shared/nm-utils/nm-udev-utils.c:21:
shared/nm-utils/nm-udev-utils.c: In function ‘nm_udev_client_enumerate_new’:
./shared/nm-utils/gsystem-local-alloc.h:53:50: error: ‘to_free’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
GS_DEFINE_CLEANUP_FUNCTION(void*, gs_local_free, g_free)
^~~~~~
shared/nm-utils/nm-udev-utils.c:147:18: note: ‘to_free’ was declared here
gs_free char *to_free;
^~~~~~~
In file included from ./shared/nm-utils/nm-glib.h:27:0,
from ./shared/nm-utils/nm-macros-internal.h:29,
from ./shared/nm-default.h:178,
from shared/nm-utils/nm-udev-utils.c:21:
shared/nm-utils/nm-udev-utils.c: In function ‘nm_udev_client_new’:
./shared/nm-utils/gsystem-local-alloc.h:53:50: error: ‘to_free’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
GS_DEFINE_CLEANUP_FUNCTION(void*, gs_local_free, g_free)
^~~~~~
shared/nm-utils/nm-udev-utils.c:243:20: note: ‘to_free’ was declared here
gs_free char *to_free;
^~~~~~~
Fixes: e32839838e
I used to use g_strv_length ((char **) p) instead, but that feels
ugly because it g_strv_length() is not designed to operate on
arbitrary pointer arrays.
Commit a8730c51c8 moved the enum
utils from libnm-core to shared/nm-utils.
However, three of those functions are part of public API in libnm.
So, when statically linking against "shared/nm-utils/nm-enum-utils.c"
and dynamically linking against libnm.so, those symbols are present
twice and cause a linker failure.
Fix that by moving the public API back to libnm-core.
Fixes: a8730c51c8
libnm contains the public function nm_utils_enum_from_str() et al.
The function is not flexible enough for nmcli's usecase. So, I would
need another public function like nm_utils_enum_from_str_full() that
has an extended API.
That was already required previously for ifcfg-rh writer, but in that
case I could just add it as internal API as libnm-core is linked statically
with NetworkManager.
I don't want to commit to a public API for an utility function. So move
the code instead to the shared directory, so that nmcli may link
statically against it and use the internal API.
These functions are only used by nm-meta-setting-desc.c. Make them internal.
Unfortunately, they are part of "common.h" which cannot be used without
the rest of nmcli. Still todo.
GUdevClient always creates a monitor instance, even if there are no subsystems
or handlers defined. Hence the first iteration of NMUdevClient did that as
well.
I think that can be avoided however. We only need a monitor when there is
a event handler subscribed. Contrary to GUdevClient, we know that from the
very beginning.
_NM_GET_PRIVATE() macro is used to implement a standard private-getter, but it
requires that "self" is a pointer of either "const type *" or "type *". That
is great in most cases, but sometimes we have predominatly self pointers of
different type, so it would require a lot of casts.
Add a different form _NM_GET_PRIVATE_VOID() where self pointer can be any
non-const pointer and returns a non-const private pointer after casting.
I think NM_CACHED_QUARK_FCN() is better because:
- the implementation is in our hand, meaning it is clear that
putting a "static" before NM_CACHED_QUARK_FCN() is guaranteed to
work -- without relying on G_DEFINE_QUARK() to be defined in a way
that this works (in fact, we currently never do that and instead
make all functions non-static).
- it does not construct function names by appending "_quark".
Thus you can grep for the entire function name and finding
the place where it is implemented.
- same with the stings, where the new macro doesn't stringify the
argument, which is less surpising. Again, now you can grep
for the string including the double quoting.
(yes, I really use grep to understand the source-code)
NM_CACHED_QUARK_FCN() is a replacement for G_DEFINE_QUARK().
G_DEFINE_QUARK() is mostly used to define GError quarks. As
such, it always appends _quark() to the function name, which
is unfavorable because it makes it harder to grep for the
definition of the function.
In general I think that macros that defined symbols by concatenating
something should be avoided because that makes it harder to locate
where the symbol was defined.
Fix it by converting the macro to an inline function. It's anyway
nicer.
$ make src/src_libNetworkManagerBase_la-main-utils.lo
CC src/src_libNetworkManagerBase_la-main-utils.lo
In file included from ./shared/nm-utils/nm-macros-internal.h:29:0,
from ./shared/nm-default.h:178,
from src/main-utils.c:22:
src/main-utils.c: In function ‘nm_main_utils_setup_signals’:
./shared/nm-utils/nm-glib.h:144:36: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
&& glib_micro_version >= (micro))))
^
src/main-utils.c:82:6: note: in expansion of macro ‘nm_glib_check_version’
if (nm_glib_check_version (2, 36, 0)) {
^~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
Makefile:12312: recipe for target 'src/src_libNetworkManagerBase_la-main-utils.lo' failed
Similar to systemd's PROTECT_ERRNO. The difference it, that it doesn't
treat the auto-variable as internal, so it is allowed to use it. E.g.
if (!(fd = open (...)) {
NM_AUTO_PROTECT_ERRNO (errno_saved);
printf ("error: %s", g_strerror (errno_saved));
return FALSE;
}
We already have gs_fd_close, which however doesn't preserve
errno and only checks for fd != -1. Add our own define.
Downside is, we have to include stdio.h and errno.h,
which effectively ends up to be included *everywhere*.
