Merge pull request #1546 from antonio-nino-diaz-arm/an/log-misra

Fix some MISRA defect in log helpers

common/tf_log.c

@@ -28,19 +28,21 @@ void tf_log(const char *fmt, ...)
 	log_level = fmt[0];
 
 	/* Verify that log_level is one of LOG_MARKER_* macro defined in debug.h */
-	assert(log_level && log_level <= LOG_LEVEL_VERBOSE);
-	assert(log_level % 10 == 0);
+	assert((log_level > 0U) && (log_level <= LOG_LEVEL_VERBOSE));
+	assert((log_level % 10U) == 0U);
 
 	if (log_level > max_log_level)
 		return;
 
 	prefix_str = plat_log_get_prefix(log_level);
 
-	while (*prefix_str)
-		putchar(*prefix_str++);
+	while (*prefix_str != '\0') {
+		(void)putchar(*prefix_str);
+		prefix_str++;
+	}
 
 	va_start(args, fmt);
-	vprintf(fmt + 1, args);
+	(void)vprintf(fmt + 1, args);
 	va_end(args);
 }
 
@@ -52,10 +54,9 @@ void tf_log(const char *fmt, ...)
 void tf_log_set_max_level(unsigned int log_level)
 {
 	assert(log_level <= LOG_LEVEL_VERBOSE);
-	assert((log_level % 10) == 0);
+	assert((log_level % 10U) == 0U);
 
 	/* Cap log_level to the compile time maximum. */
-	if (log_level < LOG_LEVEL)
+	if (log_level < (unsigned int)LOG_LEVEL)
 		max_log_level = log_level;
-
 }

include/common/debug.h

@@ -7,6 +7,8 @@
 #ifndef DEBUG_H
 #define DEBUG_H
 
+#include <utils_def.h>
+
 /*
  * The log output macros print output to the console. These macros produce
  * compiled log output only if the LOG_LEVEL defined in the makefile (or the
@@ -18,12 +20,12 @@
  * WARN("Warning %s.\n", "message") -> WARNING: Warning message.
  */
 
-#define LOG_LEVEL_NONE			0
-#define LOG_LEVEL_ERROR			10
-#define LOG_LEVEL_NOTICE		20
-#define LOG_LEVEL_WARNING		30
-#define LOG_LEVEL_INFO			40
-#define LOG_LEVEL_VERBOSE		50
+#define LOG_LEVEL_NONE			U(0)
+#define LOG_LEVEL_ERROR			U(10)
+#define LOG_LEVEL_NOTICE		U(20)
+#define LOG_LEVEL_WARNING		U(30)
+#define LOG_LEVEL_INFO			U(40)
+#define LOG_LEVEL_VERBOSE		U(50)
 
 #ifndef __ASSEMBLY__
 #include <cdefs.h>
@@ -50,10 +52,10 @@
  */
 #define no_tf_log(fmt, ...)				\
 	do {						\
-		if (0) {				\
+		if (false) {				\
 			tf_log(fmt, ##__VA_ARGS__);	\
 		}					\
-	} while (0)
+	} while (false)
 
 #if LOG_LEVEL >= LOG_LEVEL_NOTICE
 # define NOTICE(...)	tf_log(LOG_MARKER_NOTICE __VA_ARGS__)

lib/libc/exit.c

@@ -10,7 +10,7 @@ static void (*exitfun)(void);
 
 void exit(int status)
 {
-	if (exitfun)
+	if (exitfun != NULL)
 		(*exitfun)();
 	for (;;)
 		;
@@ -18,7 +18,7 @@ void exit(int status)
 
 int atexit(void (*fun)(void))
 {
-	if (exitfun)
+	if (exitfun != NULL)
 		return -1;
 	exitfun = fun;
 

lib/libc/printf.c

@@ -6,28 +6,27 @@
 #include <assert.h>
 #include <debug.h>
 #include <stdarg.h>
+#include <stdbool.h>
 #include <stdint.h>
 
-/***********************************************************
- * The printf implementation for all BL stages
- ***********************************************************/
-
 #define get_num_va_args(_args, _lcount)				\
 	(((_lcount) > 1)  ? va_arg(_args, long long int) :	\
-	((_lcount) ? va_arg(_args, long int) : va_arg(_args, int)))
+	(((_lcount) == 1) ? va_arg(_args, long int) :		\
+			    va_arg(_args, int)))
 
 #define get_unum_va_args(_args, _lcount)				\
 	(((_lcount) > 1)  ? va_arg(_args, unsigned long long int) :	\
-	((_lcount) ? va_arg(_args, unsigned long int) : va_arg(_args, unsigned int)))
+	(((_lcount) == 1) ? va_arg(_args, unsigned long int) :		\
+			    va_arg(_args, unsigned int)))
 
 static int string_print(const char *str)
 {
 	int count = 0;
 
-	assert(str);
+	assert(str != NULL);
 
-	while (*str) {
-		putchar(*str++);
+	for ( ; *str != '\0'; str++) {
+		(void)putchar(*str);
 		count++;
 	}
 
@@ -38,26 +37,30 @@ static int unsigned_num_print(unsigned long long int unum, unsigned int radix,
 			      char padc, int padn)
 {
 	/* Just need enough space to store 64 bit decimal integer */
-	unsigned char num_buf[20];
-	int i = 0, rem, count = 0;
+	char num_buf[20];
+	int i = 0, count = 0;
+	unsigned int rem;
 
 	do {
 		rem = unum % radix;
 		if (rem < 0xa)
-			num_buf[i++] = '0' + rem;
+			num_buf[i] = '0' + rem;
 		else
-			num_buf[i++] = 'a' + (rem - 0xa);
-	} while (unum /= radix);
+			num_buf[i] = 'a' + (rem - 0xa);
+		i++;
+		unum /= radix;
+	} while (unum > 0U);
 
 	if (padn > 0) {
-		while (i < padn--) {
-			putchar(padc);
+		while (i < padn) {
+			(void)putchar(padc);
 			count++;
+			padn--;
 		}
 	}
 
 	while (--i >= 0) {
-		putchar(num_buf[i]);
+		(void)putchar(num_buf[i]);
 		count++;
 	}
 
@@ -90,11 +93,11 @@ int vprintf(const char *fmt, va_list args)
 	long long int num;
 	unsigned long long int unum;
 	char *str;
-	char padc = 0; /* Padding character */
+	char padc = '\0'; /* Padding character */
 	int padn; /* Number of characters to pad */
 	int count = 0; /* Number of printed characters */
 
-	while (*fmt) {
+	while (*fmt != '\0') {
 		l_count = 0;
 		padn = 0;
 
@@ -107,7 +110,7 @@ loop:
 			case 'd':
 				num = get_num_va_args(args, l_count);
 				if (num < 0) {
-					putchar('-');
+					(void)putchar('-');
 					unum = (unsigned long long int)-num;
 					padn--;
 				} else
@@ -122,7 +125,7 @@ loop:
 				break;
 			case 'p':
 				unum = (uintptr_t)va_arg(args, void *);
-				if (unum) {
+				if (unum > 0U) {
 					count += string_print("0x");
 					padn -= 2;
 				}
@@ -136,7 +139,7 @@ loop:
 							    padc, padn);
 				break;
 			case 'z':
-				if (sizeof(size_t) == 8)
+				if (sizeof(size_t) == 8U)
 					l_count = 2;
 
 				fmt++;
@@ -155,9 +158,9 @@ loop:
 				padn = 0;
 				fmt++;
 
-				while (1) {
+				for (;;) {
 					char ch = *fmt;
-					if (ch < '0' || ch > '9') {
+					if ((ch < '0') || (ch > '9')) {
 						goto loop;
 					}
 					padn = (padn * 10) + (ch - '0');
@@ -170,7 +173,8 @@ loop:
 			fmt++;
 			continue;
 		}
-		putchar(*fmt++);
+		(void)putchar(*fmt);
+		fmt++;
 		count++;
 	}
 

lib/libc/puts.c

@@ -10,9 +10,10 @@ int puts(const char *s)
 {
 	int count = 0;
 
-	while (*s) {
-		if (putchar(*s++) == EOF)
+	while (*s != '\0') {
+		if (putchar(*s) == EOF)
 			return EOF;
+		s++;
 		count++;
 	}
 

lib/libc/snprintf.c

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, ARM Limited and Contributors. All rights reserved.
+ * Copyright (c) 2017-2018, ARM Limited and Contributors. All rights reserved.
  *
  * SPDX-License-Identifier: BSD-3-Clause
  */
@@ -11,9 +11,12 @@
 static void string_print(char **s, size_t n, size_t *chars_printed,
 			 const char *str)
 {
-	while (*str) {
-		if (*chars_printed < n)
-			*(*s)++ = *str;
+	while (*str != '\0') {
+		if (*chars_printed < n) {
+			*(*s) = *str;
+			(*s)++;
+		}
+
 		(*chars_printed)++;
 		str++;
 	}
@@ -23,17 +26,22 @@ static void unsigned_dec_print(char **s, size_t n, size_t *chars_printed,
 			       unsigned int unum)
 {
 	/* Enough for a 32-bit unsigned decimal integer (4294967295). */
-	unsigned char num_buf[10];
-	int i = 0, rem;
+	char num_buf[10];
+	int i = 0;
+	unsigned int rem;
 
 	do {
-		rem = unum % 10;
+		rem = unum % 10U;
 		num_buf[i++] = '0' + rem;
-	} while (unum /= 10);
+		unum /= 10U;
+	} while (unum > 0U);
 
 	while (--i >= 0) {
-		if (*chars_printed < n)
-			*(*s)++ = num_buf[i];
+		if (*chars_printed < n) {
+			*(*s) = num_buf[i];
+			(*s)++;
+		}
+
 		(*chars_printed)++;
 	}
 }
@@ -58,19 +66,21 @@ int snprintf(char *s, size_t n, const char *fmt, ...)
 	int num;
 	unsigned int unum;
 	char *str;
-	size_t chars_printed = 0;
+	size_t chars_printed = 0U;
 
-	if (n == 1) {
+	if (n == 0U) {
+		/* There isn't space for anything. */
+	} else if (n == 1U) {
 		/* Buffer is too small to actually write anything else. */
 		*s = '\0';
-		n = 0;
-	} else if (n >= 2) {
+		n = 0U;
+	} else {
 		/* Reserve space for the terminator character. */
 		n--;
 	}
 
 	va_start(args, fmt);
-	while (*fmt) {
+	while (*fmt != '\0') {
 
 		if (*fmt == '%') {
 			fmt++;
@@ -81,8 +91,10 @@ int snprintf(char *s, size_t n, const char *fmt, ...)
 				num = va_arg(args, int);
 
 				if (num < 0) {
-					if (chars_printed < n)
-						*s++ = '-';
+					if (chars_printed < n) {
+						*s = '-';
+						s++;
+					}
 					chars_printed++;
 
 					unum = (unsigned int)-num;
@@ -110,16 +122,19 @@ int snprintf(char *s, size_t n, const char *fmt, ...)
 			continue;
 		}
 
-		if (chars_printed < n)
-			*s++ = *fmt;
+		if (chars_printed < n) {
+			*s = *fmt;
+			s++;
+		}
+
 		fmt++;
 		chars_printed++;
 	}
 
 	va_end(args);
 
-	if (n > 0)
+	if (n > 0U)
 		*s = '\0';
 
-	return chars_printed;
+	return (int)chars_printed;
 }

plat/common/plat_log_common.c

@@ -1,25 +1,28 @@
 /*
- * Copyright (c) 2017, ARM Limited and Contributors. All rights reserved.
+ * Copyright (c) 2017-2018, ARM Limited and Contributors. All rights reserved.
  *
  * SPDX-License-Identifier: BSD-3-Clause
  */
 
-#include <assert.h>
 #include <debug.h>
-#include <platform.h>
 
 /* Allow platforms to override the log prefix string */
 #pragma weak plat_log_get_prefix
 
-static const char *prefix_str[] = {
+static const char *plat_prefix_str[] = {
 	"ERROR:   ", "NOTICE:  ", "WARNING: ", "INFO:    ", "VERBOSE: "};
 
 const char *plat_log_get_prefix(unsigned int log_level)
 {
-	if (log_level < LOG_LEVEL_ERROR)
-		log_level = LOG_LEVEL_ERROR;
-	else if (log_level > LOG_LEVEL_VERBOSE)
-		log_level = LOG_LEVEL_VERBOSE;
+	unsigned int level;
 
-	return prefix_str[(log_level/10) - 1];
+	if (log_level < LOG_LEVEL_ERROR) {
+		level = LOG_LEVEL_ERROR;
+	} else if (log_level > LOG_LEVEL_VERBOSE) {
+		level = LOG_LEVEL_VERBOSE;
+	} else {
+		level = log_level;
+	}
+
+	return plat_prefix_str[(level / 10U) - 1U];
 }