From 5c5876732c51adcf0e1973021bc26a663b240ec9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 4 Oct 2017 11:14:48 +0200 Subject: [PATCH 6/8] keyfile: minor cleanup in get_one_int() to use _nm_utils_ascii_str_to_int64() (cherry picked from commit 72c28cb6bcc26e6a63083e4d92f8f66ee5c121e4) (cherry picked from commit 14f0f23e77219364c0ee7ae692aae35551101ed8) --- libnm-core/nm-keyfile-reader.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/libnm-core/nm-keyfile-reader.c b/libnm-core/nm-keyfile-reader.c index eb257eeb1..15a0e406f 100644 --- a/libnm-core/nm-keyfile-reader.c +++ b/libnm-core/nm-keyfile-reader.c @@ -133,8 +133,7 @@ read_array_of_uint (GKeyFile *file, static gboolean get_one_int (KeyfileReaderInfo *info, const char *property_name, const char *str, guint32 max_val, guint32 *out) { - long tmp; - char *endptr; + gint64 tmp; g_return_val_if_fail (!info == !property_name, FALSE); @@ -145,13 +144,13 @@ get_one_int (KeyfileReaderInfo *info, const char *property_name, const char *str return FALSE; } - errno = 0; - tmp = strtol (str, &endptr, 10); - if (errno || (tmp < 0) || (tmp > max_val) || *endptr != 0) { - if (property_name) + tmp = _nm_utils_ascii_str_to_int64 (str, 10, 0, max_val, -1); + if (tmp == -1) { + if (property_name) { handle_warn (info, property_name, NM_KEYFILE_WARN_SEVERITY_WARN, _("ignoring invalid number '%s'"), str); + } return FALSE; } -- 2.13.6 From e843259d6a13e9219cf151432ed3794246c7d067 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 4 Oct 2017 11:16:36 +0200 Subject: [PATCH 7/8] keyfile: cleanup error argument for read_field() Rename @error to @out_err_str, because @error is usually used for GError output arguments. Also, make the string variables "const char *". Use nm_assert() in read_field(), because it is a static function with only four call sites. It's easily verified that the assertion holds, so no need for a run-time check in production builds. (cherry picked from commit 29e9b567f0938fd202a433e7098092f0a39723ed) (cherry picked from commit f889aa783d776afa200587b5891e3578a3033518) --- libnm-core/nm-keyfile-reader.c | 58 ++++++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 25 deletions(-) diff --git a/libnm-core/nm-keyfile-reader.c b/libnm-core/nm-keyfile-reader.c index 15a0e406f..5934c833b 100644 --- a/libnm-core/nm-keyfile-reader.c +++ b/libnm-core/nm-keyfile-reader.c @@ -249,17 +249,17 @@ build_route (KeyfileReaderInfo *info, * When @current target is %NULL, gracefully fail returning %NULL while * leaving the @current target %NULL end setting @error to %NULL; */ -static char * -read_field (char **current, char **error, const char *characters, const char *delimiters) +static const char * +read_field (char **current, const char **out_err_str, const char *characters, const char *delimiters) { - char *start; + const char *start; - g_return_val_if_fail (current, NULL); - g_return_val_if_fail (error, NULL); - g_return_val_if_fail (characters, NULL); - g_return_val_if_fail (delimiters, NULL); + nm_assert (current); + nm_assert (out_err_str); + nm_assert (characters); + nm_assert (delimiters); - *error = NULL; + *out_err_str = NULL; if (!*current) { /* graceful failure, leave '*current' NULL */ @@ -282,8 +282,8 @@ read_field (char **current, char **error, const char *characters, const char *de return start; } else { /* error, bad character */ - *error = *current; - *current = start; + *out_err_str = *current; + *current = (char *) start; return NULL; } else { @@ -332,42 +332,50 @@ read_one_ip_address_or_route (KeyfileReaderInfo *info, char **out_gateway, NMSetting *setting) { - guint32 plen = G_MAXUINT32; + guint plen; gpointer result; - char *address_str, *plen_str, *gateway_str, *metric_str, *current, *error; - gs_free char *value = NULL, *value_orig = NULL; + const char *address_str; + const char *plen_str; + const char *gateway_str; + const char *metric_str; + const char *err_str = NULL; + char *current; + gs_free char *value = NULL; + gs_free char *value_orig = NULL; #define VALUE_ORIG() (value_orig ? value_orig : (value_orig = nm_keyfile_plugin_kf_get_string (info->keyfile, setting_name, key_name, NULL))) - current = value = nm_keyfile_plugin_kf_get_string (info->keyfile, setting_name, key_name, NULL); + value = nm_keyfile_plugin_kf_get_string (info->keyfile, setting_name, key_name, NULL); if (!value) return NULL; + current = value; + /* get address field */ - address_str = read_field (¤t, &error, IP_ADDRESS_CHARS, DELIMITERS); - if (error) { + address_str = read_field (¤t, &err_str, IP_ADDRESS_CHARS, DELIMITERS); + if (err_str) { handle_warn (info, property_name, NM_KEYFILE_WARN_SEVERITY_WARN, _("unexpected character '%c' for address %s: '%s' (position %td)"), - *error, key_name, VALUE_ORIG (), error - current); + *err_str, key_name, VALUE_ORIG (), err_str - current); return NULL; } /* get prefix length field (skippable) */ - plen_str = read_field (¤t, &error, DIGITS, DELIMITERS); + plen_str = read_field (¤t, &err_str, DIGITS, DELIMITERS); /* get gateway field */ - gateway_str = read_field (¤t, &error, IP_ADDRESS_CHARS, DELIMITERS); - if (error) { + gateway_str = read_field (¤t, &err_str, IP_ADDRESS_CHARS, DELIMITERS); + if (err_str) { handle_warn (info, property_name, NM_KEYFILE_WARN_SEVERITY_WARN, _("unexpected character '%c' for %s: '%s' (position %td)"), - *error, key_name, VALUE_ORIG (), error - current); + *err_str, key_name, VALUE_ORIG (), err_str - current); return NULL; } /* for routes, get metric */ if (route) { - metric_str = read_field (¤t, &error, DIGITS, DELIMITERS); - if (error) { + metric_str = read_field (¤t, &err_str, DIGITS, DELIMITERS); + if (err_str) { handle_warn (info, property_name, NM_KEYFILE_WARN_SEVERITY_WARN, _("unexpected character '%c' in prefix length for %s: '%s' (position %td)"), - *error, key_name, VALUE_ORIG (), error - current); + *err_str, key_name, VALUE_ORIG (), err_str - current); return NULL; } } else @@ -393,7 +401,7 @@ read_one_ip_address_or_route (KeyfileReaderInfo *info, /* parse plen, fallback to defaults */ if (plen_str) { - if (!get_one_int (info, property_name, plen_str, ipv6 ? 128 : 32, &plen) + if ( !get_one_int (info, property_name, plen_str, ipv6 ? 128 : 32, &plen) || (route && plen == 0)) { plen = DEFAULT_PREFIX (route, ipv6); if ( info->error -- 2.13.6 From 0a76ddaad11baec08ab0826a5d635fa5b158c6e4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 4 Oct 2017 11:28:15 +0200 Subject: [PATCH 8/8] keyfile: fix reading/writing route metric zero Zero is a valid route metric and distinct from -1, which means unspecified. Fix reader and writer. Fixes: e374923bbe4a9f608644756f749b9bae9aa5f349 (cherry picked from commit 099be8e4db0b00d4ff3ded60a4a3cb65d55bbd40) (cherry picked from commit 482fcb507e0b7d611701d9537321cdc6d58d3b84) --- libnm-core/nm-keyfile-reader.c | 15 +++++++++------ libnm-core/nm-keyfile-writer.c | 12 +++++++----- src/settings/plugins/keyfile/tests/test-keyfile.c | 6 +++--- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/libnm-core/nm-keyfile-reader.c b/libnm-core/nm-keyfile-reader.c index 5934c833b..0ac417cdb 100644 --- a/libnm-core/nm-keyfile-reader.c +++ b/libnm-core/nm-keyfile-reader.c @@ -185,7 +185,8 @@ build_route (KeyfileReaderInfo *info, const char *gateway_str, const char *metric_str) { NMIPRoute *route; - guint32 metric = 0; + guint32 u32; + gint64 metric = -1; GError *error = NULL; g_return_val_if_fail (plen, NULL); @@ -204,9 +205,10 @@ build_route (KeyfileReaderInfo *info, **/ if ( family == AF_INET6 && !metric_str - && get_one_int (NULL, NULL, gateway_str, G_MAXUINT32, &metric)) + && get_one_int (NULL, NULL, gateway_str, G_MAXUINT32, &u32)) { + metric = u32; gateway_str = NULL; - else { + } else { if (!info->error) { handle_warn (info, property_name, NM_KEYFILE_WARN_SEVERITY_WARN, _("ignoring invalid gateway '%s' for %s route"), @@ -218,14 +220,15 @@ build_route (KeyfileReaderInfo *info, } else gateway_str = NULL; - /* parse metric, default to 0 */ + /* parse metric, default to -1 */ if (metric_str) { - if (!get_one_int (info, property_name, metric_str, G_MAXUINT32, &metric)) + if (!get_one_int (info, property_name, metric_str, G_MAXUINT32, &u32)) return NULL; + metric = u32; } route = nm_ip_route_new (family, dest_str, plen, gateway_str, - metric ? (gint64) metric : -1, + metric, &error); if (!route) { handle_warn (info, property_name, NM_KEYFILE_WARN_SEVERITY_WARN, diff --git a/libnm-core/nm-keyfile-writer.c b/libnm-core/nm-keyfile-writer.c index 6a3d9a9f4..19b734a05 100644 --- a/libnm-core/nm-keyfile-writer.c +++ b/libnm-core/nm-keyfile-writer.c @@ -137,7 +137,7 @@ write_ip_values (GKeyFile *file, GString *output; int family, i; const char *addr, *gw; - guint32 plen, metric; + guint32 plen; char key_name[64], *key_name_idx; if (!array->len) @@ -150,25 +150,27 @@ write_ip_values (GKeyFile *file, output = g_string_sized_new (2*INET_ADDRSTRLEN + 10); for (i = 0; i < array->len; i++) { + gint64 metric = -1; + if (is_route) { NMIPRoute *route = array->pdata[i]; addr = nm_ip_route_get_dest (route); plen = nm_ip_route_get_prefix (route); gw = nm_ip_route_get_next_hop (route); - metric = MAX (0, nm_ip_route_get_metric (route)); + metric = nm_ip_route_get_metric (route); } else { NMIPAddress *address = array->pdata[i]; addr = nm_ip_address_get_address (address); plen = nm_ip_address_get_prefix (address); gw = i == 0 ? gateway : NULL; - metric = 0; } g_string_set_size (output, 0); g_string_append_printf (output, "%s/%u", addr, plen); - if (metric || gw) { + if ( metric != -1 + || gw) { /* Older versions of the plugin do not support the form * "a.b.c.d/plen,,metric", so, we always have to write the * gateway, even if there isn't one. @@ -182,7 +184,7 @@ write_ip_values (GKeyFile *file, } g_string_append_printf (output, ",%s", gw); - if (metric) + if (is_route && metric != -1) g_string_append_printf (output, ",%lu", (unsigned long) metric); } diff --git a/src/settings/plugins/keyfile/tests/test-keyfile.c b/src/settings/plugins/keyfile/tests/test-keyfile.c index d9da53173..2584a7229 100644 --- a/src/settings/plugins/keyfile/tests/test-keyfile.c +++ b/src/settings/plugins/keyfile/tests/test-keyfile.c @@ -312,11 +312,11 @@ test_read_valid_wired_connection (void) check_ip_route (s_ip4, 3, "1.1.1.3", 13, NULL, -1); check_ip_route (s_ip4, 4, "1.1.1.4", 14, "2.2.2.4", -1); check_ip_route (s_ip4, 5, "1.1.1.5", 15, "2.2.2.5", -1); - check_ip_route (s_ip4, 6, "1.1.1.6", 16, "2.2.2.6", -1); + check_ip_route (s_ip4, 6, "1.1.1.6", 16, "2.2.2.6", 0); check_ip_route (s_ip4, 7, "1.1.1.7", 17, NULL, -1); check_ip_route (s_ip4, 8, "1.1.1.8", 18, NULL, -1); - check_ip_route (s_ip4, 9, "1.1.1.9", 19, NULL, -1); - check_ip_route (s_ip4, 10, "1.1.1.10", 20, NULL, -1); + check_ip_route (s_ip4, 9, "1.1.1.9", 19, NULL, 0); + check_ip_route (s_ip4, 10, "1.1.1.10", 20, NULL, 0); check_ip_route (s_ip4, 11, "1.1.1.11", 21, NULL, 21); /* Route attributes */ -- 2.13.6