From 80a19958dca72aa08670c8d04e26135ab17575df Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 3 May 2022 13:38:17 +0200 Subject: [PATCH 1/4] code-style: fix wrong indentation for code comments --- src/libnm-glib-aux/nm-default-glib.h | 42 ++++++------- src/libnm-std-aux/nm-default-std.h | 92 ++++++++++++++-------------- 2 files changed, 67 insertions(+), 67 deletions(-) diff --git a/src/libnm-glib-aux/nm-default-glib.h b/src/libnm-glib-aux/nm-default-glib.h index 9d04ddc407..40b22ee130 100644 --- a/src/libnm-glib-aux/nm-default-glib.h +++ b/src/libnm-glib-aux/nm-default-glib.h @@ -33,27 +33,27 @@ #if NM_MORE_ASSERTS == 0 #ifndef G_DISABLE_CAST_CHECKS /* Unless compiling with G_DISABLE_CAST_CHECKS, glib performs type checking - * during G_VARIANT_TYPE() via g_variant_type_checked_(). This is not necessary - * because commonly this cast is needed during something like - * - * g_variant_builder_init (&props, G_VARIANT_TYPE ("a{sv}")); - * - * Note that in if the variant type would be invalid, the check still - * wouldn't make the buggy code magically work. Instead of passing a - * bogus type string (bad), it would pass %NULL to g_variant_builder_init() - * (also bad). - * - * Also, a function like g_variant_builder_init() already validates - * the input type via something like - * - * g_return_if_fail (g_variant_type_is_container (type)); - * - * So, by having G_VARIANT_TYPE() also validate the type, we validate - * twice, whereas the first validation is rather pointless because it - * doesn't prevent the function to be called with invalid arguments. - * - * Just patch G_VARIANT_TYPE() to perform no check. - */ + * during G_VARIANT_TYPE() via g_variant_type_checked_(). This is not necessary + * because commonly this cast is needed during something like + * + * g_variant_builder_init (&props, G_VARIANT_TYPE ("a{sv}")); + * + * Note that in if the variant type would be invalid, the check still + * wouldn't make the buggy code magically work. Instead of passing a + * bogus type string (bad), it would pass %NULL to g_variant_builder_init() + * (also bad). + * + * Also, a function like g_variant_builder_init() already validates + * the input type via something like + * + * g_return_if_fail (g_variant_type_is_container (type)); + * + * So, by having G_VARIANT_TYPE() also validate the type, we validate + * twice, whereas the first validation is rather pointless because it + * doesn't prevent the function to be called with invalid arguments. + * + * Just patch G_VARIANT_TYPE() to perform no check. + */ #undef G_VARIANT_TYPE #define G_VARIANT_TYPE(type_string) ((const GVariantType *) (type_string)) #endif diff --git a/src/libnm-std-aux/nm-default-std.h b/src/libnm-std-aux/nm-default-std.h index 23f5fc604b..b4f88059cf 100644 --- a/src/libnm-std-aux/nm-default-std.h +++ b/src/libnm-std-aux/nm-default-std.h @@ -42,52 +42,52 @@ #if NM_MORE_ASSERTS == 0 /* The cast macros like NM_TYPE() are implemented via G_TYPE_CHECK_INSTANCE_CAST() - * and _G_TYPE_CIC(). The latter, by default performs runtime checks of the type - * by calling g_type_check_instance_cast(). - * This check has a certain overhead without being helpful. - * - * Example 1: - * static void foo (NMType *obj) - * { - * access_obj_without_check (obj); - * } - * foo ((NMType *) obj); - * // There is no runtime check and passing an invalid pointer - * // leads to a crash. - * - * Example 2: - * static void foo (NMType *obj) - * { - * access_obj_without_check (obj); - * } - * foo (NM_TYPE (obj)); - * // There is a runtime check which prints a g_warning(), but that doesn't - * // avoid the crash as NM_TYPE() cannot do anything then passing on the - * // invalid pointer. - * - * Example 3: - * static void foo (NMType *obj) - * { - * g_return_if_fail (NM_IS_TYPE (obj)); - * access_obj_without_check (obj); - * } - * foo ((NMType *) obj); - * // There is a runtime check which prints a g_critical() which also avoids - * // the crash. That is actually helpful to catch bugs and avoid crashes. - * - * Example 4: - * static void foo (NMType *obj) - * { - * g_return_if_fail (NM_IS_TYPE (obj)); - * access_obj_without_check (obj); - * } - * foo (NM_TYPE (obj)); - * // The runtime check is performed twice, with printing a g_warning() and - * // a g_critical() and avoiding the crash. - * - * Example 3 is how it should be done. Type checks in NM_TYPE() are pointless. - * Disable them for our production builds. - */ + * and _G_TYPE_CIC(). The latter, by default performs runtime checks of the type + * by calling g_type_check_instance_cast(). + * This check has a certain overhead without being helpful. + * + * Example 1: + * static void foo (NMType *obj) + * { + * access_obj_without_check (obj); + * } + * foo ((NMType *) obj); + * // There is no runtime check and passing an invalid pointer + * // leads to a crash. + * + * Example 2: + * static void foo (NMType *obj) + * { + * access_obj_without_check (obj); + * } + * foo (NM_TYPE (obj)); + * // There is a runtime check which prints a g_warning(), but that doesn't + * // avoid the crash as NM_TYPE() cannot do anything then passing on the + * // invalid pointer. + * + * Example 3: + * static void foo (NMType *obj) + * { + * g_return_if_fail (NM_IS_TYPE (obj)); + * access_obj_without_check (obj); + * } + * foo ((NMType *) obj); + * // There is a runtime check which prints a g_critical() which also avoids + * // the crash. That is actually helpful to catch bugs and avoid crashes. + * + * Example 4: + * static void foo (NMType *obj) + * { + * g_return_if_fail (NM_IS_TYPE (obj)); + * access_obj_without_check (obj); + * } + * foo (NM_TYPE (obj)); + * // The runtime check is performed twice, with printing a g_warning() and + * // a g_critical() and avoiding the crash. + * + * Example 3 is how it should be done. Type checks in NM_TYPE() are pointless. + * Disable them for our production builds. + */ #ifndef G_DISABLE_CAST_CHECKS #define G_DISABLE_CAST_CHECKS #endif From d75aae8b260bb522efcd31fd0163d1f3982a4b3c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 3 May 2022 13:31:37 +0200 Subject: [PATCH 2/4] all: drop redundant includes --- src/libnm-core-intern/nm-keyfile-utils.h | 4 ---- src/nm-priv-helper/nm-priv-helper.c | 1 - 2 files changed, 5 deletions(-) diff --git a/src/libnm-core-intern/nm-keyfile-utils.h b/src/libnm-core-intern/nm-keyfile-utils.h index f79f020f67..08f169ac94 100644 --- a/src/libnm-core-intern/nm-keyfile-utils.h +++ b/src/libnm-core-intern/nm-keyfile-utils.h @@ -12,10 +12,6 @@ /*****************************************************************************/ -#include "libnm-glib-aux/nm-shared-utils.h" - -/*****************************************************************************/ - #define NM_KEYFILE_GROUP_VPN_SECRETS "vpn-secrets" #define NM_KEYFILE_GROUPPREFIX_WIREGUARD_PEER "wireguard-peer." diff --git a/src/nm-priv-helper/nm-priv-helper.c b/src/nm-priv-helper/nm-priv-helper.c index ec064bca2d..e29113b112 100644 --- a/src/nm-priv-helper/nm-priv-helper.c +++ b/src/nm-priv-helper/nm-priv-helper.c @@ -9,7 +9,6 @@ #include "libnm-glib-aux/nm-dbus-aux.h" #include "libnm-glib-aux/nm-io-utils.h" #include "libnm-glib-aux/nm-logging-base.h" -#include "libnm-glib-aux/nm-shared-utils.h" #include "libnm-glib-aux/nm-time-utils.h" /* nm-priv-helper doesn't link with libnm-core nor libnm-base, but these From 0b7dc4137d0b225959ad32e74b6378328399408c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 3 May 2022 13:44:57 +0200 Subject: [PATCH 3/4] std-aux: include default std-aux headers by "nm-default-std.h" --- src/libnm-glib-aux/nm-default-glib.h | 2 -- src/libnm-std-aux/nm-default-std.h | 5 +++++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/libnm-glib-aux/nm-default-glib.h b/src/libnm-glib-aux/nm-default-glib.h index 40b22ee130..f39e16cc5b 100644 --- a/src/libnm-glib-aux/nm-default-glib.h +++ b/src/libnm-glib-aux/nm-default-glib.h @@ -63,8 +63,6 @@ #include "nm-gassert-patch.h" -#include "libnm-std-aux/nm-std-aux.h" -#include "libnm-std-aux/nm-std-utils.h" #include "libnm-glib-aux/nm-macros-internal.h" #include "libnm-glib-aux/nm-shared-utils.h" #include "libnm-glib-aux/nm-errno.h" diff --git a/src/libnm-std-aux/nm-default-std.h b/src/libnm-std-aux/nm-default-std.h index b4f88059cf..45c0d1538c 100644 --- a/src/libnm-std-aux/nm-default-std.h +++ b/src/libnm-std-aux/nm-default-std.h @@ -99,4 +99,9 @@ /*****************************************************************************/ +#include "libnm-std-aux/nm-std-aux.h" +#include "libnm-std-aux/nm-std-utils.h" + +/*****************************************************************************/ + #endif /* __NM_DEFAULT_STD_H__ */ From 501a7a3d001d7ca1a38adb22c714efcaa99a9aa3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 3 May 2022 13:46:31 +0200 Subject: [PATCH 4/4] daemon-helper: include "libnm-std-aux/nm-default-std.h" as first in "nm-daemon-helper.c" All our sources should include one of the "nm-default*.h" headers first. That one drags in , which must be included first and a few other basics. Which is the right "nm-default*.h" header depends on the component. In case of "nm-daemon-helper.c", it's "libnm-std-aux/nm-default-std.h". --- src/nm-daemon-helper/nm-daemon-helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/nm-daemon-helper/nm-daemon-helper.c b/src/nm-daemon-helper/nm-daemon-helper.c index 21a71e3c4a..a101bf9b4f 100644 --- a/src/nm-daemon-helper/nm-daemon-helper.c +++ b/src/nm-daemon-helper/nm-daemon-helper.c @@ -2,6 +2,8 @@ /* Copyright (C) 2021 Red Hat, Inc. */ +#include "libnm-std-aux/nm-default-std.h" + #include #include #include @@ -10,8 +12,6 @@ #include #endif -#include "libnm-std-aux/nm-std-aux.h" - enum { RETURN_SUCCESS = 0, RETURN_INVALID_CMD = 1,