Changes after code review

- reverted block size type back to size_t
- moved all mocks to the `mock` dir
- reworked Makefile to get rid of uneeded targets
- added all warnings to common.mk

firmware/src/hal/include/hal/nvmem.h

@@ -75,7 +75,7 @@ void nvmem_init();
  *
  * @return whether the block was successfully registered
  */
-bool nvmem_register_block(char* key, void* addr, uint8_t size);
+bool nvmem_register_block(char* key, void* addr, size_t size);
 
 /**
  * @brief Loads registered blocks into memory

firmware/src/hal/sgx/src/trusted/nvmem.c

@@ -36,7 +36,7 @@
 typedef struct {
     char* key;
     void* addr;
-    uint8_t size;
+    size_t size;
 } nvm_block_t;
 
 static nvm_block_t nvm_blocks[MAX_NVM_BLOCKS];
@@ -58,7 +58,7 @@ void nvmem_init() {
     nvm_blocks_count = 0;
 }
 
-bool nvmem_register_block(char* key, void* addr, uint8_t size) {
+bool nvmem_register_block(char* key, void* addr, size_t size) {
     if (nvm_blocks_count >= MAX_NVM_BLOCKS) {
         LOG("Error registering NVM block <%s>: too many blocks\n", key);
         return false;

firmware/src/hal/sgx/test/common/common.mk

@@ -21,13 +21,17 @@
 # SOFTWARE.
 
 SRCDIR = ../../src/trusted
-COMMONDIR = ../../../../common/src
 MOCKDIR = ../mock
 HALINCDIR = ../../../../hal/include
-THISCOMMONDIR = ../common
-CFLAGS  = -iquote $(COMMONDIR)
+TESTCOMMONDIR = ../common
+CFLAGS = -Wall -Wextra -Werror -Wno-unused-parameter -Wno-unused-function
 CFLAGS += -iquote $(SRCDIR) -iquote $(HALINCDIR)
-CFLAGS += -iquote $(THISCOMMONDIR)
+CFLAGS += -iquote $(TESTCOMMONDIR)
+CFLAGS += -iquote $(MOCKDIR)
 CFLAGS += -DHSM_PLATFORM_SGX
 
-include ../../../../../coverage/coverage.mk
+VPATH += $(MOCKDIR):$(SRCDIR)
+
+include ../../../../../coverage/coverage.mk
+
+CFLAGS += $(COVFLAGS)

firmware/src/hal/sgx/test/mock/mock.h

@@ -0,0 +1,30 @@
+/**
+ * The MIT License (MIT)
+ *
+ * Copyright (c) 2021 RSK Labs Ltd
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#ifndef __MOCK_H
+#define __MOCK_H
+
+#include "mock_secret_store.h"
+
+#endif // #ifndef __MOCK_H

firmware/src/hal/sgx/test/mock/mock_secret_store.c

@@ -4,12 +4,12 @@
 #include <stdlib.h>
 #include <string.h>
 
-#include "mock.h"
+#include "mock_secret_store.h"
 
 typedef struct mock_sest_register {
     char *key;
     uint8_t *secret;
-    uint8_t secret_length;
+    size_t secret_length;
 } mock_sest_register_t;
 
 #define MOCK_SEST_MAX_REGISTERS 10
@@ -23,7 +23,7 @@ typedef struct mock_secret_store {
 static mock_secret_store_t g_mock_secret_store;
 
 bool mock_sest_exists(char *key) {
-    for (int i = 0; i < g_mock_secret_store.num_registers; i++) {
+    for (size_t i = 0; i < g_mock_secret_store.num_registers; i++) {
         if (strcmp(g_mock_secret_store.registers[i].key, key) == 0) {
             return true;
         }
@@ -37,7 +37,7 @@ bool mock_sest_write(char *key, uint8_t *secret, size_t secret_length) {
         return false;
     }
     int register_index = -1;
-    for (int i = 0; i < g_mock_secret_store.num_registers; i++) {
+    for (size_t i = 0; i < g_mock_secret_store.num_registers; i++) {
         if (strcmp(g_mock_secret_store.registers[i].key, key) == 0) {
             register_index = i;
             break;
@@ -65,7 +65,7 @@ uint8_t mock_sest_read(char *key, uint8_t *dest, size_t dest_length) {
         g_mock_secret_store.fail_next_read = false;
         return 0;
     }
-    for (int i = 0; i < g_mock_secret_store.num_registers; i++) {
+    for (size_t i = 0; i < g_mock_secret_store.num_registers; i++) {
         if (strcmp(g_mock_secret_store.registers[i].key, key) == 0) {
             assert(dest_length >=
                    g_mock_secret_store.registers[i].secret_length);
@@ -83,7 +83,7 @@ void mock_sest_init() {
 }
 
 void mock_sest_reset() {
-    for (int i = 0; i < g_mock_secret_store.num_registers; i++) {
+    for (size_t i = 0; i < g_mock_secret_store.num_registers; i++) {
         free(g_mock_secret_store.registers[i].key);
         free(g_mock_secret_store.registers[i].secret);
     }

firmware/src/hal/sgx/test/common/mock.h → firmware/src/hal/sgx/test/mock/mock_secret_store.h

@@ -21,12 +21,14 @@
  * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
  * IN THE SOFTWARE.
  */
+
+#ifndef __MOCK_SECRET_STORE_H
+#define __MOCK_SECRET_STORE_H
+
 #include <stdbool.h>
 #include <stdint.h>
 #include <stdlib.h>
 
-// Mock functions for the secret store
-
 /**
  * @brief Initializes the mock secret store
  */
@@ -87,3 +89,5 @@ void mock_sest_fail_next_read(bool fail);
  * @param fail whether the next call to sest_write should fail
  */
 void mock_sest_fail_next_write(bool fail);
+
+#endif // #ifndef __MOCK_SECRET_STORE_H

firmware/src/hal/sgx/test/nvmem/Makefile

@@ -30,17 +30,7 @@ all: $(PROG)
 $(PROG): $(OBJS)
 	$(CC) $(COVFLAGS) -o $@ $^
 
-test_nvmem.o: test_nvmem.c
-	$(CC) $(CFLAGS) -c -o $@ $^
-
-nvmem.o: $(SRCDIR)/nvmem.c
-	$(CC) $(CFLAGS) $(COVFLAGS) -c -o $@ $^
-
-mock_secret_store.o: $(MOCKDIR)/mock_secret_store.c
-	$(CC) $(CFLAGS) -c -o $@ $^
-
 .PHONY: clean test
-
 clean:
 	rm -f $(PROG) *.o $(COVFILES)
 

firmware/src/hal/sgx/test/nvmem/test_nvmem.c

@@ -33,6 +33,8 @@
 #include "mock.h"
 
 #define TEST_NVMEM_NUM_BLOCKS 5
+// FIXME: sest_read will not work with blocks larger than 255 bytes. For this
+// reason we have to keep the block size below this limit for the nvmem tests.
 #define TEST_NVMEM_BLOCK_SIZE 255
 #define TEST_NVMEM_KEY_SIZE 32
 
@@ -73,7 +75,7 @@ void test_register_multiple_blocks() {
     setup();
     printf("Test nvmem register multiple blocks...\n");
 
-    size_t num_blocks = TEST_NVMEM_NUM_BLOCKS;
+    int num_blocks = TEST_NVMEM_NUM_BLOCKS;
     char blocks[num_blocks][TEST_NVMEM_BLOCK_SIZE];
     for (int i = 0; i < num_blocks; i++) {
         char key[TEST_NVMEM_KEY_SIZE];
@@ -86,7 +88,7 @@ void test_register_blocks_over_limit() {
     setup();
     printf("Test nvmem register blocks over the allowed limit...\n");
 
-    size_t num_blocks = TEST_NVMEM_NUM_BLOCKS + 1;
+    int num_blocks = TEST_NVMEM_NUM_BLOCKS + 1;
     char blocks[num_blocks][TEST_NVMEM_BLOCK_SIZE];
     for (int i = 0; i < num_blocks; i++) {
         char key[TEST_NVMEM_KEY_SIZE];