Change in libosmocore[master]: revisit some calls of strtol(), stroul(), strtoull()

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

neels gerrit-no-reply at lists.osmocom.org
Sun Sep 5 19:12:37 UTC 2021


neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/25347 )


Change subject: revisit some calls of strtol(), stroul(), strtoull()
......................................................................

revisit some calls of strtol(), stroul(), strtoull()

Replace some with atoi(), where the VTY has already validated correct
range of the argument.

Replace others with the new osmo_str_to_int() or osmo_str_to_int64()
functions, possibly covering more detection of invalid number strings.

Leave those strtol() callers that depend on endptr to provide the next
string token.

Related: SYS#5542
Change-Id: I0ebb06e751c28f7d1cdf328de29cd227a2449391
---
M src/ctrl/control_if.c
M src/gsm/gsm23003.c
M src/gsm/gsm23236.c
M src/vty/command.c
M src/vty/cpu_sched_vty.c
M src/vty/tdef_vty.c
M utils/osmo-aka-verify.c
7 files changed, 32 insertions(+), 73 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/47/25347/1

diff --git a/src/ctrl/control_if.c b/src/ctrl/control_if.c
index a5a90d6..23c7369 100644
--- a/src/ctrl/control_if.c
+++ b/src/ctrl/control_if.c
@@ -35,6 +35,7 @@
 #include <string.h>
 #include <time.h>
 #include <unistd.h>
+#include <limits.h>
 
 #include <arpa/inet.h>
 
@@ -83,20 +84,16 @@
  *  \returns 1 on success; 0 in case of error */
 int ctrl_parse_get_num(vector vline, int i, long *num)
 {
-	char *token, *tmp;
+	char *token;
+	int64_t val;
 
 	if (i >= vector_active(vline))
 		return 0;
 	token = vector_slot(vline, i);
 
-	errno = 0;
-	if (token[0] == '\0')
+	if (osmo_str_to_int64(&val, token, 10, LONG_MIN, LONG_MAX))
 		return 0;
-
-	*num = strtol(token, &tmp, 10);
-	if (tmp[0] != '\0' || errno != 0)
-		return 0;
-
+	*num = (long)val;
 	return 1;
 }
 
diff --git a/src/gsm/gsm23003.c b/src/gsm/gsm23003.c
index c2b3de8..f3c0123 100644
--- a/src/gsm/gsm23003.c
+++ b/src/gsm/gsm23003.c
@@ -487,22 +487,23 @@
  */
 int osmo_mnc_from_str(const char *mnc_str, uint16_t *mnc, bool *mnc_3_digits)
 {
-	long int _mnc = 0;
+	int _mnc = 0;
 	bool _mnc_3_digits = false;
-	char *endptr;
 	int rc = 0;
 
 	if (!mnc_str || !isdigit((unsigned char)mnc_str[0]) || strlen(mnc_str) > 3)
 		return -EINVAL;
 
-	errno = 0;
-	_mnc = strtol(mnc_str, &endptr, 10);
-	if (errno)
-		rc = -errno;
-	else if (*endptr)
+	rc = osmo_str_to_int(&_mnc, mnc_str, 10, 0, 999);
+	/* Heed the API definition to return -EINVAL in case of surplus chars */
+	if (rc == -E2BIG)
 		return -EINVAL;
-	if (_mnc < 0 || _mnc > 999)
-		return -ERANGE;
+	/* Heed the API definition to always return negative errno */
+	if (rc > 0)
+		return -rc;
+	if (rc < 0)
+		return rc;
+
 	_mnc_3_digits = strlen(mnc_str) > 2;
 
 	if (mnc)
diff --git a/src/gsm/gsm23236.c b/src/gsm/gsm23236.c
index 01d0eb3..d4a14a5 100644
--- a/src/gsm/gsm23236.c
+++ b/src/gsm/gsm23236.c
@@ -436,27 +436,13 @@
  */
 static int osmo_nri_parse(int16_t *dst, const char *str)
 {
-	char *endp;
-	int64_t val;
+	int val;
 	int base = 10;
-
-	if (osmo_str_startswith(str, "0x")) {
-		str += 2;
+	if (osmo_str_startswith(str, "0x"))
 		base = 16;
-	}
-
-	if (!str || !str[0])
+	if (osmo_str_to_int(&val, str, base, 0, INT16_MAX))
 		return -1;
-
-	errno = 0;
-	val = strtoull(str, &endp, base);
-	if (errno || *endp != '\0')
-		return -1;
-
-	if (val < 0 || val > INT16_MAX)
-		return -1;
-
-	*dst = val;
+	*dst = (int16_t)val;
 	return 0;
 }
 
diff --git a/src/vty/command.c b/src/vty/command.c
index bb6a665..ba6daa4 100644
--- a/src/vty/command.c
+++ b/src/vty/command.c
@@ -1349,7 +1349,6 @@
 	int colons = 0, nums = 0, double_colon = 0;
 	int mask;
 	const char *sp = NULL;
-	char *endptr = NULL;
 
 	if (str == NULL)
 		return PARTLY_MATCH;
@@ -1447,11 +1446,7 @@
 	if (state < STATE_MASK)
 		return PARTLY_MATCH;
 
-	mask = strtol(str, &endptr, 10);
-	if (*endptr != '\0')
-		return NO_MATCH;
-
-	if (mask < 0 || mask > 128)
+	if (osmo_str_to_int(&mask, str, 10, 0, 128))
 		return NO_MATCH;
 
 /* I don't know why mask < 13 makes command match partly.
@@ -3803,16 +3798,7 @@
       "Set number of lines on a screen\n"
       "Number of lines on screen (0 for no pausing)\n")
 {
-	int lines;
-	char *endptr = NULL;
-
-	lines = strtol(argv[0], &endptr, 10);
-	if (lines < 0 || lines > 512 || *endptr != '\0') {
-		vty_out(vty, "length is malformed%s", VTY_NEWLINE);
-		return CMD_WARNING;
-	}
-	vty->lines = lines;
-
+	vty->lines = atoi(argv[0]);
 	return CMD_SUCCESS;
 }
 
@@ -3831,16 +3817,7 @@
       "System wide terminal length configuration\n"
       "Number of lines of VTY (0 means no line control)\n")
 {
-	int lines;
-	char *endptr = NULL;
-
-	lines = strtol(argv[0], &endptr, 10);
-	if (lines < 0 || lines > 512 || *endptr != '\0') {
-		vty_out(vty, "length is malformed%s", VTY_NEWLINE);
-		return CMD_WARNING;
-	}
-	host.lines = lines;
-
+	host.lines = atoi(argv[0]);
 	return CMD_SUCCESS;
 }
 
diff --git a/src/vty/cpu_sched_vty.c b/src/vty/cpu_sched_vty.c
index 4ccc627..0b4b249 100644
--- a/src/vty/cpu_sched_vty.c
+++ b/src/vty/cpu_sched_vty.c
@@ -276,7 +276,6 @@
 static enum sched_vty_thread_id procname2pid(pid_t *res_pid, const char *str, bool applynow)
 {
 	size_t i, len;
-	char *end;
 	bool is_pid = true;
 
 	if (strcmp(str, "all") == 0) {
@@ -297,12 +296,12 @@
 		}
 	}
 	if (is_pid) {
-		errno = 0;
-		*res_pid = strtoul(str, &end, 0);
-		if ((errno == ERANGE && *res_pid == ULONG_MAX) || (errno && !*res_pid) ||
-		    str == end) {
+		int64_t val;
+		if (osmo_str_to_int64(&val, str, 0, 0, INT64_MAX))
 			return SCHED_VTY_THREAD_UNKNOWN;
-		}
+		*res_pid = (pid_t)val;
+		if (*res_pid != val)
+			return SCHED_VTY_THREAD_UNKNOWN;
 		if (!applynow || proc_tid_exists(*res_pid))
 			return SCHED_VTY_THREAD_ID;
 		else
diff --git a/src/vty/tdef_vty.c b/src/vty/tdef_vty.c
index 0556d8c..09459f1 100644
--- a/src/vty/tdef_vty.c
+++ b/src/vty/tdef_vty.c
@@ -50,10 +50,9 @@
  */
 struct osmo_tdef *osmo_tdef_vty_parse_T_arg(struct vty *vty, struct osmo_tdef *tdefs, const char *T_str)
 {
-	long l;
+	int l;
 	int T;
 	struct osmo_tdef *t;
-	char *endptr;
 	const char *T_nr_str;
 	int sign = 1;
 
@@ -77,9 +76,7 @@
 		return NULL;
 	}
 
-	errno = 0;
-	l = strtol(T_nr_str, &endptr, 10);
-	if (errno || *endptr || l > INT_MAX || l < 0) {
+	if (osmo_str_to_int(&l, T_nr_str, 10, 0, INT_MAX)) {
 		vty_out(vty, "%% Invalid T timer argument (should be 'T1234' or 'X1234'): '%s'%s", T_str, VTY_NEWLINE);
 		return NULL;
 	}
diff --git a/utils/osmo-aka-verify.c b/utils/osmo-aka-verify.c
index 086add5..bbc65ef 100644
--- a/utils/osmo-aka-verify.c
+++ b/utils/osmo-aka-verify.c
@@ -123,6 +123,7 @@
 	bool opc_is_set = false;
 	bool amf_is_set = false;
 	bool opc_is_op = false;
+	int64_t val64;
 
 	while (1) {
 		int c;
@@ -169,7 +170,8 @@
 			amf_is_set = true;
 			break;
 		case 's':
-			g_sqn = strtoull(optarg, 0, 10);
+			rc = osmo_str_to_int64(&val64, optarg, 10, 0, INT64_MAX);
+			g_sqn = (unsigned long long)val64;
 			sqn_is_set = true;
 			break;
 		case 'r':

-- 
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/25347
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I0ebb06e751c28f7d1cdf328de29cd227a2449391
Gerrit-Change-Number: 25347
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr at sysmocom.de>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210905/caeca529/attachment.htm>


More information about the gerrit-log mailing list