[Asterisk-code-review] core/conversions: Added string to unsigned integer and long ... (asterisk[master])

[Jenkins2]

Jenkins2 has submitted this change and it was merged. ( https://gerrit.asterisk.org/5628 )

Change subject: core/conversions: Added string to unsigned integer and long conversions
......................................................................


core/conversions: Added string to unsigned integer and long conversions

Added functions that convert a string to an unsigned integer or unsigned long.
A couple of unit test were also created to test the routines. The reasons for
adding these conversion utilities (and hopefully eventually more) are as
follows:

  * Conversion routines are functionally contained with consistent and
    better error checking
  * The function names offer a better description of what is happening
  * It encourages code reuse for easier bug fixing at a single source
  * It's simpler to use
  * It's unit testable

For instance, currently in a lot of places when converting to an integer or
similar the "sscanf" function is used. When using "sscanf" it may not be
immediately clear what's happening as it lacks semantic naming. Limited error
checking is usually done as well. For example, most of the time a check is done
to make sure the value converted, but does not check for overflows or negative
valued conversions when converting unsigned numbers.

Why use/wrap "strtoul" and not "sscanf" then? Primarily, it lacks some of the
built in error handling that "strtoul" has. For instance "strtoul" contains
overflow checks. Less so, but can still factor as reasons, "sscanf" is slightly
more complex in its use. And maybe a bit controversial, but it may be ("big if")
potentially slower than "strtoul" in some cases.

Change-Id: If7eaca4a48f8c7b89cc8b5a1f4bed2852fca82bb
---
A include/asterisk/conversions.h
A main/conversions.c
A tests/test_conversions.c
3 files changed, 275 insertions(+), 0 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, but someone else must approve
  Jenkins2: Approved for Submit
  Joshua Colp: Looks good to me, approved



diff --git a/include/asterisk/conversions.h b/include/asterisk/conversions.h
new file mode 100644
index 0000000..2997760
--- /dev/null
+++ b/include/asterisk/conversions.h
@@ -0,0 +1,62 @@
+/*
+ * Asterisk -- An open source telephony toolkit.
+ *
+ * Copyright (C) 2017, Digium, Inc.
+ *
+ * Kevin Harwell <kharwell at digium.com>
+ *
+ * See http://www.asterisk.org for more information about
+ * the Asterisk project. Please do not directly contact
+ * any of the maintainers of this project for assistance;
+ * the project provides a web site, mailing lists and IRC
+ * channels for your use.
+ *
+ * This program is free software, distributed under the terms of
+ * the GNU General Public License Version 2. See the LICENSE file
+ * at the top of the source tree.
+ */
+
+/*! \file
+ * \brief Conversion utility functions
+ */
+
+#ifndef _ASTERISK_CONVERSIONS_H
+#define _ASTERISK_CONVERSIONS_H
+
+/*!
+ * \brief Convert the given string to an unsigned integer
+ *
+ * This function will return failure for the following reasons:
+ *
+ *   The given string to convert is NULL
+ *   The given string to convert is empty.
+ *   The given string to convert is negative (starts with a '-')
+ *   The given string to convert contains non numeric values
+ *   Once converted the number is out of range (greater than UINT_MAX)
+ *
+ * \param str The string to convert
+ * \param res [out] The converted value
+ *
+ * \returns -1 if it fails to convert, 0 on success
+ */
+int ast_str_to_uint(const char *str, unsigned int *res);
+
+/*!
+ * \brief Convert the given string to an unsigned long
+ *
+ * This function will return failure for the following reasons:
+ *
+ *   The given string to convert is NULL
+ *   The given string to convert is empty.
+ *   The given string to convert is negative (starts with a '-')
+ *   The given string to convert contains non numeric values
+ *   Once converted the number is out of range (greater than ULONG_MAX)
+ *
+ * \param str The string to convert
+ * \param res [out] The converted value
+ *
+ * \returns -1 if it fails to convert, 0 on success
+ */
+int ast_str_to_ulong(const char *str, unsigned long *res);
+
+#endif /* _ASTERISK_CONVERSIONS_H */
diff --git a/main/conversions.c b/main/conversions.c
new file mode 100644
index 0000000..e73e1a2
--- /dev/null
+++ b/main/conversions.c
@@ -0,0 +1,77 @@
+/*
+ * Asterisk -- An open source telephony toolkit.
+ *
+ * Copyright (C) 2017, Digium, Inc.
+ *
+ * See http://www.asterisk.org for more information about
+ * the Asterisk project. Please do not directly contact
+ * any of the maintainers of this project for assistance;
+ * the project provides a web site, mailing lists and IRC
+ * channels for your use.
+ *
+ * This program is free software, distributed under the terms of
+ * the GNU General Public License Version 2. See the LICENSE file
+ * at the top of the source tree.
+ */
+
+/*! \file
+ *
+ * \brief Conversion utility functions
+ */
+
+/*** MODULEINFO
+	<support_level>core</support_level>
+ ***/
+
+#include <ctype.h>
+#include <errno.h>
+#include <limits.h>
+#include <stdlib.h>
+
+#include "asterisk/conversions.h"
+
+static int str_is_negative(const char *str)
+{
+	/* Ignore any preceding white space */
+	while (isspace(*str) && *++str);
+	return *str == '-';
+}
+
+int ast_str_to_uint(const char *str, unsigned int *res)
+{
+	unsigned long val;
+
+	if (ast_str_to_ulong(str, &val) || val > UINT_MAX) {
+		return -1;
+	}
+
+	*res = val;
+	return 0;
+}
+
+int ast_str_to_ulong(const char *str, unsigned long *res)
+{
+	char *end;
+	unsigned long val;
+
+	if (!str || str_is_negative(str)) {
+		return -1;
+	}
+
+	errno = 0;
+	val = strtoul(str, &end, 0);
+
+	/*
+	 * If str equals end then no digits were found. If end is not pointing to
+	 * a null character then the string contained some numbers that could be
+	 * converted, but some characters that could not, which we'll consider
+	 * invalid.
+	 */
+	if ((str == end || *end != '\0' || (errno == ERANGE && val == ULONG_MAX))) {
+		return -1;
+	}
+
+	*res = val;
+	return 0;
+
+}
diff --git a/tests/test_conversions.c b/tests/test_conversions.c
new file mode 100644
index 0000000..689aba9
--- /dev/null
+++ b/tests/test_conversions.c
@@ -0,0 +1,136 @@
+/*
+ * Asterisk -- An open source telephony toolkit.
+ *
+ * Copyright (C) 2017, Digium, Inc.
+ *
+ * Kevin Harwell <kharwell at digium.com>
+ *
+ * See http://www.asterisk.org for more information about
+ * the Asterisk project. Please do not directly contact
+ * any of the maintainers of this project for assistance;
+ * the project provides a web site, mailing lists and IRC
+ * channels for your use.
+ *
+ * This program is free software, distributed under the terms of
+ * the GNU General Public License Version 2. See the LICENSE file
+ * at the top of the source tree.
+ */
+
+/*!
+ * \file
+ * \brief Conversions Unit Tests
+ *
+ * \author Kevin Harwell <kharwell at digium.com>
+ *
+ */
+
+/*** MODULEINFO
+	<depend>TEST_FRAMEWORK</depend>
+	<support_level>core</support_level>
+ ***/
+
+#include "asterisk.h"
+
+#include "asterisk/test.h"
+#include "asterisk/module.h"
+#include "asterisk/conversions.h"
+
+#define CATEGORY "/main/conversions/"
+
+AST_TEST_DEFINE(str_to_uint)
+{
+	const char *invalid = "abc";
+	const char *invalid_partial = "7abc";
+	const char *negative = "-7";
+	const char *negative_spaces = "  -7";
+	const char *out_of_range = "9999999999";
+	const char *spaces = "  ";
+	const char *valid = "7";
+	const char *valid_spaces = "  7";
+	unsigned int val;
+	char str[64];
+
+	switch (cmd) {
+	case TEST_INIT:
+		info->name = __func__;
+		info->category = CATEGORY;
+		info->summary = "convert a string to an unsigned integer";
+		info->description = info->summary;
+		return AST_TEST_NOT_RUN;
+	case TEST_EXECUTE:
+		break;
+	}
+
+	ast_test_validate(test, ast_str_to_uint(NULL, &val));
+	ast_test_validate(test, ast_str_to_uint(invalid, &val));
+	ast_test_validate(test, ast_str_to_uint(invalid_partial, &val));
+	ast_test_validate(test, ast_str_to_uint(negative, &val));
+	ast_test_validate(test, ast_str_to_uint(negative_spaces, &val));
+	ast_test_validate(test, ast_str_to_uint(out_of_range, &val));
+	ast_test_validate(test, ast_str_to_uint(spaces, &val));
+	ast_test_validate(test, !ast_str_to_uint(valid, &val));
+	ast_test_validate(test, !ast_str_to_uint(valid_spaces, &val));
+
+	ast_test_validate(test, snprintf(str, sizeof(str), "%u", UINT_MAX) > 0);
+	ast_test_validate(test, !ast_str_to_uint(str, &val));
+	ast_test_validate(test, val == UINT_MAX);
+
+	return AST_TEST_PASS;
+}
+
+AST_TEST_DEFINE(str_to_ulong)
+{
+	const char *invalid = "abc";
+	const char *invalid_partial = "7abc";
+	const char *negative = "-7";
+	const char *negative_spaces = "  -7";
+	const char *out_of_range = "99999999999999999999";
+	const char *spaces = "  ";
+	const char *valid = "7";
+	const char *valid_spaces = "  7";
+	unsigned long val;
+	char str[64];
+
+	switch (cmd) {
+	case TEST_INIT:
+		info->name = __func__;
+		info->category = CATEGORY;
+		info->summary = "convert a string to an unsigned long";
+		info->description = info->summary;
+		return AST_TEST_NOT_RUN;
+	case TEST_EXECUTE:
+		break;
+	}
+
+	ast_test_validate(test, ast_str_to_ulong(NULL, &val));
+	ast_test_validate(test, ast_str_to_ulong(invalid, &val));
+	ast_test_validate(test, ast_str_to_ulong(invalid_partial, &val));
+	ast_test_validate(test, ast_str_to_ulong(negative, &val));
+	ast_test_validate(test, ast_str_to_ulong(negative_spaces, &val));
+	ast_test_validate(test, ast_str_to_ulong(out_of_range, &val));
+	ast_test_validate(test, ast_str_to_ulong(spaces, &val));
+	ast_test_validate(test, !ast_str_to_ulong(valid, &val));
+	ast_test_validate(test, !ast_str_to_ulong(valid_spaces, &val));
+
+	ast_test_validate(test, snprintf(str, sizeof(str), "%lu", ULONG_MAX) > 0);
+	ast_test_validate(test, !ast_str_to_ulong(str, &val));
+	ast_test_validate(test, val == ULONG_MAX);
+
+	return AST_TEST_PASS;
+}
+
+static int load_module(void)
+{
+	AST_TEST_REGISTER(str_to_uint);
+	AST_TEST_REGISTER(str_to_ulong);
+	return AST_MODULE_LOAD_SUCCESS;
+}
+
+static int unload_module(void)
+{
+	AST_TEST_UNREGISTER(str_to_uint);
+	AST_TEST_UNREGISTER(str_to_ulong);
+	return 0;
+}
+
+AST_MODULE_INFO_STANDARD(ASTERISK_GPL_KEY, "URI test module");

-- 
To view, visit https://gerrit.asterisk.org/5628
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: If7eaca4a48f8c7b89cc8b5a1f4bed2852fca82bb
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>