summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJulius Werner <jwerner@chromium.org>2020-06-10 15:35:08 -0700
committerJulius Werner <jwerner@chromium.org>2020-06-12 19:40:19 +0000
commit1e14de8bda93fda4c0ace8b6421d67e8605758bb (patch)
tree3d356c70f7f95779423480f75d866a2197c144a8
parenta2977ae72d7711fa24592f9e64d3ed62c3028664 (diff)
downloadcoreboot-1e14de8bda93fda4c0ace8b6421d67e8605758bb.tar.xz
tests: Add some basic warnings and fix resulting issues
The current test framework builds the test code without any warnings at all, which isn't great -- we have already slipped in some cases of non-void functions not returning a defined value, for example. It would likely be overkill to try to use all the same warnings we use for normal coreboot code (e.g. some stuff like -Wmissing-prototypes makes cmocka's __wrap_xxx() mock functions unnecessarily cumbersome to work with, and other things like -Wvla may be appropriate for firmware but is probably too aggressive for some simple test code). Therefore, let's just add some of the stuff that points out the most obvious errors. Signed-off-by: Julius Werner <jwerner@chromium.org> Change-Id: I4d9801f52a8551f55f419f4141dc21ccb835d676 Reviewed-on: https://review.coreboot.org/c/coreboot/+/42259 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Jan Dabros <jsd@semihalf.com> Reviewed-by: Paul Fagerburg <pfagerburg@chromium.org>
-rw-r--r--tests/Makefile.inc9
-rw-r--r--tests/commonlib/region-test.c8
-rw-r--r--tests/device/i2c-test.c6
3 files changed, 17 insertions, 6 deletions
diff --git a/tests/Makefile.inc b/tests/Makefile.inc
index bfd18060ab..53c0edf2a5 100644
--- a/tests/Makefile.inc
+++ b/tests/Makefile.inc
@@ -25,7 +25,14 @@ TEST_CFLAGS = -include $(src)/include/kconfig.h \
TEST_CFLAGS += -I$(testsrc)/include/mocks -I$(testsrc)/include
TEST_CFLAGS += -I$(src)/include -I$(src)/commonlib/include \
- -I$(src)/commonlib/bsd/include -I$(src)/arch/x86/include \
+ -I$(src)/commonlib/bsd/include -I$(src)/arch/x86/include
+
+# Note: This is intentionally just a subset of the warnings in the toplevel
+# Makefile.inc. We don't need to be as strict with test code, and things like
+# -Wmissing-prototypes just make working with the test framework cumbersome.
+# Only put conservative warnings here that really detect code that's obviously
+# unintentional.
+TEST_CFLAGS += -Wall -Werror -Wundef -Wstrict-prototypes
# Path for Kconfig autoheader
TEST_CFLAGS += -I$(dir $(TEST_KCONFIG_AUTOHEADER))
diff --git a/tests/commonlib/region-test.c b/tests/commonlib/region-test.c
index 219ed31f79..545463e1dd 100644
--- a/tests/commonlib/region-test.c
+++ b/tests/commonlib/region-test.c
@@ -69,6 +69,8 @@ static ssize_t mock_readat(const struct region_device *rdev, void *buffer,
ssize_t ret = mock();
if (!ret)
return size;
+ else
+ return ret;
}
static ssize_t mock_writeat(const struct region_device *rdev, const void *buffer,
@@ -82,6 +84,8 @@ static ssize_t mock_writeat(const struct region_device *rdev, const void *buffer
ssize_t ret = mock();
if (!ret)
return size;
+ else
+ return ret;
}
static ssize_t mock_eraseat(const struct region_device *rdev, size_t offset, size_t size)
@@ -93,6 +97,8 @@ static ssize_t mock_eraseat(const struct region_device *rdev, size_t offset, siz
ssize_t ret = mock();
if (!ret)
return size;
+ else
+ return ret;
}
struct region_device_ops mock_rdev_ops = {
@@ -155,8 +161,6 @@ static void rdev_mock_defaults(void)
static void test_rdev_success(void **state)
{
- struct region_device child;
-
expect_value(mock_mmap, size, region_device_sz(&mock_rdev));
rdev_mock_defaults();
diff --git a/tests/device/i2c-test.c b/tests/device/i2c-test.c
index acf7b0741d..b564e20435 100644
--- a/tests/device/i2c-test.c
+++ b/tests/device/i2c-test.c
@@ -48,7 +48,7 @@ int __wrap_platform_i2c_transfer(unsigned int bus, struct i2c_msg *segments,
reg = tmp->buf[0];
/* Find object for requested device */
- for (i = 0; i < ARRAY_SIZE(i2c_ex_devs); i++, i2c_dev++)
+ for (i = 0; i < ARRAY_SIZE(i2c_ex_devs); i++)
if (i2c_ex_devs[i].slave == tmp->slave) {
i2c_dev = &i2c_ex_devs[i];
break;
@@ -67,6 +67,8 @@ int __wrap_platform_i2c_transfer(unsigned int bus, struct i2c_msg *segments,
if (tmp->flags & I2C_M_RD) {
*(tmp->buf) = i2c_dev->regs[reg].data;
};
+
+ return 0;
}
static void mock_expect_params_platform_i2c_transfer(void)
@@ -90,7 +92,6 @@ static void mock_expect_params_platform_i2c_transfer(void)
static void i2c_read_field_test(void **state)
{
- int bus, slave, reg;
int i, j;
uint8_t buf;
@@ -121,7 +122,6 @@ static void i2c_read_field_test(void **state)
static void i2c_write_field_test(void **state)
{
- int bus, slave, reg;
int i, j;
uint8_t buf, tmp;