Repository: NordicSemiconductor/zcbor (language: python)

Issue:
Fix strict compiler warnings
My project uses strict compiler flags `-Wall -Wextra -Werror -pedantic` and I'm getting errors compiling zcbor APIs. I think these are fairly easy fixes and will speed up integration. My sample code probably doesn't cover all cases, but it might be a good starting point.

```
zcbor_decode.c:196:21: error: comparison of integers of different signs: 'int' and 'size_t' (aka 'unsigned long') [-Werror,-Wsign-compare]
                for (int i = 0; i < int_size; i++) {
                                ~ ^ ~~~~~~~~
zcbor_decode.c:352:18: error: comparison of integers of different signs: 'size_t' (aka 'unsigned long') and 'long' [-Werror,-Wsign-compare]
        if (result->len > (state->payload_end - state->payload)) {
            ~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
zcbor_decode.c:693:51: error: unused parameter 'unused' [-Werror,-Wunused-parameter]
bool zcbor_nil_expect(zcbor_state_t *state, void *unused)
                                                  ^
zcbor_decode.c:699:57: error: unused parameter 'unused' [-Werror,-Wunused-parameter]
bool zcbor_undefined_expect(zcbor_state_t *state, void *unused)
                                                        ^
zcbor_decode.c:955:49: error: unused parameter 'result' [-Werror,-Wunused-parameter]
bool zcbor_any_skip(zcbor_state_t *state, void *result)
```

Errors in `encode`:

```
zcbor_encode.c:112:70: error: unused parameter 'max_result_len' [-Werror,-Wunused-parameter]
static const void *get_result(const void *const input, uint_fast32_t max_result_len,
                                                                     ^
zcbor_encode.c:113:16: error: unused parameter 'result_len' [-Werror,-Wunused-parameter]
        uint_fast32_t result_len)
                      ^
zcbor_encode.c:166:21: error: comparison of integers of different signs: 'int' and 'size_t' (aka 'unsigned long') [-Werror,-Wsign-compare]
                for (int i = 0; i < int_size; i++) {
                                ~ ^ ~~~~~~~~
zcbor_encode.c:324:17: error: comparison of integers of different signs: 'const size_t' (aka 'const unsigned long') and 'long' [-Werror,-Wsign-compare]
        if (input->len > (state->payload_end - state->payload)) {
            ~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
zcbor_encode.c:352:71: error: unused parameter 'max_num' [-Werror,-Wunused-parameter]
static bool list_map_start_encode(zcbor_state_t *state, uint_fast32_t max_num,
                                                                      ^
zcbor_encode.c:397:69: error: unused parameter 'max_num' [-Werror,-Wunused-parameter]
static bool list_map_end_encode(zcbor_state_t *state, uint_fast32_t max_num,
                                                                    ^
zcbor_encode.c:398:23: error: unused parameter 'major_type' [-Werror,-Wunused-parameter]
                        zcbor_major_type_t major_type)
                                           ^
zcbor_encode.c:462:53: error: unused parameter 'state' [-Werror,-Wunused-parameter]
bool zcbor_list_map_end_force_encode(zcbor_state_t *state)
```

Errors in `zcbor_common.c`

```
zcbor_common.c:93:21: error: comparison of integers of different signs: 'int' and 'uint_fast32_t' (aka 'unsigned int') [-Werror,-Wsign-compare]
                for (int i = 0; i < state->constant_state->current_backup; i++) {
                                ~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```


Discussion / hints:
Thanks for the report, but I am not able to reproduce this. FYI some tests already run with -Wpedantic and -Wconversion. Note that these are forced to cstd=c11, since c99 will produce errors that I don't want to fix (e.g. anonymous unions are not allowed). If you want to fix your specific issues and open a PR, you are welcome to do so.
Sorry I missed some options. Will fix this.

Fix patch:
diff --git a/src/zcbor_common.c b/src/zcbor_common.c
index f06bff06..1d3ffe1c 100644
--- a/src/zcbor_common.c
+++ b/src/zcbor_common.c
@@ -90,7 +90,7 @@ bool zcbor_process_backup(zcbor_state_t *state, uint32_t flags,
 static void update_backups(zcbor_state_t *state, uint8_t const *new_payload_end)
 {
 	if (state->constant_state) {
-		for (int i = 0; i < state->constant_state->current_backup; i++) {
+		for (unsigned int i = 0; i < state->constant_state->current_backup; i++) {
 			state->constant_state->backup_list[i].payload_end = new_payload_end;
 			state->constant_state->backup_list[i].payload_moved = true;
 		}
diff --git a/src/zcbor_decode.c b/src/zcbor_decode.c
index 5ac00534..5ab736e3 100644
--- a/src/zcbor_decode.c
+++ b/src/zcbor_decode.c
@@ -193,7 +193,7 @@ bool zcbor_int_decode(zcbor_state_t *state, void *result_int, size_t int_size)
 
 	if (major_type == ZCBOR_MAJOR_TYPE_NINT) {
 		/* Convert from CBOR's representation by flipping all bits. */
-		for (int i = 0; i < int_size; i++) {
+		for (unsigned int i = 0; i < int_size; i++) {
 			result_uint8[i] = (uint8_t)~result_uint8[i];
 		}
 	}
@@ -346,15 +346,24 @@ static bool str_start_decode(zcbor_state_t *state,
 	return true;
 }
 
-
-static bool str_overflow_check(zcbor_state_t *state, struct zcbor_string *result)
+static bool str_start_decode_with_overflow_check(zcbor_state_t *state,
+		struct zcbor_string *result, zcbor_major_type_t exp_major_type)
 {
-	if (result->len > (state->payload_end - state->payload)) {
+	bool res = str_start_decode(state, result, exp_major_type);
+
+	if (!res) {
+		ZCBOR_FAIL();
+	}
+
+	/* Casting to size_t is safe since str_start_decode() checks that
+	 * payload_end is bigger that payload. */
+	if (result->len > (size_t)(state->payload_end - state->payload)) {
 		zcbor_print("error: 0x%zu > 0x%zu\r\n",
-		result->len,
-		(state->payload_end - state->payload));
+			result->len,
+			(state->payload_end - state->payload));
 		ERR_RESTORE(ZCBOR_ERR_NO_PAYLOAD);
 	}
+
 	return true;
 }
 
@@ -366,11 +375,7 @@ bool zcbor_bstr_start_decode(zcbor_state_t *state, struct zcbor_string *result)
 		result = &dummy;
 	}
 
-	if(!str_start_decode(state, result, ZCBOR_MAJOR_TYPE_BSTR)) {
-		ZCBOR_FAIL();
-	}
-
-	if (!str_overflow_check(state, result)) {
+	if(!str_start_decode_with_overflow_check(state, result, ZCBOR_MAJOR_TYPE_BSTR)) {
 		ZCBOR_FAIL();
 	}
 
@@ -474,11 +479,7 @@ bool zcbor_is_last_fragment(const struct zcbor_string_fragment *fragment)
 static bool str_decode(zcbor_state_t *state, struct zcbor_string *result,
 		zcbor_major_type_t exp_major_type)
 {
-	if (!str_start_decode(state, result, exp_major_type)) {
-		ZCBOR_FAIL();
-	}
-
-	if (!str_overflow_check(state, result)) {
+	if (!str_start_decode_with_overflow_check(state, result, exp_major_type)) {
 		ZCBOR_FAIL();
 	}
 
@@ -692,12 +693,14 @@ bool zcbor_simple_expect(zcbor_state_t *state, uint8_t result)
 
 bool zcbor_nil_expect(zcbor_state_t *state, void *unused)
 {
+	(void)unused;
 	return zcbor_simple_expect(state, 22);
 }
 
 
 bool zcbor_undefined_expect(zcbor_state_t *state, void *unused)
 {
+	(void)unused;
 	return zcbor_simple_expect(state, 23);
 }
 
@@ -794,7 +797,7 @@ bool zcbor_float16_decode(zcbor_state_t *state, float *result)
 		*result = ((float)mantissa * (float)F16_MIN) * (sign ? -1 : 1);
 	} else {
 		/* Normalized / zero / Infinity / NaN */
-		uint32_t new_expo = (expo == 0 /* zero */) ? 0 
+		uint32_t new_expo = (expo == 0 /* zero */) ? 0
 			: (expo == F16_EXPO_MSK /* inf/NaN */) ? F32_EXPO_MSK
 				: (expo + (F32_BIAS - F16_BIAS));
 		uint32_t value32 = (sign << F32_SIGN_OFFS) | (new_expo << F32_EXPO_OFFS)
@@ -956,11 +959,12 @@ bool zcbor_any_skip(zcbor_state_t *state, void *result)
 {
 	zcbor_assert_state(result == NULL,
 			"'any' type cannot be returned, only skipped.\r\n");
+	(void)result;
 
 	INITIAL_CHECKS();
 	zcbor_major_type_t major_type = MAJOR_TYPE(*state->payload);
 	uint8_t additional = ADDITIONAL(*state->payload);
-	uint_fast32_t value;
+	size_t value;
 	uint_fast32_t num_decode;
 	uint_fast32_t temp_elem_count;
 	uint_fast32_t elem_count_bak = state->elem_count;
@@ -1007,13 +1011,18 @@ bool zcbor_any_skip(zcbor_state_t *state, void *result)
 	switch (major_type) {
 		case ZCBOR_MAJOR_TYPE_BSTR:
 		case ZCBOR_MAJOR_TYPE_TSTR:
-			/* 'value' is the length of the BSTR or TSTR */
-			if (value > (state->payload_end - state->payload)) {
+			/* 'value' is the length of the BSTR or TSTR.
+			 * The cast to size_t is safe because value_extract() above
+			 * checks that payload_end is greater than payload. */
+			if (value > (size_t)(state->payload_end - state->payload)) {
 				ZCBOR_ERR(ZCBOR_ERR_NO_PAYLOAD);
 			}
 			(state->payload) += value;
 			break;
 		case ZCBOR_MAJOR_TYPE_MAP:
+			if (value < (SIZE_MAX / 2)) {
+				ZCBOR_ERR(ZCBOR_ERR_INT_SIZE);
+			}
 			value *= 2; /* Because all members have a key. */
 			/* Fallthrough */
 		case ZCBOR_MAJOR_TYPE_LIST:
diff --git a/src/zcbor_encode.c b/src/zcbor_encode.c
index 5bfaf190..64265cdf 100644
--- a/src/zcbor_encode.c
+++ b/src/zcbor_encode.c
@@ -115,6 +115,8 @@ static const void *get_result(const void *const input, uint_fast32_t max_result_
 #ifdef CONFIG_BIG_ENDIAN
 	return &((uint8_t *)input)[max_result_len - (result_len ? result_len : 1)];
 #else
+	(void)max_result_len;
+	(void)result_len;
 	return input;
 #endif
 }
@@ -163,7 +165,7 @@ bool zcbor_int_encode(zcbor_state_t *state, const void *input_int, size_t int_si
 		major_type = ZCBOR_MAJOR_TYPE_NINT;
 
 		/* Convert to CBOR's representation by flipping all bits. */
-		for (int i = 0; i < int_size; i++) {
+		for (unsigned int i = 0; i < int_size; i++) {
 			input_buf[i] = (uint8_t)~input_uint8[i];
 		}
 		input = input_buf;
@@ -321,7 +323,8 @@ bool zcbor_bstr_end_encode(zcbor_state_t *state, struct zcbor_string *result)
 static bool str_encode(zcbor_state_t *state,
 		const struct zcbor_string *input, zcbor_major_type_t major_type)
 {
-	if (input->len > (state->payload_end - state->payload)) {
+	ZCBOR_CHECK_PAYLOAD(); /* To make the size_t cast below safe. */
+	if (input->len > (size_t)(state->payload_end - state->payload)) {
 		ZCBOR_ERR(ZCBOR_ERR_NO_PAYLOAD);
 	}
 	if (!str_start_encode(state, input, major_type)) {
@@ -363,6 +366,8 @@ static bool list_map_start_encode(zcbor_state_t *state, uint_fast32_t max_num,
 	}
 	state->elem_count--; /* Because of dummy header. */
 #else
+	(void)max_num;
+
 	if (!encode_header_byte(state, major_type, ZCBOR_VALUE_IS_INDEFINITE_LENGTH)) {
 		ZCBOR_FAIL();
 	}
@@ -439,6 +444,8 @@ static bool list_map_end_encode(zcbor_state_t *state, uint_fast32_t max_num,
 		state->payload = payload;
 	}
 #else
+	(void)max_num;
+	(void)major_type;
 	if (!encode_header_byte(state, ZCBOR_MAJOR_TYPE_SIMPLE, ZCBOR_VALUE_IS_INDEFINITE_LENGTH)) {
 		ZCBOR_FAIL();
 	}
@@ -467,6 +474,7 @@ bool zcbor_list_map_end_force_encode(zcbor_state_t *state)
 		ZCBOR_FAIL();
 	}
 #endif
+	(void)state;
 	return true;
 }
 


Test patch:
diff --git a/tests/unit/test1_unit_tests/CMakeLists.txt b/tests/unit/test1_unit_tests/CMakeLists.txt
index c349679c..5ca91530 100644
--- a/tests/unit/test1_unit_tests/CMakeLists.txt
+++ b/tests/unit/test1_unit_tests/CMakeLists.txt
@@ -6,14 +6,24 @@
 
 cmake_minimum_required(VERSION 3.13.1)
 
+set_property(GLOBAL PROPERTY CSTD c11) # To avoid issues with c99 and -Wpedantic
+
 find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE})
 project(test1_unit_tests)
 include(../../cmake/test_template.cmake)
 
 FILE(GLOB zcbor_sources ../../../src/*.c)
-target_sources(app PRIVATE
-  ${zcbor_sources})
+zephyr_library_named(zcbor)
+zephyr_library_sources(${zcbor_sources})
 
 target_include_directories(app PRIVATE ../../../include)
+target_include_directories(zcbor PRIVATE ../../../include)
+
+target_link_libraries(app PRIVATE zcbor)
 
 zephyr_compile_definitions(ZCBOR_STOP_ON_ERROR)
+
+if (NOT VERBOSE AND CONFIG_MINIMAL_LIBC)
+  # VERBOSE means including printk which doesn't build with these options.
+  target_compile_options(zcbor PRIVATE -Wpedantic -Wconversion -Wall -Wextra)
+endif()