From d6625d51116d4de58d5554ec44304978b09a3ee0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 6 Feb 2017 18:44:31 +0100 Subject: [PATCH 1/6] settings: drop redundant range check from NMSettingBridgePort::verify() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit priv->path_cost and priv->priority can only be set as GObject properties, which already does the same range check. Hence, the checks are never reached. This also avoids a compiler warning: libnm-core/nm-setting-bridge-port.c: In function ‘verify’: libnm-core/nm-setting-bridge-port.c:132:22: error: comparison is always false due to limited range of data type [-Werror=type-limits] if (priv->path_cost > BR_MAX_PATH_COST) { ^ (cherry picked from commit 31c0c66c0e8c23a2428b3d0f5dcf7e32c71d3b92) --- libnm-core/nm-setting-bridge-port.c | 31 ++--------------------------- libnm-util/nm-setting-bridge-port.c | 30 ++-------------------------- 2 files changed, 4 insertions(+), 57 deletions(-) diff --git a/libnm-core/nm-setting-bridge-port.c b/libnm-core/nm-setting-bridge-port.c index 331fa5aea0..0116a8364c 100644 --- a/libnm-core/nm-setting-bridge-port.c +++ b/libnm-core/nm-setting-bridge-port.c @@ -115,33 +115,6 @@ nm_setting_bridge_port_get_hairpin_mode (NMSettingBridgePort *setting) static gboolean verify (NMSetting *setting, NMConnection *connection, GError **error) { - NMSettingBridgePortPrivate *priv = NM_SETTING_BRIDGE_PORT_GET_PRIVATE (setting); - - if (priv->priority > BR_MAX_PORT_PRIORITY) { - g_set_error (error, - NM_CONNECTION_ERROR, - NM_CONNECTION_ERROR_INVALID_PROPERTY, - _("'%d' is not a valid value for the property (should be <= %d)"), - priv->priority, BR_MAX_PORT_PRIORITY); - g_prefix_error (error, "%s.%s: ", - NM_SETTING_BRIDGE_PORT_SETTING_NAME, - NM_SETTING_BRIDGE_PORT_PRIORITY); - return FALSE; - } - - if (priv->path_cost > BR_MAX_PATH_COST) { - g_set_error (error, - NM_CONNECTION_ERROR, - NM_CONNECTION_ERROR_INVALID_PROPERTY, - _("'%d' is not a valid value for the property (should be <= %d)"), - priv->path_cost, BR_MAX_PATH_COST); - g_prefix_error (error, "%s.%s: ", - NM_SETTING_BRIDGE_PORT_SETTING_NAME, - NM_SETTING_BRIDGE_PORT_PATH_COST); - return FALSE; - } - - if (connection) { NMSettingConnection *s_con; const char *slave_type; @@ -202,10 +175,10 @@ set_property (GObject *object, guint prop_id, switch (prop_id) { case PROP_PRIORITY: - priv->priority = (guint16) (g_value_get_uint (value) & 0xFFFF); + priv->priority = g_value_get_uint (value); break; case PROP_PATH_COST: - priv->path_cost = (guint16) (g_value_get_uint (value) & 0xFFFF); + priv->path_cost = g_value_get_uint (value); break; case PROP_HAIRPIN_MODE: priv->hairpin_mode = g_value_get_boolean (value); diff --git a/libnm-util/nm-setting-bridge-port.c b/libnm-util/nm-setting-bridge-port.c index 3d671ae376..24417059e2 100644 --- a/libnm-util/nm-setting-bridge-port.c +++ b/libnm-util/nm-setting-bridge-port.c @@ -145,32 +145,6 @@ nm_setting_bridge_port_get_hairpin_mode (NMSettingBridgePort *setting) static gboolean verify (NMSetting *setting, GSList *all_settings, GError **error) { - NMSettingBridgePortPrivate *priv = NM_SETTING_BRIDGE_PORT_GET_PRIVATE (setting); - - if (priv->priority > BR_MAX_PORT_PRIORITY) { - g_set_error (error, - NM_SETTING_BRIDGE_PORT_ERROR, - NM_SETTING_BRIDGE_PORT_ERROR_INVALID_PROPERTY, - _("'%d' is not a valid value for the property (should be <= %d)"), - priv->priority, BR_MAX_PORT_PRIORITY); - g_prefix_error (error, "%s.%s: ", - NM_SETTING_BRIDGE_PORT_SETTING_NAME, - NM_SETTING_BRIDGE_PORT_PRIORITY); - return FALSE; - } - - if (priv->path_cost > BR_MAX_PATH_COST) { - g_set_error (error, - NM_SETTING_BRIDGE_PORT_ERROR, - NM_SETTING_BRIDGE_PORT_ERROR_INVALID_PROPERTY, - _("'%d' is not a valid value for the property (should be <= %d)"), - priv->path_cost, BR_MAX_PATH_COST); - g_prefix_error (error, "%s.%s: ", - NM_SETTING_BRIDGE_PORT_SETTING_NAME, - NM_SETTING_BRIDGE_PORT_PATH_COST); - return FALSE; - } - return TRUE; } @@ -204,10 +178,10 @@ set_property (GObject *object, guint prop_id, switch (prop_id) { case PROP_PRIORITY: - priv->priority = (guint16) (g_value_get_uint (value) & 0xFFFF); + priv->priority = g_value_get_uint (value); break; case PROP_PATH_COST: - priv->path_cost = (guint16) (g_value_get_uint (value) & 0xFFFF); + priv->path_cost = g_value_get_uint (value); break; case PROP_HAIRPIN_MODE: priv->hairpin_mode = g_value_get_boolean (value); From cdf862f04898a4bbc0520e7614915919ca73f1d5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 6 Feb 2017 18:58:44 +0100 Subject: [PATCH 2/6] shared: fix -Wtype-limits warning in nm_glib_check_version() macro MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 (cherry picked from commit 1a190b9038ca6d01c6e92887cfad8d3bb45f1823) --- shared/nm-utils/nm-glib.h | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/shared/nm-utils/nm-glib.h b/shared/nm-utils/nm-glib.h index 824a08ca0d..03251ca058 100644 --- a/shared/nm-utils/nm-glib.h +++ b/shared/nm-utils/nm-glib.h @@ -134,14 +134,17 @@ __g_type_ensure (GType type) /* Rumtime check for glib version. First do a compile time check which * (if satisfied) shortcuts the runtime check. */ -#define nm_glib_check_version(major, minor, micro) \ - ( GLIB_CHECK_VERSION ((major), (minor), (micro)) \ - || ( ( glib_major_version > (major)) \ - || ( glib_major_version == (major) \ - && glib_minor_version > (minor)) \ - || ( glib_major_version == (major) \ - && glib_minor_version == (minor) \ - && glib_micro_version >= (micro)))) +inline static gboolean +nm_glib_check_version (guint major, guint minor, guint micro) +{ + return GLIB_CHECK_VERSION (major, minor, micro) + || ( ( glib_major_version > major) + || ( glib_major_version == major + && glib_minor_version > minor) + || ( glib_major_version == major + && glib_minor_version == minor + && glib_micro_version < micro)); +} /* g_test_skip() is only available since glib 2.38. Add a compatibility wrapper. */ inline static void From edf6f0b600b29bd4cd52eba7e983c40c1ddc5ae4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 6 Feb 2017 18:28:56 +0100 Subject: [PATCH 3/6] build: fix -Wignored-qualifiers warnings ./src/nm-config-data.h:163:1: error: 'const' type qualifier on return type has no effect [-Werror,-Wignored-qualifiers] const guint nm_config_data_get_connectivity_interval (const NMConfigData *config_data); ^~~~~~ (cherry picked from commit e68cc17f438b555e3d69603ffe12c01add732ffb) --- src/nm-config-data.c | 2 +- src/nm-config-data.h | 2 +- src/nm-rfkill-manager.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/nm-config-data.c b/src/nm-config-data.c index 0d554e55ea..68e66971b2 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -246,7 +246,7 @@ nm_config_data_get_connectivity_uri (const NMConfigData *self) return NM_CONFIG_DATA_GET_PRIVATE (self)->connectivity.uri; } -const guint +guint nm_config_data_get_connectivity_interval (const NMConfigData *self) { g_return_val_if_fail (self, 0); diff --git a/src/nm-config-data.h b/src/nm-config-data.h index b5e3c67430..d7d14a61e9 100644 --- a/src/nm-config-data.h +++ b/src/nm-config-data.h @@ -160,7 +160,7 @@ gint nm_config_data_get_value_boolean (const NMConfigData *self, const char *gro char **nm_config_data_get_plugins (const NMConfigData *config_data, gboolean allow_default); const char *nm_config_data_get_connectivity_uri (const NMConfigData *config_data); -const guint nm_config_data_get_connectivity_interval (const NMConfigData *config_data); +guint nm_config_data_get_connectivity_interval (const NMConfigData *config_data); const char *nm_config_data_get_connectivity_response (const NMConfigData *config_data); const char *const*nm_config_data_get_no_auto_default (const NMConfigData *config_data); diff --git a/src/nm-rfkill-manager.c b/src/nm-rfkill-manager.c index 36afd35ec7..09a32965cc 100644 --- a/src/nm-rfkill-manager.c +++ b/src/nm-rfkill-manager.c @@ -262,7 +262,7 @@ killswitch_find_by_name (NMRfkillManager *self, const char *name) return NULL; } -static const RfKillType +static RfKillType rfkill_type_to_enum (const char *str) { g_return_val_if_fail (str != NULL, RFKILL_TYPE_UNKNOWN); From 54584cb3ab9af7506b533f87fe67e67df5313fcb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 6 Feb 2017 18:43:24 +0100 Subject: [PATCH 4/6] build: fix -Wold-style-declaration warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit libnm-core/nm-setting-bond.c:502:1: error: ‘static’ is not at beginning of declaration [-Werror=old-style-declaration] const static struct { ^~~~~ In file included from clients/cli/common.c:32:0: ./clients/common/nm-vpn-helpers.h:27:1: error: ‘typedef’ is not at beginning of declaration [-Werror=old-style-declaration] } typedef VpnPasswordName; ^ (cherry picked from commit cb365b33f3271d4cac8192f5434c672c29e01b64) --- clients/common/nm-vpn-helpers.h | 4 ++-- libnm-core/nm-setting-bond.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/clients/common/nm-vpn-helpers.h b/clients/common/nm-vpn-helpers.h index 9c23f37c5d..9ad6b3b61f 100644 --- a/clients/common/nm-vpn-helpers.h +++ b/clients/common/nm-vpn-helpers.h @@ -21,10 +21,10 @@ #include -struct { +typedef struct { const char *name; const char *ui_name; -} typedef VpnPasswordName; +} VpnPasswordName; GSList *nm_vpn_get_plugin_infos (void); diff --git a/libnm-core/nm-setting-bond.c b/libnm-core/nm-setting-bond.c index a01adc12b9..904ebbfc6d 100644 --- a/libnm-core/nm-setting-bond.c +++ b/libnm-core/nm-setting-bond.c @@ -499,7 +499,7 @@ _nm_setting_bond_mode_from_string (const char *str) #define BIT(x) (1 << (x)) -const static struct { +static const struct { const char *option; NMBondMode unsupp_modes; } bond_unsupp_modes[] = { From bbb827b4b36d773825a642bdcb6cd73e22a2bcd7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 6 Feb 2017 18:24:33 +0100 Subject: [PATCH 5/6] build: enable -Wextra warning (cherry picked from commit 5120205f9898f19e5738b2df7539447331d2c8ea) --- m4/compiler_options.m4 | 2 ++ 1 file changed, 2 insertions(+) diff --git a/m4/compiler_options.m4 b/m4/compiler_options.m4 index 5f04644534..05272b3254 100644 --- a/m4/compiler_options.m4 +++ b/m4/compiler_options.m4 @@ -62,6 +62,8 @@ if test "$GCC" = "yes" -a "$set_more_warnings" != "no"; then fi for option in \ + -Wextra \ + -Wno-missing-field-initializers \ -Wimplicit-fallthrough \ -Wshadow \ -Wmissing-declarations \ From 053bc1de9e30bd30d2fee30387b1b100b3da8536 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 6 Feb 2017 19:17:49 +0100 Subject: [PATCH 6/6] build: reorder flags in "m4/compiler_options.m4" Mostly sort alphabetically, but - keep -Wextra first - move "-Wno-*" flags to the end (cherry picked from commit 4a72c121edeaf3bb317bb30fae926045add64915) --- m4/compiler_options.m4 | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/m4/compiler_options.m4 b/m4/compiler_options.m4 index 05272b3254..c998711d72 100644 --- a/m4/compiler_options.m4 +++ b/m4/compiler_options.m4 @@ -63,28 +63,28 @@ if test "$GCC" = "yes" -a "$set_more_warnings" != "no"; then for option in \ -Wextra \ - -Wno-missing-field-initializers \ - -Wimplicit-fallthrough \ - -Wshadow \ - -Wmissing-declarations \ - -Wmissing-prototypes \ -Wdeclaration-after-statement \ - -Wformat-security \ -Wfloat-equal \ - -Wno-unused-parameter \ - -Wno-sign-compare \ - -Wno-duplicate-decl-specifier \ - -Wstrict-prototypes \ - -Wno-unused-but-set-variable \ - -Wno-format-y2k \ - -Wundef \ - -Wimplicit-function-declaration \ - -Wpointer-arith \ - -Winit-self \ -Wformat-nonliteral \ + -Wformat-security \ + -Wimplicit-fallthrough \ + -Wimplicit-function-declaration \ + -Winit-self \ + -Wmissing-declarations \ -Wmissing-include-dirs \ - -Wno-pragmas \ + -Wmissing-prototypes \ + -Wpointer-arith \ + -Wshadow \ + -Wstrict-prototypes \ + -Wundef \ + -Wno-duplicate-decl-specifier \ -Wno-format-truncation \ + -Wno-format-y2k \ + -Wno-missing-field-initializers \ + -Wno-pragmas \ + -Wno-sign-compare \ + -Wno-unused-but-set-variable \ + -Wno-unused-parameter \ ; do dnl GCC 4.4 does not warn when checking for -Wno-* flags (https://gcc.gnu.org/wiki/FAQ#wnowarning) _NM_COMPILER_FLAG([$(printf '%s' "$option" | sed 's/^-Wno-/-W/')], [],