HV: cleanup sprintf&string.c MISRA-C issues

main focus on integral issues, and change some functions
interface to unify the params data type; also modify to
simplify the code logic.

Signed-off-by: Minggui Cao <minggui.cao@intel.com>
Reviewed-by: Yonghua Huang <yonghua.huang@intel.com>
Acked-by: Eddie Dong <eddie.dong@intel.com>

hypervisor/debug/printf.c

@@ -6,29 +6,24 @@
 
 #include <hypervisor.h>
 
-static int charout(int cmd, const char *s, int32_t sz_arg, void *hnd)
+static int charout(int cmd, const char *s, uint32_t sz, void *hnd)
 {
 	/* pointer to an integer to store the number of characters */
 	int *nchars = (int *)hnd;
 	/* working pointer */
 	const char *p = s;
-	int32_t sz = sz_arg;
 
 	/* copy mode ? */
 	if (cmd == PRINT_CMD_COPY) {
-		/* copy all characters until NUL is found */
+		if (sz > 0U) { /* copy 'sz' characters */
-		if (sz < 0) {
-			s += console_puts(s);
-		} else { /* copy 'sz' characters */
 			s += console_write(s, sz);
 		}
 
 		*nchars += (s - p);
-		return *nchars;
 	} else {
 	/* fill mode */
 		*nchars += sz;
-		while (sz != 0) {
+		while (sz != 0U) {
 			(void)console_putc(*s);
 			sz--;
 		}

hypervisor/include/lib/sprintf.h

@@ -16,7 +16,7 @@
 /* Structure used to parse parameters and variables to subroutines. */
 struct print_param {
 	/* A pointer to the function that is used to emit characters. */
-	int (*emit)(int, const char *, int, void *);
+	int (*emit)(int, const char *, uint32_t, void *);
 	/* An opaque pointer that is passed as third argument to the emit
 	 * function.
 	 */
@@ -26,9 +26,9 @@ struct print_param {
 		/* A bitfield with the parsed format flags. */
 		uint32_t flags;
 		/* The parsed format width. */
-		int width;
+		uint32_t width;
 		/* The parsed format precision. */
-		int precision;
+		uint32_t precision;
 		/* The bitmask for unsigned values. */
 		uint64_t mask;
 		/* A pointer to the preformated value. */
@@ -56,8 +56,7 @@ int do_print(const char *fmt, struct print_param *param,
  * @return The number of bytes which would be written, even if the destination
  *         is smaller. On error a negative number is returned.
  */
-
+int vsnprintf(char *dst, size_t sz, const char *fmt, va_list args);
-int vsnprintf(char *dst, int32_t sz_arg, const char *fmt, va_list args);
 
 /** The well known snprintf() function.
  *

hypervisor/lib/sprintf.c

@@ -52,9 +52,9 @@ struct snprint_param {
 	/** The destination buffer. */
 	char *dst;
 	/** The size of the destination buffer. */
-	int sz;
+	uint32_t sz;
 	/** Counter for written chars. */
-	int wrtn;
+	uint32_t wrtn;
 };
 
 /** The characters to use for upper case hexadecimal conversion.
@@ -83,28 +83,22 @@ static const char lower_hex_digits[] = {
 	'a', 'b', 'c', 'd', 'e', 'f', 'x'
 };
 
-static const char *get_int(const char *s, int *x)
+static const char *get_param(const char *s, uint32_t *x)
 {
-	int negative = 0;
+	*x = 0U;
-	*x = 0;
 
-	/* evaluate leading '-' for negative numbers */
+	/* ignore '-' for negative numbers, it will be handled in flags*/
 	if (*s == '-') {
-		negative = 1;
 		++s;
 	}
 
 	/* parse uint32_teger */
 	while ((*s >= '0') && (*s <= '9')) {
-		*x = (*x * 10) + (*s - '0');
+		char delta = *s - '0';
+		*x = *x * 10U + (uint32_t)delta;
 		s++;
 	}
 
-	/* apply sign to result */
-	if (negative != 0) {
-		*x = -*x;
-	}
-
 	return s;
 }
 
@@ -195,26 +189,21 @@ static int format_number(struct print_param *param)
 	/* effective width of the result */
 	uint32_t width;
 	/* number of characters to insert for width (w) and precision (p) */
-	uint32_t p, w;
+	uint32_t p = 0U, w = 0U;
 	/* the result */
-	int res;
+	int res = 0;
 
 	/* initialize variables */
-	p = 0U;
-	w = 0U;
-	res = 0;
 	width = param->vars.valuelen + param->vars.prefixlen;
 
 	/* calculate additional characters for precision */
-	p = (uint32_t)(param->vars.precision);
+	if (param->vars.precision > width) {
-	if (p > width) {
+		p = param->vars.precision - width;
-		p = p - width;
 	}
 
 	/* calculate additional characters for width */
-	w = (uint32_t)(param->vars.width);
+	if (param->vars.width > (width + p)) {
-	if (w > (width + p)) {
+		w = param->vars.width - (width + p);
-		w = w - (width + p);
 	}
 
 	/* handle case of right justification */
@@ -421,33 +410,25 @@ static int print_decimal(struct print_param *param, int64_t value)
 static int print_string(struct print_param *param, const char *s)
 {
 	/* the length of the string (-1) if unknown */
-	int len;
+	uint32_t len;
 	/* the number of additional characters to insert to reach the required
 	 * width
 	 */
-	uint32_t w;
+	uint32_t w = 0U;
 	/* the last result of the emit function */
 	int res;
 
-	w = 0U;
-	len = -1;
-
-	/* we need the length of the string if either width or precision is
-	 * given
-	 */
-	if ((param->vars.precision != 0)|| (param->vars.width != 0)) {
 	len = strnlen_s(s, PRINT_STRING_MAX_LEN);
-	}
 
 	/* precision gives the max. number of characters to emit. */
-	if ((param->vars.precision != 0) && (len > param->vars.precision)) {
+	if ((param->vars.precision != 0U) && (len > param->vars.precision)) {
 		len = param->vars.precision;
 	}
 
 	/* calculate the number of additional characters to get the required
 	 * width
 	 */
-	if (param->vars.width > 0 && param->vars.width > len) {
+	if (param->vars.width > 0U && param->vars.width > len) {
 		w = param->vars.width - len;
 	}
 
@@ -461,7 +442,6 @@ static int print_string(struct print_param *param, const char *s)
 		}
 	}
 
-	/* emit the string, return early if an error occurred */
 	res = param->emit(PRINT_CMD_COPY, s, len, param->data);
 	if (res < 0) {
 		return res;
@@ -527,14 +507,11 @@ int do_print(const char *fmt, struct print_param *param,
 			 *   - get the length modifier
 			 */
 			fmt = get_flags(fmt, &(param->vars.flags));
-			fmt = get_int(fmt, &(param->vars.width));
+			fmt = get_param(fmt, &(param->vars.width));
 
 			if (*fmt == '.') {
 				fmt++;
-				fmt = get_int(fmt, &(param->vars.precision));
+				fmt = get_param(fmt, &(param->vars.precision));
-				if (param->vars.precision < 0) {
-					param->vars.precision = 0;
-				}
 			}
 
 			fmt = get_length_modifier(fmt, &(param->vars.flags),
@@ -544,57 +521,61 @@ int do_print(const char *fmt, struct print_param *param,
 
 			/* a single '%'? => print out a single '%' */
 			if (ch == '%') {
-				res = param->emit(PRINT_CMD_COPY, &ch, 1,
+				res = param->emit(PRINT_CMD_COPY, &ch, 1U,
 						param->data);
 			} else if ((ch == 'd') || (ch == 'i')) {
 			/* decimal number */
+				if ((param->vars.flags &
+					PRINT_FLAG_LONG_LONG) != 0U) {
 					res = print_decimal(param,
-						((param->vars.flags &
+						__builtin_va_arg(args, long));
-						PRINT_FLAG_LONG_LONG) != 0U) ?
+				} else {
-						__builtin_va_arg(args,
+					res = print_decimal(param,
-							long long)
+						__builtin_va_arg(args, int));
-						: (long long)
+				}
-						__builtin_va_arg(args,
-							int));
 			}
 			/* unsigned decimal number */
 			else if (ch == 'u') {
 				param->vars.flags |= PRINT_FLAG_UINT32;
+				if ((param->vars.flags &
+					PRINT_FLAG_LONG_LONG) != 0U) {
 					res = print_decimal(param,
-						((param->vars.flags &
+						(int64_t)__builtin_va_arg(args,
-						PRINT_FLAG_LONG_LONG) != 0U) ?
+						uint64_t));
-						__builtin_va_arg(args,
+				} else {
-							unsigned long long)
+					res = print_decimal(param,
-						: (unsigned long long)
+						(int64_t)__builtin_va_arg(args,
-						__builtin_va_arg(args,
+						uint32_t));
-							unsigned int));
+				}
 			}
 			/* octal number */
 			else if (ch == 'o') {
+				if ((param->vars.flags &
+					PRINT_FLAG_LONG_LONG) != 0U) {
 					res = print_pow2(param,
-						((param->vars.flags &
-						PRINT_FLAG_LONG_LONG) != 0U) ?
 						__builtin_va_arg(args,
-							unsigned long long)
+						uint64_t), 3U);
-						: (unsigned long long)
+				} else {
+					res = print_pow2(param,
 						__builtin_va_arg(args,
-							uint32_t),
+						uint32_t), 3U);
-						3U);
+				}
 			}
 			/* hexadecimal number */
 			else if ((ch == 'X') || (ch == 'x')) {
 				if (ch == 'X') {
 					param->vars.flags |= PRINT_FLAG_UPPER;
 				}
+				if ((param->vars.flags &
+					PRINT_FLAG_LONG_LONG) != 0U) {
 					res = print_pow2(param,
-						((param->vars.flags &
-						PRINT_FLAG_LONG_LONG) != 0U) ?
 						__builtin_va_arg(args,
-							unsigned long long)
+						uint64_t), 4U);
-						: (unsigned long long)
+				} else {
+					res = print_pow2(param,
 						__builtin_va_arg(args,
-							uint32_t),
+						uint32_t), 4U);
-						4U);
+				}
 			}
 			/* string argument */
 			else if (ch == 's') {
@@ -635,28 +616,18 @@ int do_print(const char *fmt, struct print_param *param,
 	return res;
 }
 
-static int charmem(int cmd, const char *s, int sz, void *hnd)
+static int charmem(int cmd, const char *s, uint32_t sz, void *hnd)
 {
 	/* pointer to the snprint parameter list */
 	struct snprint_param *param = (struct snprint_param *) hnd;
 	/* pointer to the destination */
 	char *p = param->dst + param->wrtn;
 	/* characters actually written */
-	int n = 0;
+	uint32_t n = 0U;
 
 	/* copy mode ? */
 	if (cmd == PRINT_CMD_COPY) {
-		if (sz < 0) {
+		if (sz > 0U) {
-			while ((*s) != '\0') {
-				if (n < (param->sz - param->wrtn)) {
-					*p = *s;
-				}
-				p++;
-				s++;
-				n++;
-			}
-
-		} else if (sz > 0) {
 			while (((*s) != '\0') && n < sz) {
 				if (n < (param->sz - param->wrtn)) {
 					*p = *s;
@@ -665,33 +636,26 @@ static int charmem(int cmd, const char *s, int sz, void *hnd)
 				s++;
 				n++;
 			}
-		} else {
-			/* sz == 0, no copy needed. */
 		}
 
 		param->wrtn += n;
-		return n;
 	}
 	/* fill mode */
 	else {
-		n = (sz < (param->sz - param->wrtn)) ? sz : 0;
+		n = (sz < param->sz - param->wrtn) ? sz : 0U;
 		param->wrtn += sz;
-		(void)memset(p, *s, n);
+		(void)memset(p, (uint8_t)*s, n);
 	}
 
-	return n;
+	return (int)n;
 }
 
-int vsnprintf(char *dst, int32_t sz_arg, const char *fmt, va_list args)
+int vsnprintf(char *dst, size_t sz, const char *fmt, va_list args)
 {
-	char c[1];
-	/* the result of this function */
-	int32_t sz = sz_arg;
 	int res = 0;
 
-	if (sz <= 0 || (dst == NULL)) {
+	if (sz == 0U || (dst == NULL)) {
-		dst = c;
+		return -1;
-		sz = 1;
 	}
 
 	/* struct to store all necessary parameters */
@@ -722,7 +686,7 @@ int vsnprintf(char *dst, int32_t sz_arg, const char *fmt, va_list args)
 	}
 
 	/* return the number of chars which would be written */
-	res = snparam.wrtn;
+	res = (int)snparam.wrtn;
 
 	/* done */
 	return res;
@@ -738,7 +702,7 @@ int snprintf(char *dest, int sz, const char *fmt, ...)
 	va_start(args, fmt);
 
 	/* execute the printf() */
-	res = vsnprintf(dest, sz, fmt, args);
+	res = vsnprintf(dest, (size_t)sz, fmt, args);
 
 	/* destroy parameter list */
 	va_end(args);

hypervisor/lib/string.c

@@ -6,8 +6,8 @@
 #include <hypervisor.h>
 
 #define ULONG_MAX       ((uint64_t)(~0UL))              /* 0xFFFFFFFF */
-#define LONG_MAX        ((long)(ULONG_MAX >> 1))        /* 0x7FFFFFFF */
+#define LONG_MAX        (ULONG_MAX >> 1U)        /* 0x7FFFFFFF */
-#define LONG_MIN        ((long)(~LONG_MAX))             /* 0x80000000 */
+#define LONG_MIN        (~LONG_MAX)             /* 0x80000000 */
 
 #define ISSPACE(c) ((c == ' ') || (c == '\t'))
 
@@ -17,11 +17,10 @@
 long strtol_deci(const char *nptr)
 {
 	const char *s = nptr;
-	uint64_t acc;
 	char c;
-	uint64_t cutoff;
+	uint64_t acc, cutoff, cutlim;
-	int neg = 0, any, cutlim;
+	int neg = 0, any;
-	int base = 10;
+	uint64_t base = 10UL;
 
 	/*
 	 * Skip white space and pick up leading +/- sign if any.
@@ -30,6 +29,7 @@ long strtol_deci(const char *nptr)
 		c = *s;
 		s++;
 	} while (ISSPACE(c));
+
 	if (c == '-') {
 		neg = 1;
 		c = *s;
@@ -58,41 +58,36 @@ long strtol_deci(const char *nptr)
 	 * Set any if any `digits' consumed; make it negative to indicate
 	 * overflow.
 	 */
-	cutoff = (neg != 0) ? -(uint64_t)LONG_MIN : LONG_MAX;
+	cutoff = (neg != 0) ? LONG_MIN : LONG_MAX;
-	cutlim = cutoff % (uint64_t)base;
+	cutlim = cutoff % base;
-	cutoff /= (uint64_t)base;
+	cutoff /= base;
-	acc = 0U;
+	acc = 0UL;
 	any = 0;
-	do {
+
-		if (c >= '0' && c <= '9') {
+	while (c >= '0' && c <= '9') {
 		c -= '0';
-		} else {
+		if ((acc > cutoff) ||
-			break;
+			(acc == cutoff && (uint64_t)c > cutlim)) {
-		}
-		if (c >= base) {
-			break;
-		}
-		if (any < 0 || acc > cutoff || (acc == cutoff && c > cutlim)) {
 			any = -1;
+			break;
 		} else {
-			any = 1;
 			acc *= base;
 			acc += c;
 		}
 
 		c = *s;
 		s++;
-	} while (true);
+	}
 
 	if (any < 0) {
 		acc = (neg != 0) ? LONG_MIN : LONG_MAX;
 	} else if (neg != 0) {
-		acc = -acc;
+		acc = ~acc + 1UL;
 	} else {
 		/* There is no overflow and no leading '-' exists. In such case
 		 * acc already holds the right number. No action required. */
 	}
-	return acc;
+	return (long)acc;
 }
 
 /*
@@ -101,10 +96,10 @@ long strtol_deci(const char *nptr)
 uint64_t strtoul_hex(const char *nptr)
 {
 	const char *s = nptr;
-	uint64_t acc;
 	char c;
-	uint64_t cutoff;
+	uint64_t acc, cutoff, cutlim;
-	int base = 16, any, cutlim;
+	uint64_t base = 16UL;
+	int any;
 
 	/*
 	 * See strtol for comments as to the logic used.
@@ -119,9 +114,9 @@ uint64_t strtoul_hex(const char *nptr)
 		s += 2;
 	}
 
-	cutoff = (uint64_t)ULONG_MAX / (uint64_t)base;
+	cutoff = ULONG_MAX / base;
-	cutlim = (uint64_t)ULONG_MAX % (uint64_t)base;
+	cutlim = ULONG_MAX % base;
-	acc = 0U;
+	acc = 0UL;
 	any = 0;
 	do {
 		if (c >= '0' && c <= '9') {
@@ -133,13 +128,12 @@ uint64_t strtoul_hex(const char *nptr)
 		} else {
 			break;
 		}
-		if (c >= base) {
+
-			break;
+		if ((acc > cutoff) ||
-		}
+			(acc == cutoff && (uint64_t)c > cutlim)) {
-		if (any < 0 || acc > cutoff || (acc == cutoff && c > cutlim)) {
 			any = -1;
+			break;
 		} else {
-			any = 1;
 			acc *= base;
 			acc += c;
 		}
@@ -148,7 +142,7 @@ uint64_t strtoul_hex(const char *nptr)
 		s++;
 	} while (true);
 
-	if (any <= 0) {
+	if (any < 0) {
 		acc = ULONG_MAX;
 	}
 	return acc;