all: merge branch 'th/strstrip'

https://github.com/NetworkManager/NetworkManager/pull/329

5 files changed, 219 insertions, 143 deletions

diff --git a/clients/common/nm-meta-setting-desc.c b/clients/common/nm-meta-setting-desc.c
index d509b7a7e9..d47e1bd67e 100644
--- a/clients/common/nm-meta-setting-desc.c
+++ b/clients/common/nm-meta-setting-desc.c
@@ -305,7 +305,7 @@ _parse_ip_route (int family,
 	nm_assert (str);
 	nm_assert (!error || !*error);
 
-	str_clean = nm_strstrip_avoid_copy (str, &str_clean_free);
+	str_clean = nm_strstrip_avoid_copy_a (300, str, &str_clean_free);
 	routev = nm_utils_strsplit_set (str_clean, " \t", FALSE);
 	if (!routev) {
 		g_set_error (error, 1, 0,
@@ -479,7 +479,7 @@ _parse_team_link_watcher (const char *str,
 	nm_assert (str);
 	nm_assert (!error || !*error);
 
-	str_clean = nm_strstrip_avoid_copy (str, &str_clean_free);
+	str_clean = nm_strstrip_avoid_copy_a (300, str, &str_clean_free);
 	watcherv = nm_utils_strsplit_set (str_clean, " \t", FALSE);
 	if (!watcherv) {
 		g_set_error (error, 1, 0, "'%s' is not valid", str);
@@ -1275,7 +1275,7 @@ _set_fcn_gobject_int (ARGS_SET_FCN)
 			gs_free char *vv_free = NULL;
 			const char *vv;
 
-			vv = nm_strstrip_avoid_copy (value, &vv_free);
+			vv = nm_strstrip_avoid_copy_a (300, value, &vv_free);
 			for (; value_infos->nick; value_infos++) {
 				if (nm_streq (value_infos->nick, vv)) {
 					v = value_infos->value;
@@ -2169,7 +2169,7 @@ _set_fcn_gobject_bytes (ARGS_SET_FCN)
 	if (_SET_FCN_DO_RESET_DEFAULT (property_info, modifier, value))
 		return _gobject_property_reset_default (setting, property_info->property_name);
 
-	val_strip = nm_strstrip_avoid_copy (value, &val_strip_free);
+	val_strip = nm_strstrip_avoid_copy_a (300, value, &val_strip_free);
 
 	/* First try hex string in the format of AAbbCCDd */
 	bytes = nm_utils_hexstr2bin (val_strip);
@@ -2253,7 +2253,7 @@ _set_fcn_cert_8021x (ARGS_SET_FCN)
 	if (_SET_FCN_DO_RESET_DEFAULT (property_info, modifier, value))
 		return _gobject_property_reset_default (setting, property_info->property_name);
 
-	value = nm_strstrip_avoid_copy (value, &value_to_free);
+	value = nm_strstrip_avoid_copy_a (300, value, &value_to_free);
 
 	if (strncmp (value, NM_SETTING_802_1X_CERT_SCHEME_PREFIX_PKCS11, NM_STRLEN (NM_SETTING_802_1X_CERT_SCHEME_PREFIX_PKCS11)) == 0)
 		scheme = NM_SETTING_802_1X_CK_SCHEME_PKCS11;
@@ -3330,7 +3330,7 @@ _set_fcn_ip_config_gateway (ARGS_SET_FCN)
 	if (_SET_FCN_DO_RESET_DEFAULT (property_info, modifier, value))
 		return _gobject_property_reset_default (setting, property_info->property_name);
 
-	value = nm_strstrip_avoid_copy (value, &value_to_free);
+	value = nm_strstrip_avoid_copy_a (300, value, &value_to_free);
 
 	if (!nm_utils_ipaddr_valid (addr_family, value)) {
 		g_set_error (error, NM_UTILS_ERROR, NM_UTILS_ERROR_INVALID_ARGUMENT,
@@ -4209,7 +4209,7 @@ _set_fcn_ethtool (ARGS_SET_FCN)
 		goto set;
 	}
 
-	value = nm_strstrip_avoid_copy (value, &value_to_free);
+	value = nm_strstrip_avoid_copy_a (300, value, &value_to_free);
 
 	if (NM_IN_STRSET (value, "1", "yes", "true", "on"))
 		val = NM_TERNARY_TRUE;
diff --git a/shared/nm-utils/nm-macros-internal.h b/shared/nm-utils/nm-macros-internal.h
index 245bb1d8af..25e5559744 100644
--- a/shared/nm-utils/nm-macros-internal.h
+++ b/shared/nm-utils/nm-macros-internal.h
@@ -1342,14 +1342,14 @@ _NM_BACKPORT_SYMBOL_IMPL(version, return_type, func, _##func##_##version, args_t
 
 #define nm_str_skip_leading_spaces(str) \
 	({ \
-		typeof (*(str)) *_str = (str); \
-		_nm_unused const char *_str_type_check = _str; \
+		typeof (*(str)) *_str_sls = (str); \
+		_nm_unused const char *const _str_type_check = _str_sls; \
 		\
-		if (_str) { \
-			while (g_ascii_isspace (_str[0])) \
-				_str++; \
+		if (_str_sls) { \
+			while (g_ascii_isspace (_str_sls[0])) \
+				_str_sls++; \
 		} \
-		_str; \
+		_str_sls; \
 	})
 
 static inline char *
@@ -1386,6 +1386,34 @@ nm_strstrip_avoid_copy (const char *str, char **str_free)
 	return s;
 }
 
+#define nm_strstrip_avoid_copy_a(alloca_maxlen, str, out_str_free) \
+	({ \
+		const char *_str_ssac = (str); \
+		char **_out_str_free_ssac = (out_str_free); \
+		\
+		G_STATIC_ASSERT_EXPR ((alloca_maxlen) > 0); \
+		\
+		nm_assert ( _out_str_free_ssac || ((alloca_maxlen) > (str ? strlen (str) : 0u))); \
+		nm_assert (!_out_str_free_ssac || !*_out_str_free_ssac); \
+		\
+		if (_str_ssac) { \
+			_str_ssac = nm_str_skip_leading_spaces (_str_ssac); \
+			if (_str_ssac[0] != '\0') { \
+				gsize _l = strlen (_str_ssac); \
+				\
+				if (g_ascii_isspace (_str_ssac[--_l])) { \
+					while (   _l > 0 \
+					       && g_ascii_isspace (_str_ssac[_l - 1])) { \
+						_l--; \
+					} \
+					_str_ssac = nm_strndup_a ((alloca_maxlen), _str_ssac, _l, _out_str_free_ssac); \
+				} \
+			} \
+		} \
+		\
+		_str_ssac; \
+	})
+
 /* g_ptr_array_sort()'s compare function takes pointers to the
  * value. Thus, you cannot use strcmp directly. You can use
  * nm_strcmp_p().
@@ -1464,6 +1492,118 @@ nm_strcmp_p (gconstpointer a, gconstpointer b)
 
 /*****************************************************************************/
 
+/* like g_memdup(). The difference is that the @size argument is of type
+ * gsize, while g_memdup() has type guint. Since, the size of container types
+ * like GArray is guint as well, this means trying to g_memdup() an
+ * array,
+ *    g_memdup (array->data, array->len * sizeof (ElementType))
+ * will lead to integer overflow, if there are more than G_MAXUINT/sizeof(ElementType)
+ * bytes. That seems unnecessarily dangerous to me.
+ * nm_memdup() avoids that, because its size argument is always large enough
+ * to contain all data that a GArray can hold.
+ *
+ * Another minor difference to g_memdup() is that the glib version also
+ * returns %NULL if @data is %NULL. E.g. g_memdup(NULL, 1)
+ * gives %NULL, but nm_memdup(NULL, 1) crashes. I think that
+ * is desirable, because @size MUST be correct at all times. @size
+ * may be zero, but one must not claim to have non-zero bytes when
+ * passing a %NULL @data pointer.
+ */
+static inline gpointer
+nm_memdup (gconstpointer data, gsize size)
+{
+	gpointer p;
+
+	if (size == 0)
+		return NULL;
+	p = g_malloc (size);
+	memcpy (p, data, size);
+	return p;
+}
+
+static inline char *
+_nm_strndup_a_step (char *s, const char *str, gsize len)
+{
+	NM_PRAGMA_WARNING_DISABLE ("-Wstringop-truncation");
+	if (len > 0)
+		strncpy (s, str, len);
+	s[len] = '\0';
+	return s;
+	NM_PRAGMA_WARNING_REENABLE;
+}
+
+/* Similar to g_strndup(), however, if the string (including the terminating
+ * NUL char) fits into alloca_maxlen, this will alloca() the memory.
+ *
+ * It's a mix of strndup() and strndupa(), but deciding based on @alloca_maxlen
+ * which one to use.
+ *
+ * In case malloc() is necessary, @out_str_free will be set (this string
+ * must be freed afterwards). It is permissible to pass %NULL as @out_str_free,
+ * if you ensure that len < alloca_maxlen.
+ *
+ * Note that just like g_strndup(), this always returns a buffer with @len + 1
+ * bytes, even if strlen(@str) is shorter than that (NUL terminated early). We fill
+ * the buffer with strncpy(), which means, that @str is copied up to the first
+ * NUL character and then filled with NUL characters. */
+#define nm_strndup_a(alloca_maxlen, str, len, out_str_free) \
+	({ \
+		const gsize _alloca_maxlen_snd = (alloca_maxlen); \
+		const char *const _str_snd = (str); \
+		const gsize _len_snd = (len); \
+		char **const _out_str_free_snd = (out_str_free); \
+		char *_s_snd; \
+		\
+		G_STATIC_ASSERT_EXPR ((alloca_maxlen) <= 300); \
+		\
+		if (   _out_str_free_snd \
+		    && _len_snd >= _alloca_maxlen_snd) { \
+			_s_snd = g_malloc (_len_snd + 1); \
+			*_out_str_free_snd = _s_snd; \
+		} else { \
+			g_assert (_len_snd < _alloca_maxlen_snd); \
+			_s_snd = g_alloca (_len_snd + 1); \
+		} \
+		_nm_strndup_a_step (_s_snd, _str_snd, _len_snd); \
+	})
+
+/*****************************************************************************/
+
+/* generic macro to convert an int to a (heap allocated) string.
+ *
+ * Usually, an inline function nm_strdup_int64() would be enough. However,
+ * that cannot be used for guint64. So, we would also need nm_strdup_uint64().
+ * This causes subtle error potential, because the caller needs to ensure to
+ * use the right one (and compiler isn't going to help as it silently casts).
+ *
+ * Instead, this generic macro is supposed to handle all integers correctly. */
+#if _NM_CC_SUPPORT_GENERIC
+#define nm_strdup_int(val) \
+	_Generic ((val), \
+	          char:               g_strdup_printf ("%d",   (int)                (val)), \
+	          \
+	          signed char:        g_strdup_printf ("%d",   (signed)             (val)), \
+	          signed short:       g_strdup_printf ("%d",   (signed)             (val)), \
+	          signed:             g_strdup_printf ("%d",   (signed)             (val)), \
+	          signed long:        g_strdup_printf ("%ld",  (signed long)        (val)), \
+	          signed long long:   g_strdup_printf ("%lld", (signed long long)   (val)), \
+	          \
+	          unsigned char:      g_strdup_printf ("%u",   (unsigned)           (val)), \
+	          unsigned short:     g_strdup_printf ("%u",   (unsigned)           (val)), \
+	          unsigned:           g_strdup_printf ("%u",   (unsigned)           (val)), \
+	          unsigned long:      g_strdup_printf ("%lu",  (unsigned long)      (val)), \
+	          unsigned long long: g_strdup_printf ("%llu", (unsigned long long) (val))  \
+	)
+#else
+#define nm_strdup_int(val) \
+	(  (   sizeof (val) == sizeof (guint64) \
+	    && ((typeof (val)) -1) > 0) \
+	 ? g_strdup_printf ("%"G_GUINT64_FORMAT, (guint64) (val)) \
+	 : g_strdup_printf ("%"G_GINT64_FORMAT, (gint64) (val)))
+#endif
+
+/*****************************************************************************/
+
 static inline guint
 nm_encode_version (guint major, guint minor, guint micro)
 {
diff --git a/shared/nm-utils/nm-shared-utils.c b/shared/nm-utils/nm-shared-utils.c
index 33d7ad12f2..5f317e4144 100644
--- a/shared/nm-utils/nm-shared-utils.c
+++ b/shared/nm-utils/nm-shared-utils.c
@@ -1181,31 +1181,27 @@ int
 _nm_utils_ascii_str_to_bool (const char *str,
                              int default_value)
 {
-	gsize len;
-	char *s = NULL;
+	gs_free char *str_free = NULL;
 
 	if (!str)
 		return default_value;
 
-	while (str[0] && g_ascii_isspace (str[0]))
-		str++;
-
-	if (!str[0])
+	str = nm_strstrip_avoid_copy_a (300, str, &str_free);
+	if (str[0] == '\0')
 		return default_value;
 
-	len = strlen (str);
-	if (g_ascii_isspace (str[len - 1])) {
-		s = g_strdup (str);
-		g_strchomp (s);
-		str = s;
-	}
+	if (   !g_ascii_strcasecmp (str, "true")
+	    || !g_ascii_strcasecmp (str, "yes")
+	    || !g_ascii_strcasecmp (str, "on")
+	    || !g_ascii_strcasecmp (str, "1"))
+		return TRUE;
+
+	if (   !g_ascii_strcasecmp (str, "false")
+	    || !g_ascii_strcasecmp (str, "no")
+	    || !g_ascii_strcasecmp (str, "off")
+	    || !g_ascii_strcasecmp (str, "0"))
+		return FALSE;
 
-	if (!g_ascii_strcasecmp (str, "true") || !g_ascii_strcasecmp (str, "yes") || !g_ascii_strcasecmp (str, "on") || !g_ascii_strcasecmp (str, "1"))
-		default_value = TRUE;
-	else if (!g_ascii_strcasecmp (str, "false") || !g_ascii_strcasecmp (str, "no") || !g_ascii_strcasecmp (str, "off") || !g_ascii_strcasecmp (str, "0"))
-		default_value = FALSE;
-	if (s)
-		g_free (s);
 	return default_value;
 }
 
diff --git a/shared/nm-utils/nm-shared-utils.h b/shared/nm-utils/nm-shared-utils.h
index af9e1cf6e9..95ed5c946f 100644
--- a/shared/nm-utils/nm-shared-utils.h
+++ b/shared/nm-utils/nm-shared-utils.h
@@ -258,118 +258,6 @@ gboolean nm_utils_memeqzero (gconstpointer data, gsize length);
 
 /*****************************************************************************/
 
-/* like g_memdup(). The difference is that the @size argument is of type
- * gsize, while g_memdup() has type guint. Since, the size of container types
- * like GArray is guint as well, this means trying to g_memdup() an
- * array,
- *    g_memdup (array->data, array->len * sizeof (ElementType))
- * will lead to integer overflow, if there are more than G_MAXUINT/sizeof(ElementType)
- * bytes. That seems unnecessarily dangerous to me.
- * nm_memdup() avoids that, because its size argument is always large enough
- * to contain all data that a GArray can hold.
- *
- * Another minor difference to g_memdup() is that the glib version also
- * returns %NULL if @data is %NULL. E.g. g_memdup(NULL, 1)
- * gives %NULL, but nm_memdup(NULL, 1) crashes. I think that
- * is desirable, because @size MUST be correct at all times. @size
- * may be zero, but one must not claim to have non-zero bytes when
- * passing a %NULL @data pointer.
- */
-static inline gpointer
-nm_memdup (gconstpointer data, gsize size)
-{
-	gpointer p;
-
-	if (size == 0)
-		return NULL;
-	p = g_malloc (size);
-	memcpy (p, data, size);
-	return p;
-}
-
-static inline char *
-_nm_strndup_a_step (char *s, const char *str, gsize len)
-{
-	NM_PRAGMA_WARNING_DISABLE ("-Wstringop-truncation");
-	if (len > 0)
-		strncpy (s, str, len);
-	s[len] = '\0';
-	return s;
-	NM_PRAGMA_WARNING_REENABLE;
-}
-
-/* Similar to g_strndup(), however, if the string (including the terminating
- * NUL char) fits into alloca_maxlen, this will alloca() the memory.
- *
- * It's a mix of strndup() and strndupa(), but deciding based on @alloca_maxlen
- * which one to use.
- *
- * In case malloc() is necessary, @out_str_free will be set (this string
- * must be freed afterwards). It is permissible to pass %NULL as @out_str_free,
- * if you ensure that len < alloca_maxlen.
- *
- * Note that just like g_strndup(), this always returns a buffer with @len + 1
- * bytes, even if strlen(@str) is shorter than that (NUL terminated early). We fill
- * the buffer with strncpy(), which means, that @str is copied up to the first
- * NUL character and then filled with NUL characters. */
-#define nm_strndup_a(alloca_maxlen, str, len, out_str_free) \
-	({ \
-		const gsize _alloca_maxlen = (alloca_maxlen); \
-		const char *const _str = (str); \
-		const gsize _len = (len); \
-		char **const _out_str_free = (out_str_free); \
-		char *_s; \
-		\
-		G_STATIC_ASSERT_EXPR ((alloca_maxlen) <= 300); \
-		\
-		if (   _out_str_free \
-		    && _len >= _alloca_maxlen) { \
-			_s = g_malloc (_len + 1); \
-			*_out_str_free = _s; \
-		} else { \
-			g_assert (_len < _alloca_maxlen); \
-			_s = g_alloca (_len + 1); \
-		} \
-		_nm_strndup_a_step (_s, _str, _len); \
-	})
-
-/*****************************************************************************/
-
-/* generic macro to convert an int to a (heap allocated) string.
- *
- * Usually, an inline function nm_strdup_int64() would be enough. However,
- * that cannot be used for guint64. So, we would also need nm_strdup_uint64().
- * This causes subtle error potential, because the caller needs to ensure to
- * use the right one (and compiler isn't going to help as it silently casts).
- *
- * Instead, this generic macro is supposed to handle all integers correctly. */
-#if _NM_CC_SUPPORT_GENERIC
-#define nm_strdup_int(val) \
-	_Generic ((val), \
-	          char:               g_strdup_printf ("%d",   (int)                (val)), \
-	          \
-	          signed char:        g_strdup_printf ("%d",   (signed)             (val)), \
-	          signed short:       g_strdup_printf ("%d",   (signed)             (val)), \
-	          signed:             g_strdup_printf ("%d",   (signed)             (val)), \
-	          signed long:        g_strdup_printf ("%ld",  (signed long)        (val)), \
-	          signed long long:   g_strdup_printf ("%lld", (signed long long)   (val)), \
-	          \
-	          unsigned char:      g_strdup_printf ("%u",   (unsigned)           (val)), \
-	          unsigned short:     g_strdup_printf ("%u",   (unsigned)           (val)), \
-	          unsigned:           g_strdup_printf ("%u",   (unsigned)           (val)), \
-	          unsigned long:      g_strdup_printf ("%lu",  (unsigned long)      (val)), \
-	          unsigned long long: g_strdup_printf ("%llu", (unsigned long long) (val))  \
-	)
-#else
-#define nm_strdup_int(val) \
-	(  (   sizeof (val) == sizeof (guint64) \
-	    && ((typeof (val)) -1) > 0) \
-	 ? g_strdup_printf ("%"G_GUINT64_FORMAT, (guint64) (val)) \
-	 : g_strdup_printf ("%"G_GINT64_FORMAT, (gint64) (val)))
-#endif
-
-/*****************************************************************************/
-
 extern const void *const _NM_PTRARRAY_EMPTY[1];
 
 #define NM_PTRARRAY_EMPTY(type) ((type const*) _NM_PTRARRAY_EMPTY)
diff --git a/shared/nm-utils/tests/test-shared-general.c b/shared/nm-utils/tests/test-shared-general.c
index 5b220c10e0..f56bd15e50 100644
--- a/shared/nm-utils/tests/test-shared-general.c
+++ b/shared/nm-utils/tests/test-shared-general.c
@@ -392,6 +392,57 @@ test_strv_cmp (void)
 
 /*****************************************************************************/
 
+static void
+_do_strstrip_avoid_copy (const char *str)
+{
+	gs_free char *str1 = g_strdup (str);
+	gs_free char *str2 = g_strdup (str);
+	gs_free char *str3 = NULL;
+	gs_free char *str4 = NULL;
+	const char *s3;
+	const char *s4;
+
+	if (str1)
+		g_strstrip (str1);
+
+	nm_strstrip (str2);
+
+	g_assert_cmpstr (str1, ==, str2);
+
+	s3 = nm_strstrip_avoid_copy (str, &str3);
+	g_assert_cmpstr (str1, ==, s3);
+
+	s4 = nm_strstrip_avoid_copy_a (10, str, &str4);
+	g_assert_cmpstr (str1, ==, s4);
+	g_assert (!str == !s4);
+	g_assert (!s4 || strlen (s4) <= strlen (str));
+	if (s4 && s4 == &str[strlen (str) - strlen (s4)]) {
+		g_assert (!str4);
+		g_assert (s3 == s4);
+	} else if (s4 && strlen (s4) >= 10) {
+		g_assert (str4);
+		g_assert (s4 == str4);
+	} else
+		g_assert (!str4);
+
+	if (!nm_streq0 (str1, str))
+		_do_strstrip_avoid_copy (str1);
+}
+
+static void
+test_strstrip_avoid_copy (void)
+{
+	_do_strstrip_avoid_copy (NULL);
+	_do_strstrip_avoid_copy ("");
+	_do_strstrip_avoid_copy (" ");
+	_do_strstrip_avoid_copy (" a ");
+	_do_strstrip_avoid_copy (" 012345678 ");
+	_do_strstrip_avoid_copy (" 0123456789 ");
+	_do_strstrip_avoid_copy (" 01234567890 ");
+	_do_strstrip_avoid_copy (" 012345678901 ");
+}
+/*****************************************************************************/
+
 NMTST_DEFINE ();
 
 int main (int argc, char **argv)
@@ -406,6 +457,7 @@ int main (int argc, char **argv)
 	g_test_add_func ("/general/test_nm_ip4_addr_is_localhost", test_nm_ip4_addr_is_localhost);
 	g_test_add_func ("/general/test_unaligned", test_unaligned);
 	g_test_add_func ("/general/test_strv_cmp", test_strv_cmp);
+	g_test_add_func ("/general/test_strstrip_avoid_copy", test_strstrip_avoid_copy);
 
 	return g_test_run ();
 }