[PATCH 0/6] Rewrite test-suite using check and fff

65 views
Skip to first unread message

Andreas J. Reichel

unread,
Oct 24, 2017, 8:27:30 AM10/24/17
to efibootg...@googlegroups.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>

This patch series is based on the last two patches on the mailing list
(from HEAD to HEAD~2):
* [PATCH] Bugfix: zeroing errno before strtol is mandatory
* [PATCH v2] Bugfix: bgenv_get: Error codes overlap return values
* [PATCH] Bugfix: env_api_fat.c: Return correct buffer size

It contains a complete replacement for the current test-suite,
because cmocka is not compatible to the license of efibootguard.

Instead, fff (fake function framework) and check is used.
* https://github.com/meekrosoft/fff (MIT License)
* https://libcheck.github.io/check/ (LGPL License)

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>

Andreas Reichel (6):
Remove cmocka test suite
Add libcheck unit testing framework
Add function faking framework as submodule
Update travis build script
Make code more modular
Add new tests based on check, fff and linker wrapping

.gitmodules | 3 +
.travis-build.sh | 33 +-
Makefile.am | 4 +-
configure.ac | 3 +
docs/COMPILE.md | 4 +-
env/env_api_fat.c | 230 +----------
env/env_config_file.c | 90 ++++
env/env_config_partitions.c | 83 ++++
env/env_disk_utils.c | 99 +++++
fff | 1 +
include/env_config_file.h | 20 +
include/env_config_partitions.h | 18 +
include/env_disk_utils.h | 20 +
include/test-interface.h | 4 +
tools/tests/Makefile.am | 65 ++-
tools/tests/fake_devices.c | 94 +++++
tools/tests/fake_devices.h | 28 ++
tools/tests/test_api.c | 315 --------------
tools/tests/test_bgenv_init_retval.c | 100 +++++
tools/tests/test_ebgenv_api.c | 641 +++++++++++++++++++++++++++++
tools/tests/test_ebgenv_api_internal.c | 416 +++++++++++++++++++
tools/tests/test_environment.c | 119 ------
tools/tests/test_main.c | 34 ++
tools/tests/test_partitions.c | 154 -------
tools/tests/test_probe_config_file.c | 202 +++++++++
tools/tests/test_probe_config_partitions.c | 99 +++++
26 files changed, 2034 insertions(+), 845 deletions(-)
create mode 100644 .gitmodules
create mode 100644 env/env_config_file.c
create mode 100644 env/env_config_partitions.c
create mode 100644 env/env_disk_utils.c
create mode 160000 fff
create mode 100644 include/env_config_file.h
create mode 100644 include/env_config_partitions.h
create mode 100644 include/env_disk_utils.h
create mode 100644 tools/tests/fake_devices.c
create mode 100644 tools/tests/fake_devices.h
delete mode 100644 tools/tests/test_api.c
create mode 100644 tools/tests/test_bgenv_init_retval.c
create mode 100644 tools/tests/test_ebgenv_api.c
create mode 100644 tools/tests/test_ebgenv_api_internal.c
delete mode 100644 tools/tests/test_environment.c
create mode 100644 tools/tests/test_main.c
delete mode 100644 tools/tests/test_partitions.c
create mode 100644 tools/tests/test_probe_config_file.c
create mode 100644 tools/tests/test_probe_config_partitions.c

--
2.14.2

Andreas J. Reichel

unread,
Oct 24, 2017, 8:27:39 AM10/24/17
to efibootg...@googlegroups.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>

Due to license incompatbility, remove cmocka from project.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---
.travis-build.sh | 4 +-
tools/tests/Makefile.am | 22 +--
tools/tests/test_api.c | 315 -----------------------------------------
tools/tests/test_environment.c | 119 ----------------
tools/tests/test_partitions.c | 154 --------------------
5 files changed, 6 insertions(+), 608 deletions(-)
delete mode 100644 tools/tests/test_api.c
delete mode 100644 tools/tests/test_environment.c
delete mode 100644 tools/tests/test_partitions.c

diff --git a/.travis-build.sh b/.travis-build.sh
index 6dc6c0c..470c710 100755
--- a/.travis-build.sh
+++ b/.travis-build.sh
@@ -29,7 +29,7 @@ install_native_deps()
'deb http://archive.ubuntu.com/ubuntu xenial universe'
sudo apt-get update -qq
sudo apt-get install --no-install-recommends \
- --target-release xenial libcmocka-dev
+ --target-release xenial
}

install_i586_deps()
@@ -40,7 +40,7 @@ install_i586_deps()
'deb http://archive.ubuntu.com/ubuntu xenial universe'
sudo apt-get update -qq
sudo apt-get install --no-install-recommends \
- --target-release xenial libcmocka-dev:i386
+ --target-release xenial
}

prepare_build()
diff --git a/tools/tests/Makefile.am b/tools/tests/Makefile.am
index dd4a249..ec96981 100644
--- a/tools/tests/Makefile.am
+++ b/tools/tests/Makefile.am
@@ -4,7 +4,7 @@
# Copyright (c) Siemens AG, 2017
#
# Authors:
-# Jan Kiszka <jan.k...@siemens.com>
+# Andreas Reichel <andreas.r...@siemens.com>
#
# This work is licensed under the terms of the GNU GPL, version 2. See
# the COPYING file in the top-level directory.
@@ -27,23 +27,9 @@ AM_CFLAGS = \

CLEANFILES =

-libebgenv-test.a: $(top_builddir)/libebgenv.a
- $(AM_V_GEN) $(OBJCOPY) --weaken $< $@
+test_api:
+ ln -sf /bin/true test_api

-CLEANFILES += libebgenv-test.a
-
-test_partitions_LDADD = \
- libebgenv-test.a \
- -lcmocka
-
-test_environment_LDADD = \
- libebgenv-test.a \
- -lcmocka
-
-test_api_LDADD = \
- libebgenv-test.a \
- -lcmocka
-
-check_PROGRAMS = test_partitions test_environment test_api
+check_PROGRAMS = test_api

TESTS = $(check_PROGRAMS)
diff --git a/tools/tests/test_api.c b/tools/tests/test_api.c
deleted file mode 100644
index d6b12e2..0000000
--- a/tools/tests/test_api.c
+++ /dev/null
@@ -1,315 +0,0 @@
-/*
- * EFI Boot Guard
- *
- * Copyright (c) Siemens AG, 2017
- *
- * Authors:
- * Andreas Reichel <andreas.r...@siemens.com>
- *
- * This work is licensed under the terms of the GNU GPL, version 2. See
- * the COPYING file in the top-level directory.
- */
-
-#include <stdlib.h>
-#include <stdarg.h>
-#include <stddef.h>
-#include <stdbool.h>
-#include <setjmp.h>
-#include <cmocka.h>
-#include <string.h>
-#include <error.h>
-#include "env_api.h"
-#include "ebgenv.h"
-
-static BGENV env = {0};
-static BGENV envupdate = {0};
-static BG_ENVDATA data = {0};
-static BG_ENVDATA dataupdate = {0};
-
-#define DEFAULT_WATCHDOG_TIMEOUT_SEC 30
-static int test_env_revision = 42;
-static char *test_env_revision_str = "42";
-static ebgenv_t e;
-
-bool bgenv_init()
-{
- return true;
-}
-
-bool bgenv_write(BGENV *env)
-{
- return mock_type(bool);
-}
-
-bool bgenv_close(BGENV *env_current)
-{
- return true;
-}
-
-BGENV *bgenv_open_by_index(uint32_t index)
-{
- if (index == 1) {
- return &envupdate;
- }
- return &env;
-}
-
-BGENV *bgenv_open_latest()
-{
- return mock_ptr_type(BGENV *);
-}
-
-BGENV *bgenv_open_oldest()
-{
- return mock_ptr_type(BGENV *);
-}
-
-static void test_api_openclose(void **state)
-{
- int ret;
-
- will_return(bgenv_open_latest, &env);
- ret = ebg_env_open_current(&e);
- assert_int_equal(ret, 0);
- will_return(bgenv_write, true);
- ret = ebg_env_close(&e);
- assert_int_equal(ret, 0);
-
- will_return(bgenv_open_latest, &env);
- ret = ebg_env_open_current(&e);
- assert_int_equal(ret, 0);
- will_return(bgenv_write, false);
- ret = ebg_env_close(&e);
- assert_int_equal(ret, EIO);
-
- will_return(bgenv_open_latest, NULL);
- ret = ebg_env_open_current(&e);
- assert_int_equal(ret, EIO);
- ret = ebg_env_close(&e);
- assert_int_equal(ret, EIO);
-
- (void)state;
-}
-
-static void test_api_accesscurrent(void **state)
-{
- int ret;
- char buffer[4096];
-
- will_return(bgenv_open_latest, &env);
- ret = ebg_env_open_current(&e);
- assert_int_equal(ret, 0);
- will_return(bgenv_write, true);
- ret = ebg_env_close(&e);
- assert_int_equal(ret, 0);
-
- ret = ebg_env_set(&e, "kernelfile", "vmlinuz");
- assert_int_equal(ret, -EPERM);
-
- will_return(bgenv_open_latest, &env);
- ret = ebg_env_open_current(&e);
- assert_int_equal(ret, 0);
-
- assert_int_equal(ebg_env_set(&e, "kernelfile", "vmlinuz"), 0);
- assert_int_equal(ebg_env_set(&e, "kernelparams", "root=/dev/sda"), 0);
- assert_int_equal(ebg_env_set(&e, "watchdog_timeout_sec", "abc"), -EINVAL);
- assert_int_equal(ebg_env_set(&e, "watchdog_timeout_sec", "0013"), 0);
- assert_int_equal(ebg_env_set(&e, "ustate", "1"), 0);
-
- will_return(bgenv_write, true);
- ret = ebg_env_close(&e);
- assert_int_equal(ret, 0);
-
- ret = ebg_env_get(&e, "kernelfile", buffer);
- assert_int_equal(ret, -EPERM);
-
- will_return(bgenv_open_latest, &env);
- ret = ebg_env_open_current(&e);
- assert_int_equal(ret, 0);
-
- assert_int_equal(ebg_env_get(&e, "kernelfile", buffer), 0);
- assert_string_equal(buffer, "vmlinuz");
- assert_int_equal(ebg_env_get(&e, "kernelparams", buffer), 0);
- assert_string_equal(buffer, "root=/dev/sda");
- assert_int_equal(ebg_env_get(&e, "watchdog_timeout_sec", buffer), 0);
- assert_string_equal(buffer, "13");
- assert_int_equal(ebg_env_get(&e, "ustate", buffer), 0);
- assert_string_equal(buffer, "1");
- assert_int_equal(ebg_env_get(&e, "revision", buffer), 0);
- assert_string_equal(buffer, test_env_revision_str);
-
- will_return(bgenv_write, true);
- ret = ebg_env_close(&e);
- assert_int_equal(ret, 0);
-
- (void)state;
-}
-
-static void test_api_update(void **state)
-{
- will_return(bgenv_open_latest, &env);
- will_return(bgenv_open_oldest, &envupdate);
- will_return(bgenv_open_latest, &env);
- assert_int_equal(ebg_env_create_new(&e), 0);
-
- assert_int_equal(envupdate.data->revision, test_env_revision + 1);
- assert_int_equal(envupdate.data->ustate, 1);
-
- assert_int_equal(ebg_env_set(&e, "ustate", "2"), 0);
- for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {
- will_return(bgenv_write, true);
- }
- assert_int_equal(ebg_env_setglobalstate(&e, 0), 0);
-
- if (ENV_NUM_CONFIG_PARTS == 1) {
- will_return(bgenv_open_latest, &envupdate);
- }
- assert_int_equal(ebg_env_set(&e, "revision", "0"), 0);
- assert_int_equal(ebg_env_set(&e, "ustate", "3"), 0);
- assert_int_equal(ebg_env_getglobalstate(&e), 3);
-
- will_return(bgenv_open_latest, &env);
- for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {
- will_return(bgenv_write, true);
- }
- assert_int_equal(ebg_env_setglobalstate(&e, 0), 0);
- assert_int_equal(ebg_env_getglobalstate(&e), 0);
-
- will_return(bgenv_write, true);
- assert_int_equal(ebg_env_close(&e), 0);
-
- (void)state;
-}
-
-static void test_api_uservars(void **state)
-{
- int ret;
- char *test_key = "NonsenseKey";
- char *test_key2 = "TestKey2";
-
- char *test_val = "AnyValue";
- char *test_val2 = "BnyVbluf";
- char *test_val3 = "TESTTESTTESTTEST";
- char *test_val4 = "abc";
- char buffer[ENV_MEM_USERVARS];
- uint32_t space_left;
-
- will_return(bgenv_open_latest, &env);
- ret = ebg_env_open_current(&e);
- assert_int_equal(ret, 0);
-
- assert_int_equal(ebg_env_user_free(&e), ENV_MEM_USERVARS);
-
- ret = ebg_env_set(&e, test_key, test_val);
- assert_int_equal(ret, 0);
-
- space_left = ENV_MEM_USERVARS - strlen(test_key) - 1
- - strlen(test_val) - 1 - sizeof(uint32_t)
- - strlen(USERVAR_TYPE_DEFAULT) - 1;
-
- assert_int_equal(ebg_env_user_free(&e), space_left);
-
- ret = ebg_env_get(&e, test_key, buffer);
- assert_int_equal(ret, 0);
- assert_string_equal(buffer, test_val);
-
- // replace value with same length value
- ret = ebg_env_set(&e, test_key, test_val2);
- assert_int_equal(ret, 0);
- assert_int_equal(ebg_env_user_free(&e), space_left);
-
- ret = ebg_env_get(&e, test_key, buffer);
- assert_int_equal(ret, 0);
- assert_string_equal(buffer, test_val2);
-
- // replace value with larger value
- ret = ebg_env_set(&e, test_key, test_val3);
- assert_int_equal(ret, 0);
-
- space_left = ENV_MEM_USERVARS - strlen(test_key) - 1
- - strlen(test_val3) - 1 - sizeof(uint32_t)
- - strlen(USERVAR_TYPE_DEFAULT) - 1;
-
- assert_int_equal(ebg_env_user_free(&e), space_left);
-
- // replace value with smaller value
- ret = ebg_env_set(&e, test_key, test_val4);
- assert_int_equal(ret, 0);
-
- space_left = ENV_MEM_USERVARS - strlen(test_key) - 1
- - strlen(test_val4) - 1 - sizeof(uint32_t)
- - strlen(USERVAR_TYPE_DEFAULT) - 1;
-
- assert_int_equal(ebg_env_user_free(&e), space_left);
-
- // add 2nd variable
- ret = ebg_env_set(&e, test_key2, test_val2);
- assert_int_equal(ret, 0);
-
- space_left = space_left - strlen(test_key2) - 1
- - strlen(test_val2) - 1 - sizeof(uint32_t)
- - strlen(USERVAR_TYPE_DEFAULT) - 1;
-
- assert_int_equal(ebg_env_user_free(&e), space_left);
-
- // retrieve both variables
- ret = ebg_env_get(&e, test_key2, buffer);
- assert_int_equal(ret, 0);
- assert_string_equal(buffer, test_val2);
- ret = ebg_env_get(&e, test_key, buffer);
- assert_int_equal(ret, 0);
- assert_string_equal(buffer, test_val4);
-
- // overwrite first variable
- ret = ebg_env_set(&e, test_key, test_val3);
- assert_int_equal(ret, 0);
-
- space_left = space_left + strlen(test_val4)
- - strlen(test_val3);
- assert_int_equal(ebg_env_user_free(&e), space_left);
-
- // retrieve both variables
- ret = ebg_env_get(&e, test_key2, buffer);
- assert_int_equal(ret, 0);
- assert_string_equal(buffer, test_val2);
- ret = ebg_env_get(&e, test_key, buffer);
- assert_int_equal(ret, 0);
- assert_string_equal(buffer, test_val3);
-
- void *dummymem = malloc(space_left);
-
- // test out of memory
- ret = ebg_env_set_ex(&e, "A", "B", dummymem, space_left);
- free(dummymem);
-
- assert_int_equal(ret, -ENOMEM);
-
- // test user data type
- ret = ebg_env_set_ex(&e, "A", "B", "C", 2);
- assert_int_equal(ret, 0);
-
- char type[2];
- char data[2];
-
- ret = ebg_env_get_ex(&e, "A", type, data, sizeof(data));
- assert_int_equal(ret, 0);
- assert_string_equal("B", type);
- assert_string_equal("C", data);
-
- (void)state;
-}
-
-int main(void)
-{
- const struct CMUnitTest tests[] = {
- cmocka_unit_test(test_api_openclose),
- cmocka_unit_test(test_api_accesscurrent),
- cmocka_unit_test(test_api_update),
- cmocka_unit_test(test_api_uservars)};
-
- env.data = &data;
- data.revision = test_env_revision;
- envupdate.data = &dataupdate;
-
- return cmocka_run_group_tests(tests, NULL, NULL);
-}
diff --git a/tools/tests/test_environment.c b/tools/tests/test_environment.c
deleted file mode 100644
index f4116da..0000000
--- a/tools/tests/test_environment.c
+++ /dev/null
@@ -1,119 +0,0 @@
-/*
- * EFI Boot Guard
- *
- * Copyright (c) Siemens AG, 2017
- *
- * Authors:
- * Andreas Reichel <andreas.r...@siemens.com>
- *
- * This work is licensed under the terms of the GNU GPL, version 2. See
- * the COPYING file in the top-level directory.
- */
-
-#include <stdlib.h>
-#include <stdarg.h>
-#include <stddef.h>
-#include <stdbool.h>
-#include <setjmp.h>
-#include <cmocka.h>
-#include <string.h>
-#include "env_api.h"
-#include "ebgenv.h"
-#include "test-interface.h"
-
-/* Mock functions from libparted */
-
-CONFIG_PART config_parts[ENV_NUM_CONFIG_PARTS];
-BG_ENVDATA oldenvs[ENV_NUM_CONFIG_PARTS];
-
-FILE test_file;
-
-int feof(FILE *f)
-{
- return 0;
-}
-
-FILE *fopen(const char *filename, const char *mode)
-{
- if (strcmp(filename, "/nobrain/BGENV.DAT") == 0) {
- return &test_file;
- }
- return NULL;
-}
-
-FILE *fopen64(const char *filename, const char *mode)
-{
- return fopen(filename, mode);
-}
-
-int fclose(FILE *handle)
-{
- return mock_type(int);
-}
-
-size_t fread(void *ptr, size_t size, size_t count, FILE *stream)
-{
- return mock_type(size_t);
-}
-
-size_t fwrite(const void *ptr, size_t size, size_t count, FILE *stream)
-{
- return mock_type(size_t);
-}
-
-static void test_configfile_read(void **state)
-{
- CONFIG_PART part;
- BG_ENVDATA env;
-
- part.devpath = "/dev/nobrain42";
- part.mountpoint = "/nobrain";
- part.not_mounted = false;
-
- will_return(fread, 0);
- will_return(fclose, 0);
- bool ret = read_env(&part, &env);
- assert_false(ret);
-
- will_return(fread, 1);
- will_return(fclose, 0);
- ret = read_env(&part, &env);
- assert_true(ret);
-
- will_return(fread, 1);
- will_return(fclose, -1);
- ret = read_env(&part, &env);
- assert_true(ret);
-
- (void)state;
-}
-
-static void test_configfile_write(void **state)
-{
- CONFIG_PART part;
- BG_ENVDATA env;
-
- part.devpath = "/dev/nobrain42";
- part.mountpoint = "/nobrain";
- part.not_mounted = false;
-
- will_return(fwrite, 1);
- will_return(fclose, 0);
- bool ret = write_env(&part, &env);
- assert_true(ret);
-
- will_return(fwrite, 0);
- will_return(fclose, 1);
- ret = write_env(&part, &env);
- assert_false(ret);
-
- (void)state;
-}
-
-int main(void)
-{
- const struct CMUnitTest tests[] = {
- cmocka_unit_test(test_configfile_read),
- cmocka_unit_test(test_configfile_write)};
- return cmocka_run_group_tests(tests, NULL, NULL);
-}
diff --git a/tools/tests/test_partitions.c b/tools/tests/test_partitions.c
deleted file mode 100644
index 734ec49..0000000
--- a/tools/tests/test_partitions.c
+++ /dev/null
@@ -1,154 +0,0 @@
-/*
- * EFI Boot Guard
- *
- * Copyright (c) Siemens AG, 2017
- *
- * Authors:
- * Andreas Reichel <andreas.r...@siemens.com>
- *
- * This work is licensed under the terms of the GNU GPL, version 2. See
- * the COPYING file in the top-level directory.
- */
-
-#include <stdlib.h>
-#include <stdarg.h>
-#include <stddef.h>
-#include <stdbool.h>
-#include <setjmp.h>
-#include <cmocka.h>
-#include "env_api.h"
-#include "ebgpart.h"
-#include "ebgenv.h"
-#include "test-interface.h"
-
-static PedDevice ped_devices[32] = {0};
-static int num_simulated_devices = 2;
-static int curr_ped_device = 0;
-static PedPartition ped_parts[32] = {0};
-static int num_simulated_partitions_per_disk = 2;
-static PedFileSystemType ped_fstypes[32] = {0};
-
-static const char *const fsname = "fat16";
-
-static char *fakemodel = "Mocked Disk Drive";
-static char *fakedevice = "/dev/nobrain";
-
-/* Mock functions from libparted */
-void ped_device_probe_all()
-{
- /* Setup the test data structure */
- for (int i = 0; i < 32; i++) {
- ped_devices[i].model = fakemodel;
- ped_devices[i].path = fakedevice;
- ped_devices[i].part_list = &ped_parts[0];
- }
-
- for (int i = 0; i < 32; i++) {
- ped_parts[i].fs_type = &ped_fstypes[i];
- ped_parts[i].num = i + 1;
-
- /* Unfortunately, the struct member to set is
- * 'const char * const'
- */
- long ptr = (long)fsname;
- memcpy((void *)&ped_fstypes[i].name, &ptr,
- sizeof(ped_fstypes[i].name));
- }
-}
-
-PedDevice *ped_device_get_next(const PedDevice *dev)
-{
- if (dev == NULL) {
- return &ped_devices[0];
- }
- for (int i = 0; i < num_simulated_devices - 1; i++) {
- if (dev == &ped_devices[i]) {
- return &ped_devices[i + 1];
- }
- }
- return NULL;
-}
-
-PedPartition *ped_disk_next_partition(const PedDisk *disk,
- const PedPartition *part)
-{
- if (disk == NULL) {
- return NULL;
- }
- if (part == NULL) {
- return &ped_parts[0];
- }
- for (int i = 0; i < num_simulated_partitions_per_disk - 1; i++) {
- if (part == &ped_parts[i]) {
- return &ped_parts[i + 1];
- }
- }
- return NULL;
-}
-
-bool mount_partition(CONFIG_PART *cfgpart)
-{
- return true;
-}
-
-bool probe_config_file(CONFIG_PART *cfgpart)
-{
- return mock_type(bool);
-}
-
-static void test_partition_count(void **state)
-{
- CONFIG_PART cfgparts[256];
- bool ret;
-
- num_simulated_devices = 1;
- num_simulated_partitions_per_disk = ENV_NUM_CONFIG_PARTS;
- for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {
- will_return(probe_config_file, true);
- }
- ret = probe_config_partitions(cfgparts);
- assert_true(ret);
-
- num_simulated_devices = ENV_NUM_CONFIG_PARTS;
- num_simulated_partitions_per_disk = 1;
- for (int i = 0; i < ENV_NUM_CONFIG_PARTS - 1; i++) {
- will_return(probe_config_file, true);
- }
- will_return(probe_config_file, true);
- ret = probe_config_partitions(cfgparts);
- assert_true(ret);
-
- (void)state;
-}
-
-static void test_config_file_existence(void **state)
-{
- CONFIG_PART cfgparts[256];
- bool ret;
-
- num_simulated_devices = 1;
- num_simulated_partitions_per_disk = ENV_NUM_CONFIG_PARTS;
- for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {
- will_return(probe_config_file, false);
- }
- ret = probe_config_partitions(cfgparts);
- assert_false(ret);
-
- for (int i = 0; i < ENV_NUM_CONFIG_PARTS - 1; i++) {
- will_return(probe_config_file, true);
- }
- will_return(probe_config_file, false);
-
- ret = probe_config_partitions(cfgparts);
- assert_false(ret);
-
- (void)state;
-}
-
-int main(void)
-{
- const struct CMUnitTest tests[] = {
- cmocka_unit_test(test_partition_count),
- cmocka_unit_test(test_config_file_existence)};
- return cmocka_run_group_tests(tests, NULL, NULL);
-}
--
2.14.2

Andreas J. Reichel

unread,
Oct 24, 2017, 8:27:40 AM10/24/17
to efibootg...@googlegroups.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>

Use libcheck as new unit testing framework.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---
.travis-build.sh | 4 ++--
configure.ac | 3 +++
docs/COMPILE.md | 4 ++--
3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/.travis-build.sh b/.travis-build.sh
index 470c710..6e534cb 100755
--- a/.travis-build.sh
+++ b/.travis-build.sh
@@ -29,7 +29,7 @@ install_native_deps()
'deb http://archive.ubuntu.com/ubuntu xenial universe'
sudo apt-get update -qq
sudo apt-get install --no-install-recommends \
- --target-release xenial
+ --target-release xenial check
}

install_i586_deps()
@@ -40,7 +40,7 @@ install_i586_deps()
'deb http://archive.ubuntu.com/ubuntu xenial universe'
sudo apt-get update -qq
sudo apt-get install --no-install-recommends \
- --target-release xenial
+ --target-release xenial check:i386
}

prepare_build()
diff --git a/configure.ac b/configure.ac
index d1dd082..7e8e7e9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -163,6 +163,9 @@ AC_ARG_WITH([mem-uservars],
])

AC_DEFINE_UNQUOTED([ENV_MEM_USERVARS], [${ENV_MEM_USERVARS}], [Reserved memory for user variables])
+
+PKG_PROG_PKG_CONFIG()
+PKG_CHECK_MODULES(LIBCHECK, check)
# ------------------------------------------------------------------------------
AC_CONFIG_FILES([
Makefile
diff --git a/docs/COMPILE.md b/docs/COMPILE.md
index 0429d81..e194601 100644
--- a/docs/COMPILE.md
+++ b/docs/COMPILE.md
@@ -5,13 +5,13 @@
### Arch Linux ###

```
-pacman -S gnu-efi-libs pciutils
+pacman -S gnu-efi-libs pciutils check
```

### Debian 8 ###

```
-apt-get install gnu-efi libpci-dev
+apt-get install gnu-efi libpci-dev check
```

## Compilation ##
--
2.14.2

Andreas J. Reichel

unread,
Oct 24, 2017, 8:27:41 AM10/24/17
to efibootg...@googlegroups.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>

Install needed packages into build environment.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---
.travis-build.sh | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/.travis-build.sh b/.travis-build.sh
index 6e534cb..a7e6beb 100755
--- a/.travis-build.sh
+++ b/.travis-build.sh
@@ -25,22 +25,14 @@ install_common_deps()

install_native_deps()
{
- sudo apt-add-repository \
- 'deb http://archive.ubuntu.com/ubuntu xenial universe'
- sudo apt-get update -qq
sudo apt-get install --no-install-recommends \
- --target-release xenial check
+ libz-dev check
}

install_i586_deps()
{
sudo apt-get install --no-install-recommends \
- libz-dev:i386
- sudo apt-add-repository \
- 'deb http://archive.ubuntu.com/ubuntu xenial universe'
- sudo apt-get update -qq
- sudo apt-get install --no-install-recommends \
- --target-release xenial check:i386
+ libz-dev:i386 check:i386
}

prepare_build()
@@ -86,14 +78,17 @@ case "$TARGET_EFFECTIVE" in
install_i586_deps
prepare_build
enter_build
+ export PKG_CONFIG_DIR=
+ export PKG_CONFIG_PATH=/usr/lib/i386-linux-gnu/pkgconfig
+ export PKG_CONFIG_LIBDIR=/usr/lib/i386-linux-gnu
../configure --with-gnuefi-lib-dir=/usr/lib32 CFLAGS=-m32 \
host_alias=i586-linux
exec make check
;;

cppcheck)
- install_common_deps
- install_native_deps
+ install_common_deps
+ install_native_deps
echo "Building and installing cppcheck..."
if ! install_cppcheck >cppcheck_build.log 2>&1
then
@@ -103,10 +98,15 @@ case "$TARGET_EFFECTIVE" in
prepare_build
./configure

+ # Ignore directories as they are not part of the project
+ ignore=""
+ ignore+=" -i fff"
+
suppress=""
# Justified suppressions:
- # Not part of the project:
- suppress+=" --suppress=variableScope:/usr/include/bits/stdlib-bsearch.h"
+ # Does not belong to the project
+ suppress+=" --suppress=*:/usr/include/*"
+ suppress+=" --suppress=*:/usr/include/bits/*"
# Function 'efi_main' is called by efi:
suppress+=" --suppress=unusedFunction:main.c"
# Some functions are defined for API only
@@ -137,7 +137,7 @@ case "$TARGET_EFFECTIVE" in
# Exit code '1' is returned if arguments are not valid or if no input
# files are provided. Compare 'cppcheck --help'.
exec cppcheck -f -q --error-exitcode=2 \
- $enable $suppress $cpp_conf $includes .
+ $enable $suppress $ignore $cpp_conf $includes .
;;
coverity_prepare)
install_common_deps
--
2.14.2

Andreas J. Reichel

unread,
Oct 24, 2017, 8:27:41 AM10/24/17
to efibootg...@googlegroups.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>

Use function faking framework for mocking in unit tests.
Integrate it into the project as git submodule.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---
.gitmodules | 3 +++
fff | 1 +
tools/tests/Makefile.am | 3 ++-
3 files changed, 6 insertions(+), 1 deletion(-)
create mode 100644 .gitmodules
create mode 160000 fff

diff --git a/.gitmodules b/.gitmodules
new file mode 100644
index 0000000..971d7e7
--- /dev/null
+++ b/.gitmodules
@@ -0,0 +1,3 @@
+[submodule "fff"]
+ path = fff
+ url = https://github.com/meekrosoft/fff
diff --git a/fff b/fff
new file mode 160000
index 0000000..719dd8b
--- /dev/null
+++ b/fff
@@ -0,0 +1 @@
+Subproject commit 719dd8b6a39a34d0dbf71eeccd4bcd8c5e84c9b4
diff --git a/tools/tests/Makefile.am b/tools/tests/Makefile.am
index ec96981..30dbb85 100644
--- a/tools/tests/Makefile.am
+++ b/tools/tests/Makefile.am
@@ -15,7 +15,8 @@ OBJCOPY ?= objcopy
AM_CPPFLAGS = \
-I$(top_srcdir)/include \
-I$(top_srcdir)/swupdate-adapter \
- -I$(top_srcdir)/tools
+ -I$(top_srcdir)/tools \
+ -I$(top_srcdir)/fff

AM_CFLAGS = \
-Wno-unused-parameter \
--
2.14.2

Andreas J. Reichel

unread,
Oct 24, 2017, 8:27:43 AM10/24/17
to efibootg...@googlegroups.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>

For unit test support, code must be more modular.
This also avoids inline functions and static functions where possible.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---
Makefile.am | 4 +-
env/env_api_fat.c | 224 ++--------------------------------------
env/env_config_file.c | 90 ++++++++++++++++
env/env_config_partitions.c | 83 +++++++++++++++
env/env_disk_utils.c | 99 ++++++++++++++++++
include/env_config_file.h | 20 ++++
include/env_config_partitions.h | 18 ++++
include/env_disk_utils.h | 20 ++++
8 files changed, 339 insertions(+), 219 deletions(-)
create mode 100644 env/env_config_file.c
create mode 100644 env/env_config_partitions.c
create mode 100644 env/env_disk_utils.c
create mode 100644 include/env_config_file.h
create mode 100644 include/env_config_partitions.h
create mode 100644 include/env_disk_utils.h

diff --git a/Makefile.am b/Makefile.am
index 51e2658..1e0dc1c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -72,6 +72,9 @@ lib_LIBRARIES = libebgenv.a
libebgenv_a_SOURCES = \
env/@env_api_file@.c \
env/env_api.c \
+ env/env_config_file.c \
+ env/env_config_partitions.c \
+ env/env_disk_utils.c \
env/uservars.c \
tools/ebgpart.c

@@ -91,7 +94,6 @@ pkginclude_HEADERS = \
bin_PROGRAMS = bg_setenv

bg_setenv_SOURCES = \
- tools/ebgpart.c \
tools/bg_setenv.c

bg_setenv_CFLAGS = \
diff --git a/env/env_api_fat.c b/env/env_api_fat.c
index 653d3f8..a344fcb 100644
--- a/env/env_api_fat.c
+++ b/env/env_api_fat.c
@@ -11,11 +11,12 @@
*/

#include "env_api.h"
-#include "ebgpart.h"
+#include "env_disk_utils.h"
+#include "env_config_partitions.h"
+#include "env_config_file.h"
#include "uservars.h"
#include "test-interface.h"
-
-const char *tmp_mnt_dir = "/tmp/mnt-XXXXXX";
+#include "ebgpart.h"

bool bgenv_verbosity = false;

@@ -46,219 +47,6 @@ void bgenv_be_verbose(bool v)
ebgpart_beverbose(v);
}

-static char *get_mountpoint(char *devpath)
-{
- struct mntent *part = NULL;
- FILE *mtab = NULL;
-
- if ((mtab = setmntent("/proc/mounts", "r")) == NULL)
- return NULL;
-
- while ((part = getmntent(mtab)) != NULL) {
- if ((part->mnt_fsname != NULL) &&
- (strcmp(part->mnt_fsname, devpath)) == 0) {
- char *mntpoint;
-
- if (!(mntpoint =
- malloc(strlen(part->mnt_dir) + 1))) {
- break;
- };
- strncpy(mntpoint, part->mnt_dir,
- strlen(part->mnt_dir) + 1);
- return mntpoint;
- }
- }
- endmntent(mtab);
-
- return NULL;
-}
-
-__attribute__((noinline))
-bool mount_partition(CONFIG_PART *cfgpart)
-{
- char tmpdir_template[256];
- char *mountpoint;
- (void)snprintf(tmpdir_template, 256, "%s", tmp_mnt_dir);
- if (!cfgpart) {
- return false;
- }
- if (!cfgpart->devpath) {
- return false;
- }
- if (!(mountpoint = mkdtemp(tmpdir_template))) {
- VERBOSE(stderr, "Error creating temporary mount point.\n");
- return false;
- }
- if (mount(cfgpart->devpath, mountpoint, "vfat", 0, "")) {
- VERBOSE(stderr, "Error mounting to temporary mount point.\n");
- if (rmdir(tmpdir_template)) {
- VERBOSE(stderr,
- "Error deleting temporary directory.\n");
- }
- return false;
- }
- cfgpart->mountpoint = (char *)malloc(strlen(mountpoint) + 1);
- if (!cfgpart->mountpoint) {
- VERBOSE(stderr, "Error, out of memory.\n");
- return false;
- }
- strncpy(cfgpart->mountpoint, mountpoint, strlen(mountpoint) + 1);
- return true;
-}
-
-static void unmount_partition(CONFIG_PART *cfgpart)
-{
- if (!cfgpart) {
- return;
- }
- if (!cfgpart->mountpoint) {
- return;
- }
- if (umount(cfgpart->mountpoint)) {
- VERBOSE(stderr, "Error unmounting temporary mountpoint %s.\n",
- cfgpart->mountpoint);
- }
- if (rmdir(cfgpart->mountpoint)) {
- VERBOSE(stderr, "Error deleting temporary directory %s.\n",
- cfgpart->mountpoint);
- }
- free(cfgpart->mountpoint);
- cfgpart->mountpoint = NULL;
-}
-
-static FILE *open_config_file(CONFIG_PART *cfgpart, char *mode)
-{
- char *configfilepath;
- configfilepath = (char *)malloc(strlen(FAT_ENV_FILENAME) +
- strlen(cfgpart->mountpoint) + 2);
- if (!configfilepath) {
- return NULL;
- }
- strncpy(configfilepath, cfgpart->mountpoint,
- strlen(cfgpart->mountpoint) + 1);
- strncat(configfilepath, "/", 1);
- strncat(configfilepath, FAT_ENV_FILENAME, strlen(FAT_ENV_FILENAME));
- VERBOSE(stdout, "Probing config file at %s.\n", configfilepath);
- FILE *config = fopen(configfilepath, mode);
- free(configfilepath);
- return config;
-}
-
-__attribute__((noinline))
-bool probe_config_file(CONFIG_PART *cfgpart)
-{
- bool do_unmount = false;
- if (!cfgpart) {
- return false;
- }
- printf_debug("Checking device: %s\n", cfgpart->devpath);
- if (!(cfgpart->mountpoint = get_mountpoint(cfgpart->devpath))) {
- /* partition is not mounted */
- cfgpart->not_mounted = true;
- VERBOSE(stdout, "Partition %s is not mounted.\n",
- cfgpart->devpath);
- if (!mount_partition(cfgpart)) {
- return false;
- }
- do_unmount = true;
- } else {
- cfgpart->not_mounted = false;
- }
-
- if (cfgpart->mountpoint) {
- /* partition is mounted to mountpoint, either before or by this
- * program */
- VERBOSE(stdout, "Partition %s is mounted to %s.\n",
- cfgpart->devpath, cfgpart->mountpoint);
- bool result = false;
- FILE *config;
- if (!(config = open_config_file(cfgpart, "rb"))) {
- printf_debug(
- "Could not open config file on partition %s.\n",
- FAT_ENV_FILENAME);
- } else {
- result = true;
- if (fclose(config)) {
- VERBOSE(stderr, "Error closing config file on "
- "partition %s.\n",
- cfgpart->devpath);
- }
- }
- if (do_unmount) {
- unmount_partition(cfgpart);
- }
- return result;
- }
- return false;
-}
-
-bool probe_config_partitions(CONFIG_PART *cfgpart)
-{
- PedDevice *dev = NULL;
- char devpath[4096];
- int count = 0;
-
- if (!cfgpart) {
- return false;
- }
-
- ped_device_probe_all();
-
- while ((dev = ped_device_get_next(dev))) {
- printf_debug("Device: %s\n", dev->model);
- PedDisk *pd = ped_disk_new(dev);
- if (!pd) {
- continue;
- }
- PedPartition *part = pd->part_list;
- while (part) {
- if (!part->fs_type || !part->fs_type->name ||
- (strcmp(part->fs_type->name, "fat12") != 0 &&
- strcmp(part->fs_type->name, "fat16") != 0 &&
- strcmp(part->fs_type->name, "fat32") != 0)) {
- part = ped_disk_next_partition(pd, part);
- continue;
- }
- if (strncmp("/dev/mmcblk", dev->path, 11) == 0) {
- (void)snprintf(devpath, 4096, "%sp%u",
- dev->path, part->num);
- } else {
- (void)snprintf(devpath, 4096, "%s%u",
- dev->path, part->num);
- }
- if (!cfgpart[count].devpath) {
- cfgpart[count].devpath =
- malloc(strlen(devpath) + 1);
- if (!cfgpart[count].devpath) {
- VERBOSE(stderr, "Out of memory.");
- return false;
- }
- }
- strncpy(cfgpart[count].devpath, devpath,
- strlen(devpath) + 1);
- if (probe_config_file(&cfgpart[count])) {
- printf_debug("%s", "Environment file found.\n");
- if (count >= ENV_NUM_CONFIG_PARTS) {
- VERBOSE(stderr, "Error, there are "
- "more than %d config "
- "partitions.\n",
- ENV_NUM_CONFIG_PARTS);
- return false;
- }
- count++;
- }
- part = ped_disk_next_partition(pd, part);
- }
- }
- if (count < ENV_NUM_CONFIG_PARTS) {
- VERBOSE(stderr,
- "Error, less than %d config partitions exist.\n",
- ENV_NUM_CONFIG_PARTS);
- return false;
- }
- return true;
-}
-
bool read_env(CONFIG_PART *part, BG_ENVDATA *env)
{
if (!part) {
@@ -286,7 +74,7 @@ bool read_env(CONFIG_PART *part, BG_ENVDATA *env)
}
result = false;
}
- if (fclose(config)) {
+ if (close_config_file(config)) {
VERBOSE(stderr,
"Error closing environment file after reading.\n");
};
@@ -321,7 +109,7 @@ bool write_env(CONFIG_PART *part, BG_ENVDATA *env)
part->devpath);
result = false;
}
- if (fclose(config)) {
+ if (close_config_file(config)) {
VERBOSE(stderr,
"Error closing environment file after writing.\n");
result = false;
diff --git a/env/env_config_file.c b/env/env_config_file.c
new file mode 100644
index 0000000..3a802f2
--- /dev/null
+++ b/env/env_config_file.c
@@ -0,0 +1,90 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Andreas Reichel <andreas.r...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#include <stdlib.h>
+#include <stdio.h>
+#include "env_api.h"
+#include "env_disk_utils.h"
+#include "env_config_file.h"
+
+FILE *open_config_file(CONFIG_PART *cfgpart, char *mode)
+{
+ char *configfilepath;
+ configfilepath = (char *)malloc(strlen(FAT_ENV_FILENAME) +
+ strlen(cfgpart->mountpoint) + 2);
+ if (!configfilepath) {
+ return NULL;
+ }
+ strncpy(configfilepath, cfgpart->mountpoint,
+ strlen(cfgpart->mountpoint) + 1);
+ strncat(configfilepath, "/", 1);
+ strncat(configfilepath, FAT_ENV_FILENAME, strlen(FAT_ENV_FILENAME));
+ VERBOSE(stdout, "Probing config file at %s.\n", configfilepath);
+ FILE *config = fopen(configfilepath, mode);
+ free(configfilepath);
+ return config;
+}
+
+int close_config_file(FILE *config_file_handle)
+{
+ if (config_file_handle)
+ {
+ return fclose(config_file_handle);
+ }
+}
+
+bool probe_config_file(CONFIG_PART *cfgpart)
+{
+ bool do_unmount = false;
+ if (!cfgpart) {
+ return false;
+ }
+ printf_debug("Checking device: %s\n", cfgpart->devpath);
+ if (!(cfgpart->mountpoint = get_mountpoint(cfgpart->devpath))) {
+ /* partition is not mounted */
+ cfgpart->not_mounted = true;
+ VERBOSE(stdout, "Partition %s is not mounted.\n",
+ cfgpart->devpath);
+ if (!mount_partition(cfgpart)) {
+ return false;
+ }
+ do_unmount = true;
+ } else {
+ cfgpart->not_mounted = false;
+ }
+
+ if (cfgpart->mountpoint) {
+ /* partition is mounted to mountpoint, either before or by this
+ * program */
+ VERBOSE(stdout, "Partition %s is mounted to %s.\n",
+ cfgpart->devpath, cfgpart->mountpoint);
+ bool result = false;
+ FILE *config;
+ if (!(config = open_config_file(cfgpart, "rb"))) {
+ printf_debug(
+ "Could not open config file on partition %s.\n",
+ FAT_ENV_FILENAME);
+ } else {
+ result = true;
+ if (fclose(config)) {
+ VERBOSE(stderr, "Error closing config file on "
+ "partition %s.\n",
+ cfgpart->devpath);
+ }
+ }
+ if (do_unmount) {
+ unmount_partition(cfgpart);
+ }
+ return result;
+ }
+ return false;
+}
diff --git a/env/env_config_partitions.c b/env/env_config_partitions.c
new file mode 100644
index 0000000..b4ffc6c
--- /dev/null
+++ b/env/env_config_partitions.c
@@ -0,0 +1,83 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Andreas Reichel <andreas.r...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#include "env_api.h"
+#include "ebgpart.h"
+#include "env_config_partitions.h"
+#include "env_config_file.h"
+
+bool probe_config_partitions(CONFIG_PART *cfgpart)
+{
+ PedDevice *dev = NULL;
+ char devpath[4096];
+ int count = 0;
+
+ if (!cfgpart) {
+ return false;
+ }
+
+ ped_device_probe_all();
+
+ while ((dev = ped_device_get_next(dev))) {
+ printf_debug("Device: %s\n", dev->model);
+ PedDisk *pd = ped_disk_new(dev);
+ if (!pd) {
+ continue;
+ }
+ PedPartition *part = pd->part_list;
+ while (part) {
+ if (!part->fs_type || !part->fs_type->name ||
+ (strcmp(part->fs_type->name, "fat12") != 0 &&
+ strcmp(part->fs_type->name, "fat16") != 0 &&
+ strcmp(part->fs_type->name, "fat32") != 0)) {
+ part = ped_disk_next_partition(pd, part);
+ continue;
+ }
+ if (strncmp("/dev/mmcblk", dev->path, 11) == 0) {
+ (void)snprintf(devpath, 4096, "%sp%u",
+ dev->path, part->num);
+ } else {
+ (void)snprintf(devpath, 4096, "%s%u",
+ dev->path, part->num);
+ }
+ if (!cfgpart[count].devpath) {
+ cfgpart[count].devpath =
+ malloc(strlen(devpath) + 1);
+ if (!cfgpart[count].devpath) {
+ VERBOSE(stderr, "Out of memory.");
+ return false;
+ }
+ }
+ strncpy(cfgpart[count].devpath, devpath,
+ strlen(devpath) + 1);
+ if (probe_config_file(&cfgpart[count])) {
+ printf_debug("%s", "Environment file found.\n");
+ if (count >= ENV_NUM_CONFIG_PARTS) {
+ VERBOSE(stderr, "Error, there are "
+ "more than %d config "
+ "partitions.\n",
+ ENV_NUM_CONFIG_PARTS);
+ return false;
+ }
+ count++;
+ }
+ part = ped_disk_next_partition(pd, part);
+ }
+ }
+ if (count < ENV_NUM_CONFIG_PARTS) {
+ VERBOSE(stderr,
+ "Error, less than %d config partitions exist.\n",
+ ENV_NUM_CONFIG_PARTS);
+ return false;
+ }
+ return true;
+}
diff --git a/env/env_disk_utils.c b/env/env_disk_utils.c
new file mode 100644
index 0000000..344cd59
--- /dev/null
+++ b/env/env_disk_utils.c
@@ -0,0 +1,99 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Andreas Reichel <andreas.r...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#include <stdlib.h>
+#include <mntent.h>
+#include <string.h>
+#include "env_api.h"
+#include "env_disk_utils.h"
+
+const char *tmp_mnt_dir = "/tmp/mnt-XXXXXX";
+
+char *get_mountpoint(char *devpath)
+{
+ struct mntent *part = NULL;
+ FILE *mtab = NULL;
+
+ if ((mtab = setmntent("/proc/mounts", "r")) == NULL)
+ return NULL;
+
+ while ((part = getmntent(mtab)) != NULL) {
+ if ((part->mnt_fsname != NULL) &&
+ (strcmp(part->mnt_fsname, devpath)) == 0) {
+ char *mntpoint;
+
+ if (!(mntpoint =
+ malloc(strlen(part->mnt_dir) + 1))) {
+ break;
+ };
+ strncpy(mntpoint, part->mnt_dir,
+ strlen(part->mnt_dir) + 1);
+ return mntpoint;
+ }
+ }
+ endmntent(mtab);
+
+ return NULL;
+}
+
+bool mount_partition(CONFIG_PART *cfgpart)
+{
+ char tmpdir_template[256];
+ char *mountpoint;
+ (void)snprintf(tmpdir_template, 256, "%s", tmp_mnt_dir);
+ if (!cfgpart) {
+ return false;
+ }
+ if (!cfgpart->devpath) {
+ return false;
+ }
+ if (!(mountpoint = mkdtemp(tmpdir_template))) {
+ VERBOSE(stderr, "Error creating temporary mount point.\n");
+ return false;
+ }
+ if (mount(cfgpart->devpath, mountpoint, "vfat", 0, "")) {
+ VERBOSE(stderr, "Error mounting to temporary mount point.\n");
+ if (rmdir(tmpdir_template)) {
+ VERBOSE(stderr,
+ "Error deleting temporary directory.\n");
+ }
+ return false;
+ }
+ cfgpart->mountpoint = (char *)malloc(strlen(mountpoint) + 1);
+ if (!cfgpart->mountpoint) {
+ VERBOSE(stderr, "Error, out of memory.\n");
+ return false;
+ }
+ strncpy(cfgpart->mountpoint, mountpoint, strlen(mountpoint) + 1);
+ return true;
+}
+
+void unmount_partition(CONFIG_PART *cfgpart)
+{
+ if (!cfgpart) {
+ return;
+ }
+ if (!cfgpart->mountpoint) {
+ return;
+ }
+ if (umount(cfgpart->mountpoint)) {
+ VERBOSE(stderr, "Error unmounting temporary mountpoint %s.\n",
+ cfgpart->mountpoint);
+ }
+ if (rmdir(cfgpart->mountpoint)) {
+ VERBOSE(stderr, "Error deleting temporary directory %s.\n",
+ cfgpart->mountpoint);
+ }
+ free(cfgpart->mountpoint);
+ cfgpart->mountpoint = NULL;
+}
+
diff --git a/include/env_config_file.h b/include/env_config_file.h
new file mode 100644
index 0000000..b172142
--- /dev/null
+++ b/include/env_config_file.h
@@ -0,0 +1,20 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Andreas Reichel <andreas.r...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#ifndef __ENV_CONFIG_FILE_H__
+#define __ENV_CONFIG_FILE_H__
+
+FILE *open_config_file(CONFIG_PART *cfgpart, char *mode);
+int close_config_file(FILE *config_file_handle);
+bool probe_config_file(CONFIG_PART *cfgpart);
+
+#endif // __ENV_CONFIG_FILE_H__
diff --git a/include/env_config_partitions.h b/include/env_config_partitions.h
new file mode 100644
index 0000000..01f8d94
--- /dev/null
+++ b/include/env_config_partitions.h
@@ -0,0 +1,18 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Andreas Reichel <andreas.r...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#ifndef __ENV_CONFIG_PARTITIONS_H__
+#define __ENV_CONFIG_PARTITIONS_H__
+
+bool probe_config_partitions(CONFIG_PART *cfgpart);
+
+#endif // __ENV_CONFIG_PARTITIONS_H__
diff --git a/include/env_disk_utils.h b/include/env_disk_utils.h
new file mode 100644
index 0000000..9ceb019
--- /dev/null
+++ b/include/env_disk_utils.h
@@ -0,0 +1,20 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Andreas Reichel <andreas.r...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#ifndef __ENV_DISK_UTILS_H__
+#define __ENV_DISK_UTILS_H__
+
+char *get_mountpoint(char *devpath);
+bool mount_partition(CONFIG_PART *cfgpart);
+void unmount_partition(CONFIG_PART *cfgpart);
+
+#endif // __ENV_DISK_UTILS_H__
--
2.14.2

Andreas J. Reichel

unread,
Oct 24, 2017, 8:27:44 AM10/24/17
to efibootg...@googlegroups.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>

The new testcode initially provides tests for
* internal userspace API core functions
* external userspace API functions (libebgenv.a)

They are devided into the following test-suits:

* test_bgenv_init_retval.c, where partition probing must be
faked and disks must be simualted.
* test_probe_config_file.c, where the existence of environment
data is faked for simulated config partitions
* test_ebgenv_api_internal.c, core functions for userspace API
* test_ebgenv_api.c, library API functions for userspace

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---
.travis-build.sh | 3 +
env/env_api_fat.c | 6 +-
include/test-interface.h | 4 +
tools/tests/Makefile.am | 56 ++-
tools/tests/fake_devices.c | 94 +++++
tools/tests/fake_devices.h | 28 ++
tools/tests/test_bgenv_init_retval.c | 100 +++++
tools/tests/test_ebgenv_api.c | 641 +++++++++++++++++++++++++++++
tools/tests/test_ebgenv_api_internal.c | 416 +++++++++++++++++++
tools/tests/test_main.c | 34 ++
tools/tests/test_probe_config_file.c | 202 +++++++++
tools/tests/test_probe_config_partitions.c | 99 +++++
12 files changed, 1673 insertions(+), 10 deletions(-)
create mode 100644 tools/tests/fake_devices.c
create mode 100644 tools/tests/fake_devices.h
create mode 100644 tools/tests/test_bgenv_init_retval.c
create mode 100644 tools/tests/test_ebgenv_api.c
create mode 100644 tools/tests/test_ebgenv_api_internal.c
create mode 100644 tools/tests/test_main.c
create mode 100644 tools/tests/test_probe_config_file.c
create mode 100644 tools/tests/test_probe_config_partitions.c

diff --git a/.travis-build.sh b/.travis-build.sh
index a7e6beb..d977385 100755
--- a/.travis-build.sh
+++ b/.travis-build.sh
@@ -115,6 +115,9 @@ case "$TARGET_EFFECTIVE" in
suppress+=" --suppress=unusedFunction:env/fatvars.c"
suppress+=" --suppress=unusedFunction:tools/tests/test_environment.c"
suppress+=" --suppress=unusedFunction:env/env_api_fat.c"
+ # Some functions are used by linker wrapping
+ suppress+=" --suppress=unusedFunction:tools/tests/test_probe_config_file.c"
+ suppress+=" --suppress=unusedFunction:tools/tests/test_ebgenv_api.c"
# EFI uses void* as ImageBase needed for further calculations
suppress+=" --suppress=arithOperationsOnVoidPointer:main.c"

diff --git a/env/env_api_fat.c b/env/env_api_fat.c
index a344fcb..8eb1454 100644
--- a/env/env_api_fat.c
+++ b/env/env_api_fat.c
@@ -20,7 +20,7 @@

bool bgenv_verbosity = false;

-static EBGENVKEY bgenv_str2enum(char *key)
+EBGENVKEY bgenv_str2enum(char *key)
{
if (strncmp(key, "kernelfile", strlen("kernelfile") + 1) == 0) {
return EBGENV_KERNELFILE;
@@ -120,8 +120,8 @@ bool write_env(CONFIG_PART *part, BG_ENVDATA *env)
return result;
}

-static CONFIG_PART config_parts[ENV_NUM_CONFIG_PARTS];
-static BG_ENVDATA envdata[ENV_NUM_CONFIG_PARTS];
+CONFIG_PART config_parts[ENV_NUM_CONFIG_PARTS];
+BG_ENVDATA envdata[ENV_NUM_CONFIG_PARTS];

bool bgenv_init()
{
diff --git a/include/test-interface.h b/include/test-interface.h
index 36b0a62..c220d14 100644
--- a/include/test-interface.h
+++ b/include/test-interface.h
@@ -15,11 +15,15 @@

#include "env_api.h"

+extern CONFIG_PART config_parts[ENV_NUM_CONFIG_PARTS];
+extern BG_ENVDATA envdata[ENV_NUM_CONFIG_PARTS];
+
bool read_env(CONFIG_PART *part, BG_ENVDATA *env);
bool write_env(CONFIG_PART *part, BG_ENVDATA *env);

bool probe_config_file(CONFIG_PART *cfgpart);
bool probe_config_partitions(CONFIG_PART *cfgparts);
bool mount_partition(CONFIG_PART *cfgpart);
+EBGENVKEY bgenv_str2enum(char *key);

#endif // __TEST_INTERFACE_H__
diff --git a/tools/tests/Makefile.am b/tools/tests/Makefile.am
index 30dbb85..fd49e06 100644
--- a/tools/tests/Makefile.am
+++ b/tools/tests/Makefile.am
@@ -12,13 +12,12 @@

OBJCOPY ?= objcopy

-AM_CPPFLAGS = \
+AM_CFLAGS = \
-I$(top_srcdir)/include \
-I$(top_srcdir)/swupdate-adapter \
-I$(top_srcdir)/tools \
- -I$(top_srcdir)/fff
-
-AM_CFLAGS = \
+ -I$(top_srcdir)/tools/tests \
+ -I$(top_srcdir)/fff \
-Wno-unused-parameter \
-Wmissing-prototypes \
-fshort-wchar \
@@ -26,11 +25,54 @@ AM_CFLAGS = \
-D_GNU_SOURCE \
-g

+libtest_env_api_fat_a_SRC = \
+ ../../env/env_api.c \
+ ../../env/env_api_fat.c \
+ ../../tools/ebgpart.c \
+ ../../env/env_config_file.c \
+ ../../env/env_config_partitions.c \
+ ../../env/env_disk_utils.c \
+ ../../env/uservars.c
+
CLEANFILES =

-test_api:
- ln -sf /bin/true test_api
+noinst_LIBRARIES = libtest_env_api_fat.a
+libtest_env_api_fat_a_SOURCES = $(libtest_env_api_fat_a_SRC)
+
+libenvapi_testlib_fat.a: libtest_env_api_fat.a
+ $(OBJCOPY) --weaken $^ $@
+
+check_PROGRAMS = test_bgenv_init_retval \
+ test_probe_config_partitions \
+ test_probe_config_file \
+ test_ebgenv_api_internal \
+ test_ebgenv_api
+
+FAT_TESTLIB=libenvapi_testlib_fat.a
+
+SRC_TEST_COMMON=test_main.c
+
+test_bgenv_init_retval_CFLAGS = $(AM_CFLAGS)
+test_bgenv_init_retval_SOURCES = test_bgenv_init_retval.c $(SRC_TEST_COMMON)
+test_bgenv_init_retval_LDADD = $(FAT_TESTLIB) $(LIBCHECK_LIBS)
+
+test_probe_config_partitions_CFLAGS = $(AM_CFLAGS)
+test_probe_config_partitions_SOURCES = test_probe_config_partitions.c \
+ fake_devices.c \
+ $(SRC_TEST_COMMON)
+test_probe_config_partitions_LDADD = $(FAT_TESTLIB) $(LIBCHECK_LIBS)
+
+test_probe_config_file_CFLAGS = $(AM_CFLAGS) -Wl,--wrap=probe_config_file
+test_probe_config_file_SOURCES = test_probe_config_file.c fake_devices.c \
+ $(SRC_TEST_COMMON)
+test_probe_config_file_LDADD = $(FAT_TESTLIB) $(LIBCHECK_LIBS)
+
+test_ebgenv_api_internal_CFLAGS = $(AM_CFLAGS)
+test_ebgenv_api_internal_SOURCES = test_ebgenv_api_internal.c $(SRC_TEST_COMMON)
+test_ebgenv_api_internal_LDADD = $(FAT_TESTLIB) $(LIBCHECK_LIBS)

-check_PROGRAMS = test_api
+test_ebgenv_api_CFLAGS = $(AM_CFLAGS) -Wl,--wrap=bgenv_set -Wl,--wrap=bgenv_get
+test_ebgenv_api_SOURCES = test_ebgenv_api.c $(SRC_TEST_COMMON)
+test_ebgenv_api_LDADD = $(FAT_TESTLIB) $(LIBCHECK_LIBS)

TESTS = $(check_PROGRAMS)
diff --git a/tools/tests/fake_devices.c b/tools/tests/fake_devices.c
new file mode 100644
index 0000000..58a47bd
--- /dev/null
+++ b/tools/tests/fake_devices.c
@@ -0,0 +1,94 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Andreas Reichel <andreas.r...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#include <stdlib.h>
+#include <env_api.h>
+#include <env_config_file.h>
+#include <env_config_partitions.h>
+#include <fake_devices.h>
+
+PedDevice *fake_devices;
+int num_fake_devices;
+
+void allocate_fake_devices(int n)
+{
+ fake_devices = (PedDevice *)malloc(n * sizeof(PedDevice));
+ for (char i = 0; i < n; i++) {
+ asprintf(&fake_devices[i].model, "%s", "Fake Device");
+ asprintf(&fake_devices[i].path, "/dev/nobrain_%c", 'a' + i);
+ fake_devices[i].part_list = NULL;
+ fake_devices[i].next = NULL;
+ }
+ num_fake_devices = n;
+ for (char i = n - 1; i > 0; i--) {
+ fake_devices[i-1].next = &fake_devices[i];
+ }
+}
+
+void add_fake_partition(int devnum)
+{
+ PedPartition **pp = &fake_devices[devnum].part_list;
+
+ int16_t num = 0;
+ while (*pp) {
+ pp = &(*pp)->next;
+ num++;
+ }
+ *pp = (PedPartition *)malloc(sizeof(PedPartition));
+ (*pp)->num = num;
+ (*pp)->fs_type = (PedFileSystemType *)malloc(sizeof(PedFileSystemType));
+ asprintf(&(*pp)->fs_type->name, "%s", "fat16");
+ (*pp)->next = NULL;
+}
+
+void remove_fake_partitions(int n)
+{
+ PedPartition *pp = fake_devices[n].part_list;
+ PedPartition *next;
+ while(pp) {
+ next = pp->next;
+ if (!pp->fs_type)
+ goto skip_fstype;
+ if (pp->fs_type->name)
+ free(pp->fs_type->name);
+ free(pp->fs_type);
+skip_fstype:
+ free(pp);
+ pp = next;
+ }
+}
+
+void free_fake_devices()
+{
+ if (!fake_devices) {
+ return;
+ }
+
+ for (int i = 0; i < num_fake_devices; i++) {
+ if (fake_devices[i].model)
+ free(fake_devices[i].model);
+ if (fake_devices[i].path)
+ free(fake_devices[i].path);
+ if (fake_devices[i].part_list)
+ remove_fake_partitions(i);
+ }
+
+ free(fake_devices);
+}
+
+PedDevice *ped_device_get_next_custom_fake(const PedDevice *dev)
+{
+ if (!dev)
+ return fake_devices;
+
+ return dev->next;
+}
diff --git a/tools/tests/fake_devices.h b/tools/tests/fake_devices.h
new file mode 100644
index 0000000..aa5f423
--- /dev/null
+++ b/tools/tests/fake_devices.h
@@ -0,0 +1,28 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Andreas Reichel <andreas.r...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#ifndef __FAKE_DEVICES_H__
+#define __FAKE_DEVICES_H__
+
+#include <ebgpart.h>
+
+extern PedDevice *fake_devices;
+extern int num_fake_devices;
+
+void allocate_fake_devices(int n);
+void add_fake_partition(int devnum);
+void remove_fake_partitions(int n);
+void free_fake_devices(void);
+
+PedDevice *ped_device_get_next_custom_fake(const PedDevice *dev);
+
+#endif // __FAKE_DEVICES_H__
diff --git a/tools/tests/test_bgenv_init_retval.c b/tools/tests/test_bgenv_init_retval.c
new file mode 100644
index 0000000..76ec253
--- /dev/null
+++ b/tools/tests/test_bgenv_init_retval.c
@@ -0,0 +1,100 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Andreas Reichel <andreas.r...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#ifndef ENV_NUM_CONFIG_PARTITIONS
+# define ENV_NUM_CONFIG_PARTITIONS 2
+#endif
+
+#include <stdlib.h>
+#include <check.h>
+#include <fff.h>
+#include <env_api.h>
+#include <env_config_file.h>
+#include <env_config_partitions.h>
+
+DEFINE_FFF_GLOBALS;
+
+static char *devpath = "/dev/nobrain";
+
+bool read_env(CONFIG_PART *part, BG_ENVDATA *env);
+
+Suite *env_api_fat_suite(void);
+bool probe_config_partitions_custom_fake(CONFIG_PART *cfgpart);
+bool read_env_custom_fake(CONFIG_PART *cp, BG_ENVDATA *env);
+
+Suite *ebg_test_suite(void);
+
+bool probe_config_partitions_custom_fake(CONFIG_PART *cfgpart)
+{
+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {
+ cfgpart[i].devpath = devpath;
+ }
+ return true;
+}
+
+bool read_env_custom_fake(CONFIG_PART *cp, BG_ENVDATA *env)
+{
+ if (!env) {
+ return false;
+ }
+ memset(env, 0, sizeof(BG_ENVDATA));
+ return true;
+}
+
+FAKE_VALUE_FUNC(bool, probe_config_partitions, CONFIG_PART *);
+FAKE_VALUE_FUNC(bool, read_env, CONFIG_PART *, BG_ENVDATA *);
+
+START_TEST(env_api_fat_test_bgenv_init_retval)
+{
+ bool result;
+ /* In this unit test, contents of environment data are
+ * faked to be all zero
+ */
+
+ /* Test if bgenv_init fails if no config partitions are found
+ */
+ RESET_FAKE(probe_config_partitions);
+
+ probe_config_partitions_fake.return_val = false;
+
+ result = bgenv_init();
+
+ ck_assert(probe_config_partitions_fake.call_count == 1);
+ ck_assert(result == false);
+
+ /* Test if bgenv_init succeeds if config partitions are found
+ */
+ RESET_FAKE(probe_config_partitions);
+
+ probe_config_partitions_fake.custom_fake = probe_config_partitions_custom_fake;
+ read_env_fake.custom_fake = read_env_custom_fake;
+ result = bgenv_init();
+
+ ck_assert(probe_config_partitions_fake.call_count == 1);
+ ck_assert(read_env_fake.call_count == ENV_NUM_CONFIG_PARTS);
+ ck_assert(result == true);
+}
+END_TEST
+
+Suite *ebg_test_suite(void)
+{
+ Suite *s;
+ TCase *tc_core;
+
+ s = suite_create("env_api_fat");
+
+ tc_core = tcase_create("Core");
+ tcase_add_test(tc_core, env_api_fat_test_bgenv_init_retval);
+ suite_add_tcase(s, tc_core);
+
+ return s;
+}
diff --git a/tools/tests/test_ebgenv_api.c b/tools/tests/test_ebgenv_api.c
new file mode 100644
index 0000000..530f338
--- /dev/null
+++ b/tools/tests/test_ebgenv_api.c
@@ -0,0 +1,641 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Andreas Reichel <andreas.r...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#include <config.h>
+
+#ifndef ENV_NUM_CONFIG_PARTITIONS
+# define ENV_NUM_CONFIG_PARTITIONS 2
+#endif
+
+#include <stdlib.h>
+#include <check.h>
+#include <fff.h>
+#include <env_api.h>
+#include <ebgenv.h>
+#include <env_config_file.h>
+#include <env_config_partitions.h>
+
+DEFINE_FFF_GLOBALS;
+
+static char *devpath = "/dev/nobrain";
+
+Suite *ebg_test_suite(void);
+
+extern bool write_env(CONFIG_PART *part, BG_ENVDATA *env);
+extern bool bgenv_write(BGENV *);
+extern bool bgenv_init(void);
+extern bool bgenv_close(BGENV *);
+extern BGENV *bgenv_create_new(void);
+
+FAKE_VALUE_FUNC(bool, bgenv_init);
+FAKE_VALUE_FUNC(bool, bgenv_write, BGENV *);
+FAKE_VALUE_FUNC(bool, bgenv_close, BGENV *);
+
+int __real_bgenv_set(BGENV *, char *, char *, void *, uint32_t);
+int __wrap_bgenv_set(BGENV *, char *, char *, void *, uint32_t);
+int __real_bgenv_get(BGENV *, char *, char *, void *, uint32_t);
+int __wrap_bgenv_get(BGENV *, char *, char *, void *, uint32_t);
+
+BGENV *bgenv_getset_arg0;
+char *bgenv_getset_arg1;
+char *bgenv_getset_arg2;
+void *bgenv_getset_arg3;
+uint32_t bgenv_getset_arg4;
+int bgenv_get_call_count;
+int bgenv_set_call_count;
+
+/* FFF does not provide calls to the original function, so in this case
+ * we need to use the linker wrapping method and reimplement some of FFFs
+ * functionality.
+ */
+int __wrap_bgenv_get(BGENV *env, char *key, char *type, void *buffer, uint32_t len)
+{
+ bgenv_get_call_count++;
+ bgenv_getset_arg0 = env;
+ bgenv_getset_arg1 = key;
+ bgenv_getset_arg2 = type;
+ bgenv_getset_arg3 = buffer;
+ bgenv_getset_arg4 = len;
+ return __real_bgenv_get(env, key, type, buffer, len);
+}
+
+int __wrap_bgenv_set(BGENV *env, char *key, char *type, void *buffer, uint32_t len)
+{
+ bgenv_set_call_count++;
+ bgenv_getset_arg0 = env;
+ bgenv_getset_arg1 = key;
+ bgenv_getset_arg2 = type;
+ bgenv_getset_arg3 = buffer;
+ bgenv_getset_arg4 = len;
+ return __real_bgenv_set(env, key, type, buffer, len);
+}
+
+/* These variables substitute weakened symbols in the ebgenv library code
+ * so that all environment functions use these as data sources
+ */
+CONFIG_PART config_parts[ENV_NUM_CONFIG_PARTS];
+BG_ENVDATA envdata[ENV_NUM_CONFIG_PARTS];
+
+START_TEST(ebgenv_api_ebg_env_create_new)
+{
+ ebgenv_t e;
+ int ret;
+ wchar_t bufferw[10];
+ char buffer[10];
+ char *kernelfile = "kernel123";
+ char *kernelparams = "param456";
+
+ memset(&e, 0, sizeof(e));
+
+ /* Test if ebg_env_create_new returns EIO if bgenv_init
+ * returns false
+ */
+ bgenv_init_fake.return_val = false;
+ ret = ebg_env_create_new(&e);
+ ck_assert_int_eq(ret, EIO);
+
+ /* Test if errno is returned by ebg_env_created, if the bgenv pointer
+ * is NULL but ebg_new_env_created is true, which is contradictory.
+ * Also, ebg_new_env_created must be reset to false.
+ */
+ bgenv_init_fake.return_val = true;
+ bgenv_close_fake.return_val = true;
+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++)
+ {
+ envdata[i].revision = i;
+ }
+ e.ebg_new_env_created = true;
+ e.bgenv = NULL;
+ errno = 3044;
+
+ ret = ebg_env_create_new(&e);
+
+ ck_assert_int_eq(ret, 3044);
+ ck_assert(e.ebg_new_env_created == false);
+
+ /* Check if values of the latest environment are copied if a new
+ * environment is created. The new environment must overwrite the
+ * oldest environment and revision and ustate must be set correctly.
+ */
+ envdata[ENV_NUM_CONFIG_PARTS-1].watchdog_timeout_sec = 44;
+ (void)str8to16(bufferw, kernelfile);
+ memcpy(envdata[ENV_NUM_CONFIG_PARTS-1].kernelfile, bufferw,
+ strlen(kernelfile) * 2 + 2);
+ (void)str8to16(bufferw, kernelparams);
+ memcpy(envdata[ENV_NUM_CONFIG_PARTS-1].kernelparams, bufferw,
+ strlen(kernelparams) * 2 + 2);
+ errno = 0;
+
+ ret = ebg_env_create_new(&e);
+
+ ck_assert_int_eq(errno, 0);
+ ck_assert_int_eq(ret, 0);
+ ck_assert(((BGENV *)e.bgenv)->data == &envdata[0]);
+ ck_assert_int_eq(
+ ((BGENV *)e.bgenv)->data->revision, ENV_NUM_CONFIG_PARTS);
+ ck_assert_int_eq(((BGENV *)e.bgenv)->data->ustate, USTATE_INSTALLED);
+ ck_assert_int_eq(((BGENV *)e.bgenv)->data->watchdog_timeout_sec, 44);
+ (void)str16to8(buffer, ((BGENV *)e.bgenv)->data->kernelfile);
+ ck_assert_int_eq(
+ strncmp(buffer, kernelfile, strlen(kernelfile) + 1), 0);
+ (void)str16to8(buffer, ((BGENV *)e.bgenv)->data->kernelparams);
+ ck_assert_int_eq(
+ strncmp(buffer, kernelparams, strlen(kernelparams) + 1), 0);
+}
+END_TEST
+
+START_TEST(ebgenv_api_ebg_env_open_current)
+{
+ ebgenv_t e;
+ int ret;
+ memset(&e, 0, sizeof(e));
+
+ /* Test if ebg_env_open_current returns EIO if bgenv_init returns false
+ */
+ bgenv_init_fake.return_val = false;
+ ret = ebg_env_open_current(&e);
+
+ ck_assert_int_eq(ret, EIO);
+
+#if ENV_NUM_CONFIG_PARTS > 1
+
+ /* Test if ebg_env_open_current opens the environment with the highest
+ * revision
+ */
+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {
+ envdata[i].revision = i + 1;
+ }
+
+ bgenv_init_fake.return_val = true;
+ ret = ebg_env_open_current(&e);
+
+ ck_assert_int_eq(ret, 0);
+ ck_assert(((BGENV *)e.bgenv)->data == &envdata[ENV_NUM_CONFIG_PARTS-1]);
+
+ (void)ebg_env_close(&e);
+
+ envdata[0].revision = 0xFFFF;
+
+ ret = ebg_env_open_current(&e);
+
+ ck_assert_int_eq(ret, 0);
+ ck_assert(((BGENV *)e.bgenv)->data == &envdata[0]);
+
+ (void)ebg_env_close(&e);
+#endif
+
+}
+END_TEST
+
+START_TEST(ebgenv_api_ebg_env_get)
+{
+ ebgenv_t e;
+ memset(&e, 0, sizeof(e));
+ int ret;
+ char buffer[1];
+
+ /* Test if ebg_env_get calls bg_env_get correctly and that it returns
+ * -EINVAL if no key is provided
+ */
+ bgenv_get_call_count = 0;
+
+ ret = ebg_env_get(&e, NULL, NULL);
+ ck_assert_int_eq(ret, -EINVAL);
+
+ ck_assert(bgenv_get_call_count == 1);
+ ck_assert(bgenv_getset_arg0 == e.bgenv);
+ ck_assert(bgenv_getset_arg1 == NULL);
+ ck_assert(bgenv_getset_arg2 == NULL);
+
+ /* Test if ebg_env_get retrieves correct data if given a valid
+ * environment handle.
+ */
+ e.bgenv = (BGENV *)calloc(1, sizeof(BGENV));
+ ck_assert(e.bgenv != NULL);
+
+ ((BGENV *)e.bgenv)->data = (BG_ENVDATA *)calloc(1, sizeof(BG_ENVDATA));
+ ck_assert(((BGENV *)e.bgenv)->data != NULL);
+
+ bgenv_get_call_count = 0;
+
+ (void)ebg_env_get(&e, "kernelfile", buffer);
+
+ ck_assert(bgenv_get_call_count == 1);
+ ck_assert(bgenv_getset_arg0 == e.bgenv);
+ ck_assert_int_eq(strcmp(bgenv_getset_arg1, "kernelfile"), 0);
+ ck_assert(bgenv_getset_arg3 == buffer);
+
+ free(((BGENV *)e.bgenv)->data);
+ free(e.bgenv);
+
+}
+END_TEST
+
+START_TEST(ebgenv_api_ebg_env_set)
+{
+ ebgenv_t e;
+ memset(&e, 0, sizeof(e));
+ char *value = "dummy";
+
+ /* Check if ebg_env_set correctly calls bgenv_set
+ */
+ bgenv_set_call_count = 0;
+
+ e.bgenv = (BGENV *)calloc(1, sizeof(BGENV));
+ ck_assert(e.bgenv != NULL);
+
+ ((BGENV *)e.bgenv)->data = (BG_ENVDATA *)calloc(1, sizeof(BG_ENVDATA));
+ ck_assert(((BGENV *)e.bgenv)->data != NULL);
+
+ (void)ebg_env_set(&e, "kernelfile", value);
+
+ ck_assert(bgenv_set_call_count == 1);
+ ck_assert(bgenv_getset_arg0 == e.bgenv);
+ ck_assert_int_eq(strcmp(bgenv_getset_arg1, "kernelfile"), 0);
+ ck_assert(bgenv_getset_arg3 == value);
+ ck_assert(bgenv_getset_arg4 == strlen(value) + 1);
+
+ free(((BGENV *)e.bgenv)->data);
+ free(e.bgenv);
+}
+END_TEST
+
+START_TEST(ebgenv_api_ebg_env_set_ex)
+{
+
+ ebgenv_t e;
+ memset(&e, 0, sizeof(e));
+ char *key = "mykey";
+ char *value = "dummy";
+ char *usertype = "mytype";
+ int32_t datalen = 5;
+
+ /* Check if ebg_env_set_ex correctly calls bgenv_set
+ */
+ bgenv_set_call_count = 0;
+
+ e.bgenv = (BGENV *)calloc(1, sizeof(BGENV));
+ ck_assert(e.bgenv != NULL);
+
+ ((BGENV *)e.bgenv)->data = (BG_ENVDATA *)calloc(1, sizeof(BG_ENVDATA));
+ ck_assert(((BGENV *)e.bgenv)->data != NULL);
+
+ bgenv_set_call_count = 0;
+
+ (void)ebg_env_set_ex(&e, key, usertype, (uint8_t *)value, datalen);
+
+ ck_assert(bgenv_set_call_count == 1);
+ ck_assert(bgenv_getset_arg0 == e.bgenv);
+ ck_assert_int_eq(strcmp(bgenv_getset_arg1, key), 0);
+ ck_assert_int_eq(strcmp(bgenv_getset_arg2, usertype), 0);
+ ck_assert(bgenv_getset_arg3 == value);
+ ck_assert(bgenv_getset_arg4 == datalen);
+
+ free(((BGENV *)e.bgenv)->data);
+ free(e.bgenv);
+}
+END_TEST
+
+START_TEST(ebgenv_api_ebg_env_get_ex)
+{
+ ebgenv_t e;
+ memset(&e, 0, sizeof(e));
+ char *key = "mykey";
+ char buffer[5];
+ char type[7];
+ int32_t datalen = 5;
+
+ /* Check if ebg_env_get_ex correctly calls bgenv_get
+ */
+ bgenv_get_call_count = 0;
+
+ e.bgenv = (BGENV *)calloc(1, sizeof(BGENV));
+ ck_assert(e.bgenv != NULL);
+
+ ((BGENV *)e.bgenv)->data = (BG_ENVDATA *)calloc(1, sizeof(BG_ENVDATA));
+ ck_assert(((BGENV *)e.bgenv)->data != NULL);
+
+ bgenv_get_call_count = 0;
+
+ (void)ebg_env_get_ex(&e, key, type, (uint8_t *)buffer, datalen);
+
+ ck_assert(bgenv_get_call_count == 1);
+ ck_assert(bgenv_getset_arg0 == e.bgenv);
+ ck_assert_int_eq(strcmp(bgenv_getset_arg1, key), 0);
+ ck_assert(bgenv_getset_arg2 == type);
+ ck_assert(bgenv_getset_arg3 == buffer);
+ ck_assert(bgenv_getset_arg4 == datalen);
+
+ free(((BGENV *)e.bgenv)->data);
+ free(e.bgenv);
+}
+END_TEST
+
+START_TEST(ebgenv_api_ebg_env_user_free)
+{
+ ebgenv_t e;
+ uint32_t ret;
+ memset(&e, 0, sizeof(e));
+
+ /* Check if ebg_env_user_free returns 0 if no environment handle
+ * is available (invalid context).
+ */
+ ret = ebg_env_user_free(&e);
+ ck_assert_int_eq(ret, 0);
+
+ /* Check if ebg_env_user_free returns 0 if no environment data
+ * is available (NULL environment).
+ */
+ e.bgenv = (BGENV *)calloc(1, sizeof(BGENV));
+ ck_assert(e.bgenv != NULL);
+
+ ret = ebg_env_user_free(&e);
+ ck_assert_int_eq(ret, 0);
+
+ /* Check if ebg_env_user_free returns ENV_MEM_USERVARS if environment
+ * user space is empty
+ */
+ ((BGENV *)e.bgenv)->data = (BG_ENVDATA *)calloc(1, sizeof(BG_ENVDATA));
+ ck_assert(((BGENV *)e.bgenv)->data != NULL);
+
+ ret = ebg_env_user_free(&e);
+ ck_assert_int_eq(ret, ENV_MEM_USERVARS);
+
+ free(((BGENV *)e.bgenv)->data);
+ free(e.bgenv);
+}
+END_TEST
+
+START_TEST(ebgenv_api_ebg_env_getglobalstate)
+{
+#if ENV_NUM_CONFIG_PARTS > 1
+ ebgenv_t e;
+ uint16_t state;
+ memset(&e, 0, sizeof(e));
+
+ /* Test if ebg_env_getglobalstate returns OK if current environment
+ * is set to OK
+ */
+ e.bgenv = (BGENV *)calloc(1, sizeof(BGENV));
+ ck_assert(e.bgenv != NULL);
+
+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {
+ envdata[i].revision = i + 1;
+ }
+
+ envdata[1].revision = 0;
+ envdata[1].ustate = USTATE_OK;
+
+ state = ebg_env_getglobalstate(&e);
+ ck_assert_int_eq(state, USTATE_OK);
+
+ /* Test if ebg_env_getglobalstate returns FAILED if current environment
+ * is set to FAILED with revision 0
+ */
+ envdata[1].revision = 0;
+ envdata[1].ustate = USTATE_FAILED;
+
+ state = ebg_env_getglobalstate(&e);
+ ck_assert_int_eq(state, USTATE_FAILED);
+
+ /* Test if ebg_env_getglobalstate returns FAILED if current environment
+ * is set to FAILED with non-zero revision
+ */
+ envdata[1].revision = 15;
+
+ state = ebg_env_getglobalstate(&e);
+ ck_assert_int_eq(state, USTATE_FAILED);
+
+ /* Test if ebg_env_getglobalstate returns INSTALLED if current
+ * environment is set to INSTALLED
+ */
+ envdata[1].revision = 15;
+ envdata[1].ustate = USTATE_INSTALLED;
+
+ state = ebg_env_getglobalstate(&e);
+ ck_assert_int_eq(state, USTATE_INSTALLED);
+
+ /* Test if ebg_env_getglobalstate returns FAILED if current environment
+ * is set to OK and any other is set to FAILED
+ */
+ envdata[1].ustate = USTATE_OK;
+ envdata[0].revision = 0;
+ envdata[0].ustate = USTATE_FAILED;
+
+ state = ebg_env_getglobalstate(&e);
+ ck_assert_int_eq(state, USTATE_FAILED);
+
+ /* Test if ebg_env_getglobalstate returns OK if current environment is
+ * set to OK and any other is set to INSTALLED
+ */
+ envdata[0].ustate = USTATE_INSTALLED;
+
+ state = ebg_env_getglobalstate(&e);
+ ck_assert_int_eq(state, USTATE_OK);
+
+ /* Test if ebg_env_getglobalstate returns TESTING if current
+ * environment is set to TESTING and none is FAILED
+ */
+ envdata[0].ustate = USTATE_OK;
+ envdata[1].ustate = USTATE_TESTING;
+
+ state = ebg_env_getglobalstate(&e);
+ ck_assert_int_eq(state, USTATE_TESTING);
+
+ /* Test if ebg_env_getglobalstate returns OK if current environment is
+ * set to OK and none is TESTING
+ */
+ envdata[0].ustate = USTATE_TESTING;
+ envdata[1].ustate = USTATE_OK;
+
+ state = ebg_env_getglobalstate(&e);
+ ck_assert_int_eq(state, USTATE_OK);
+
+ free(e.bgenv);
+#endif
+}
+END_TEST
+
+START_TEST(ebgenv_api_ebg_env_setglobalstate)
+{
+#if ENV_NUM_CONFIG_PARTS > 1
+ ebgenv_t e;
+ int ret;
+ memset(&e, 0, sizeof(e));
+
+ /* Test if ebg_env_setglobalstate sets only current to FAILED
+ */
+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {
+ envdata[i].revision = i + 1;
+ }
+
+ bgenv_init_fake.return_val = true;
+
+ ret = ebg_env_open_current(&e);
+
+ ck_assert_int_eq(ret, 0);
+ ck_assert(((BGENV *)e.bgenv)->data == &envdata[ENV_NUM_CONFIG_PARTS-1]);
+
+ envdata[0].ustate = USTATE_OK;
+ envdata[1].ustate = USTATE_OK;
+
+ ret = ebg_env_setglobalstate(&e, 0xFFF);
+ ck_assert_int_eq(ret, EINVAL);
+
+ ret = ebg_env_setglobalstate(&e, USTATE_FAILED);
+
+ ck_assert_int_eq(ret, 0);
+
+ ck_assert_int_eq(envdata[0].ustate, USTATE_OK);
+ ck_assert_int_eq(envdata[ENV_NUM_CONFIG_PARTS-1].ustate, USTATE_FAILED);
+
+ envdata[1].ustate = USTATE_OK;
+ envdata[0].ustate = USTATE_OK;
+
+ (void)ebg_env_close(&e);
+
+ envdata[0].revision = 1313;
+
+ ret = ebg_env_open_current(&e);
+ ck_assert_int_eq(ret, 0);
+
+ ret = ebg_env_setglobalstate(&e, USTATE_FAILED);
+ ck_assert_int_eq(ret, 0);
+ ck_assert_int_eq(envdata[0].ustate, USTATE_FAILED);
+ ck_assert_int_eq(envdata[1].ustate, USTATE_OK);
+
+ /* Test if ebg_env_setglobalstate sets ALL environments to OK
+ */
+ envdata[0].ustate = USTATE_FAILED;
+ envdata[1].ustate = USTATE_FAILED;
+
+ bgenv_write_fake.return_val = true;
+ bgenv_close_fake.return_val = true;
+
+ ret = ebg_env_setglobalstate(&e, USTATE_OK);
+
+ ck_assert_int_eq(ret, 0);
+ ck_assert_int_eq(envdata[0].ustate, USTATE_OK);
+ ck_assert_int_eq(envdata[1].ustate, USTATE_OK);
+
+ /* Test if ebg_env_setglobalstate sets current environment to TESTING
+ */
+ envdata[0].ustate = USTATE_INSTALLED;
+ envdata[1].ustate = USTATE_INSTALLED;
+
+ ret = ebg_env_setglobalstate(&e, USTATE_TESTING);
+
+ ck_assert_int_eq(ret, 0);
+ ck_assert_int_eq(envdata[0].ustate, USTATE_TESTING);
+ ck_assert_int_eq(envdata[1].ustate, USTATE_INSTALLED);
+
+ /* Test if ebg_env_setglobalstate fails and returns EIO if bgenv_write
+ * fails
+ */
+ bgenv_write_fake.return_val = false;
+ bgenv_close_fake.return_val = true;
+
+ ret = ebg_env_setglobalstate(&e, USTATE_OK);
+
+ ck_assert_int_eq(ret, EIO);
+
+ /* Test if ebg_env_setglobalstate fails and returns EIO if bgenv_close
+ * fails
+ */
+ bgenv_write_fake.return_val = true;
+ bgenv_close_fake.return_val = false;
+
+ ret = ebg_env_setglobalstate(&e, USTATE_OK);
+
+ ck_assert_int_eq(ret, EIO);
+
+ (void)ebg_env_close(&e);
+#endif
+}
+END_TEST
+
+START_TEST(ebgenv_api_ebg_env_close)
+{
+ ebgenv_t e;
+ int ret;
+ memset(&e, 0, sizeof(e));
+
+ /* Test if ebg_env_close fails with invalid context and returns EIO
+ */
+ ret = ebg_env_close(&e);
+ ck_assert_int_eq(ret, EIO);
+
+ /* Test if ebg_env_close fails and returns EIO if bgenv_write fails
+ */
+ e.bgenv = calloc(1, sizeof(BGENV));
+ ck_assert(e.bgenv != NULL);
+
+ ((BGENV *)e.bgenv)->data = calloc(1, sizeof(BG_ENVDATA));
+ bgenv_write_fake.return_val = false;
+ bgenv_close_fake.return_val = true;
+ ret = ebg_env_close(&e);
+
+ ck_assert_int_eq(ret, EIO);
+
+ /* Test if ebg_env_close fails and returns EIO if bgenv_close fails.
+ */
+ bgenv_write_fake.return_val = true;
+ bgenv_close_fake.return_val = false;
+ ret = ebg_env_close(&e);
+
+ ck_assert_int_eq(ret, EIO);
+
+ /* Test if ebg_env_close is successful if all prerequisites are met
+ */
+ bgenv_write_fake.return_val = true;
+ bgenv_close_fake.return_val = true;
+ BGENV *save_ptr = e.bgenv;
+ ret = ebg_env_close(&e);
+
+ ck_assert_int_eq(ret, 0);
+ ck_assert(e.bgenv == NULL);
+
+ free(save_ptr->data);
+ free(save_ptr);
+}
+END_TEST
+
+Suite *ebg_test_suite(void)
+{
+ Suite *s;
+ TCase *tc_core;
+
+ s = suite_create("ebgenv_api");
+
+ TFun tfuncs[] = {
+ ebgenv_api_ebg_env_create_new,
+ ebgenv_api_ebg_env_open_current,
+ ebgenv_api_ebg_env_get,
+ ebgenv_api_ebg_env_set,
+ ebgenv_api_ebg_env_set_ex,
+ ebgenv_api_ebg_env_get_ex,
+ ebgenv_api_ebg_env_user_free,
+ ebgenv_api_ebg_env_getglobalstate,
+ ebgenv_api_ebg_env_setglobalstate,
+ ebgenv_api_ebg_env_close
+ };
+
+ tc_core = tcase_create("Core");
+
+ for (int i = 0; i < sizeof(tfuncs)/sizeof(void *); i++) {
+ tcase_add_test(tc_core, tfuncs[i]);
+ }
+
+ suite_add_tcase(s, tc_core);
+
+ return s;
+}
diff --git a/tools/tests/test_ebgenv_api_internal.c b/tools/tests/test_ebgenv_api_internal.c
new file mode 100644
index 0000000..1f5ce93
--- /dev/null
+++ b/tools/tests/test_ebgenv_api_internal.c
@@ -0,0 +1,416 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Andreas Reichel <andreas.r...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#ifndef ENV_NUM_CONFIG_PARTITIONS
+# define ENV_NUM_CONFIG_PARTITIONS 2
+#endif
+
+#include <stdlib.h>
+#include <check.h>
+#include <fff.h>
+#include <env_api.h>
+#include <env_config_file.h>
+#include <env_config_partitions.h>
+
+DEFINE_FFF_GLOBALS;
+
+static char *devpath = "/dev/nobrain";
+
+Suite *ebg_test_suite(void);
+
+extern bool write_env(CONFIG_PART *part, BG_ENVDATA *env);
+extern EBGENVKEY bgenv_str2enum(char *);
+extern BGENV *bgenv_open_by_index(uint32_t index);
+
+bool write_env_custom_fake(CONFIG_PART *part, BG_ENVDATA *env);
+
+bool write_env_custom_fake(CONFIG_PART *part, BG_ENVDATA *env)
+{
+ return true;
+}
+
+FAKE_VALUE_FUNC(bool, write_env, CONFIG_PART *, BG_ENVDATA *);
+
+CONFIG_PART config_parts[ENV_NUM_CONFIG_PARTS];
+BG_ENVDATA envdata[ENV_NUM_CONFIG_PARTS];
+
+START_TEST(ebgenv_api_internal_strXtoY)
+{
+ wchar_t *exp_res = L"This is a test";
+ wchar_t bufferw[16];
+ char buffer[16];
+ char *input = "This is a test";
+ wchar_t *resw;
+ char *res;
+
+ /* Test conversion from ASCII bits to 16 bit encoding
+ */
+ resw = str8to16(bufferw, input);
+
+ /* cannot use glibc for 16-bit wchar_t
+ * string compare since glibc has 32-bit wchar_t
+ */
+ for (int i = 0; i < strlen(input); i++) {
+ ck_assert(resw[i] == exp_res[i]);
+ }
+
+ /* Test conversion from 16 bit encoding to ASCII
+ */
+ res = str16to8(buffer, exp_res);
+
+ ck_assert(strcmp(res, input) == 0);
+}
+END_TEST
+
+START_TEST(ebgenv_api_internal_bgenv_str2enum)
+{
+ EBGENVKEY e;
+
+ /* Test bgenv_str2enum for correct key conversion
+ */
+ e = bgenv_str2enum("kernelfile");
+ ck_assert(e == EBGENV_KERNELFILE);
+
+ e = bgenv_str2enum("kernelparams");
+ ck_assert(e == EBGENV_KERNELPARAMS);
+
+ e = bgenv_str2enum("watchdog_timeout_sec");
+ ck_assert(e == EBGENV_WATCHDOG_TIMEOUT_SEC);
+
+ e = bgenv_str2enum("revision");
+ ck_assert(e == EBGENV_REVISION);
+
+ e = bgenv_str2enum("ustate");
+ ck_assert(e == EBGENV_USTATE);
+
+ /* Test if bgenv_str2enum returns EBGENV_UNKNOWN for empty and invalid
+ * keys
+ */
+ e = bgenv_str2enum("XZXOOZOOZIOFZOFZ");
+ ck_assert(e == EBGENV_UNKNOWN);
+
+ e = bgenv_str2enum("");
+ ck_assert(e == EBGENV_UNKNOWN);
+
+}
+END_TEST
+
+START_TEST(ebgenv_api_internal_bgenv_open_by_index)
+{
+ BGENV *handle;
+
+ handle = bgenv_open_by_index(0);
+ ck_assert(handle != NULL);
+ ck_assert(handle->desc == &config_parts[0]);
+ ck_assert(handle->data == &envdata[0]);
+ free(handle);
+
+ handle = bgenv_open_by_index(ENV_NUM_CONFIG_PARTS-1);
+ ck_assert(handle != NULL);
+ ck_assert(handle->desc == &config_parts[ENV_NUM_CONFIG_PARTS-1]);
+ ck_assert(handle->data == &envdata[ENV_NUM_CONFIG_PARTS-1]);
+ free(handle);
+
+ /* Test if bgenv_open_by_index returns NULL if parameter is out of
+ * range
+ */
+ handle = bgenv_open_by_index(ENV_NUM_CONFIG_PARTS);
+ ck_assert(handle == NULL);
+}
+END_TEST
+
+START_TEST(ebgenv_api_internal_bgenv_open_oldest)
+{
+ BGENV *handle;
+
+ /* Test if bgenv_open_oldest returns a handle for the environment with
+ * the lowest revision
+ */
+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++)
+ {
+ envdata[i].revision = ENV_NUM_CONFIG_PARTS - i;
+ }
+ handle = bgenv_open_oldest();
+ ck_assert(handle != NULL);
+ ck_assert(handle->desc == &config_parts[ENV_NUM_CONFIG_PARTS-1]);
+ ck_assert(handle->data == &envdata[ENV_NUM_CONFIG_PARTS-1]);
+ free(handle);
+}
+END_TEST
+
+START_TEST(ebgenv_api_internal_bgenv_open_latest)
+{
+ BGENV *handle;
+
+ /* Test if bgenv_open_latest returns a handle for the environment with
+ * the highest revision
+ */
+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++)
+ {
+ envdata[i].revision = ENV_NUM_CONFIG_PARTS - i;
+ }
+ handle = bgenv_open_latest();
+ ck_assert(handle != NULL);
+ ck_assert(handle->desc == &config_parts[0]);
+ ck_assert(handle->data == &envdata[0]);
+ free(handle);
+}
+END_TEST
+
+START_TEST(ebgenv_api_internal_bgenv_write)
+{
+ bool res;
+ BGENV *dummy_env;
+
+ dummy_env = calloc(1, sizeof(BGENV));
+ if (!dummy_env)
+ goto bgew_error;
+
+ RESET_FAKE(write_env);
+ write_env_fake.custom_fake = write_env_custom_fake;
+
+ /* Test if writing with a NULL-handle fails
+ */
+ res = bgenv_write(NULL);
+ ck_assert(write_env_fake.call_count == 0);
+ ck_assert(res == false);
+
+ /* Test if writing with a handle describing no partition
+ * and no environment data fails
+ */
+ res = bgenv_write(dummy_env);
+ ck_assert(write_env_fake.call_count == 0);
+ ck_assert(res == false);
+
+ /* Test if writing with a handle describing both partition
+ * and envrionment data succeeds
+ */
+ dummy_env->desc = calloc(1, sizeof(CONFIG_PART));
+ if (!dummy_env->desc)
+ goto bgew_error;
+
+ dummy_env->data = calloc(1, sizeof(BG_ENVDATA));
+ if (!dummy_env->data)
+ goto bgew_error;
+
+ res = bgenv_write(dummy_env);
+ ck_assert(write_env_fake.call_count == 1);
+ ck_assert(res == true);
+
+ return;
+
+bgew_error:
+ free(dummy_env->data);
+ free(dummy_env->desc);
+ free(dummy_env);
+ exit(errno);
+}
+END_TEST
+
+START_TEST(ebgenv_api_internal_bgenv_read)
+{
+ BGENV env;
+ BG_ENVDATA data;
+
+ env.data = &data;
+
+ /* Test if bgenv_read returns a pointer to the environment data
+ */
+ BG_ENVDATA *res = bgenv_read(&env);
+ ck_assert(res == env.data);
+}
+END_TEST
+
+START_TEST(ebgenv_api_internal_bgenv_create_new)
+{
+ BGENV *handle;
+ int max = ENV_NUM_CONFIG_PARTS;
+
+ for (int i = 0; i < max; i++)
+ {
+ envdata[i].revision = max - i;
+ }
+
+ /* Test if bgenv_create_new updates the oldest environment with default
+ * values and sets its revision to revision(latest)+1
+ */
+ handle = bgenv_create_new();
+
+ ck_assert(handle != NULL);
+ ck_assert(handle->data == &envdata[max-1]);
+ ck_assert(envdata[max-1].revision == max+1);
+ ck_assert(envdata[max-1].watchdog_timeout_sec == 30);
+
+ free(handle);
+}
+END_TEST
+
+START_TEST(ebgenv_api_internal_bgenv_get)
+{
+ BGENV *handle = bgenv_open_latest();
+ ck_assert(handle != NULL);
+
+ wchar_t buffer[ENV_STRING_LENGTH];
+ char *test_strings[] = {
+ "kernelfile_test123",
+ "kernelparams_test123",
+ };
+ void *dests[] = {
+ &handle->data->kernelfile,
+ &handle->data->kernelparams,
+ };
+
+ for (int i = 0; i < sizeof(test_strings)/sizeof(void*); i++)
+ {
+ memcpy(dests[i], str8to16(buffer, test_strings[i]),
+ strlen(test_strings[i]) * 2 + 2);
+ }
+ handle->data->watchdog_timeout_sec = 44;
+ handle->data->revision = 10000;
+ handle->data->ustate = USTATE_INSTALLED;
+
+ char *type = NULL, *data = NULL;
+ char buffera[22];
+ int res;
+
+ /* Test if bgenv_get fails if maxlen is set to 0
+ */
+ res = bgenv_get(handle, "kernelfile", type, data, 0);
+ ck_assert_int_eq(res, -EINVAL);
+
+ /* Test if bgenv_get fails if key is NULL
+ */
+ res = bgenv_get(handle, NULL, type, data, 1000);
+ ck_assert_int_eq(res, -EINVAL);
+
+ /* Test if bgenv_get fails if no environment is provided
+ */
+ res = bgenv_get(NULL, "kernelfile", type, NULL, 1000);
+ ck_assert_int_eq(res, -EPERM);
+
+ /* Test if bgenv_get returns the correct size of the needed
+ * buffer if provided with a NULL buffer
+ */
+ res = bgenv_get(handle, "kernelfile", type, NULL, 1000);
+ ck_assert_int_eq(res, strlen(test_strings[0]) + 1);
+
+ /* Test if bgenv_get returns the correct value
+ */
+ res = bgenv_get(handle, "kernelfile", type, buffera, res);
+ ck_assert_int_eq(strcmp(buffera, test_strings[0]), 0);
+
+ res = bgenv_get(handle, "kernelparams", type, NULL, 1000);
+ res = bgenv_get(handle, "kernelparams", type, buffera, res);
+ ck_assert_int_eq(strcmp(buffera, test_strings[1]), 0);
+
+ free(handle);
+}
+END_TEST
+
+START_TEST(ebgenv_api_internal_bgenv_set)
+{
+ int res;
+
+ BGENV *handle = bgenv_open_latest();
+ ck_assert(handle != NULL);
+ ck_assert(handle->data != NULL);
+
+ /* Test if bgenv_set returns -EINVAL if the handle is invalid
+ */
+ res = bgenv_set(NULL, "kernelfile", NULL, NULL, 0);
+ ck_assert_int_eq(res, -EINVAL);
+
+ /* Test if bgenv_set returns -EINVAL if the key is invalid
+ */
+ res = bgenv_set(handle, "AOFIJAOEGIHA", NULL, NULL, 0);
+ ck_assert_int_eq(res, -EINVAL);
+
+ /* Test if bgenv_set works correctly for valid parameters
+ */
+ res = bgenv_set(handle, "kernelfile", NULL, "vmlinuz", 8);
+ ck_assert_int_eq(res, 0);
+
+ char buffer[8];
+ char *kfile = str16to8(buffer, handle->data->kernelfile);
+
+ ck_assert(strcmp(kfile, "vmlinuz") == 0);
+
+ res = bgenv_set(handle, "watchdog_timeout_sec", NULL, "-0", 2);
+ ck_assert_int_eq(res, 0);
+ ck_assert_int_eq(handle->data->watchdog_timeout_sec, 0);
+
+ res = bgenv_set(handle, "watchdog_timeout_sec", NULL, "311", 4);
+ ck_assert_int_eq(res, 0);
+ ck_assert_int_eq(handle->data->watchdog_timeout_sec, 311);
+
+ res = bgenv_set(handle, "kernelparams", NULL, "root=", 6);
+ ck_assert_int_eq(res, 0);
+
+ char *kparm = str16to8(buffer, handle->data->kernelparams);
+
+ ck_assert(strcmp(kparm, "root=") == 0);
+
+ res = bgenv_set(handle, "ustate", NULL, "2", 2);
+ ck_assert_int_eq(res, 0);
+
+ ck_assert_int_eq(handle->data->ustate, 2);
+
+ res = bgenv_set(handle, "revision", NULL, "0", 2);
+ ck_assert_int_eq(res, 0);
+ ck_assert_int_eq(handle->data->revision, 0);
+
+ res = bgenv_set(handle, "revision", NULL, "10301", 6);
+ ck_assert_int_eq(res, 0);
+ ck_assert_int_eq(handle->data->revision, 10301);
+
+ free(handle);
+}
+END_TEST
+
+START_TEST(ebgenv_api_internal_uservars)
+{
+
+
+}
+END_TEST
+
+Suite *ebg_test_suite(void)
+{
+ Suite *s;
+ TCase *tc_core;
+
+ s = suite_create("ebgenv_api");
+
+ TFun tfuncs[] = {
+ ebgenv_api_internal_strXtoY,
+ ebgenv_api_internal_bgenv_str2enum,
+ ebgenv_api_internal_bgenv_open_by_index,
+ ebgenv_api_internal_bgenv_open_oldest,
+ ebgenv_api_internal_bgenv_open_latest,
+ ebgenv_api_internal_bgenv_write,
+ ebgenv_api_internal_bgenv_read,
+ ebgenv_api_internal_bgenv_create_new,
+ ebgenv_api_internal_bgenv_get,
+ ebgenv_api_internal_bgenv_set,
+ ebgenv_api_internal_uservars
+ };
+
+ tc_core = tcase_create("Core");
+
+ for (int i = 0; i < sizeof(tfuncs)/sizeof(void *); i++) {
+ tcase_add_test(tc_core, tfuncs[i]);
+ }
+
+ suite_add_tcase(s, tc_core);
+
+ return s;
+}
diff --git a/tools/tests/test_main.c b/tools/tests/test_main.c
new file mode 100644
index 0000000..a25e092
--- /dev/null
+++ b/tools/tests/test_main.c
@@ -0,0 +1,34 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Andreas Reichel <andreas.r...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#include <stdlib.h>
+#include <check.h>
+#include <fff.h>
+
+extern Suite *ebg_test_suite(void);
+
+int main(void)
+{
+ int number_failed;
+
+ Suite *s;
+ SRunner *sr;
+
+ s = ebg_test_suite();
+ sr = srunner_create(s);
+
+ srunner_run_all(sr, CK_NORMAL);
+ number_failed = srunner_ntests_failed(sr);
+ srunner_free(sr);
+
+ return (number_failed == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
+}
diff --git a/tools/tests/test_probe_config_file.c b/tools/tests/test_probe_config_file.c
new file mode 100644
index 0000000..edc8f6a
--- /dev/null
+++ b/tools/tests/test_probe_config_file.c
@@ -0,0 +1,202 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Andreas Reichel <andreas.r...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#ifndef ENV_NUM_CONFIG_PARTITIONS
+# define ENV_NUM_CONFIG_PARTITIONS 2
+#endif
+
+#include <stdlib.h>
+#include <errno.h>
+#include <sys/queue.h>
+#include <check.h>
+#include <fff.h>
+#include <env_api.h>
+#include <env_config_file.h>
+#include <env_config_partitions.h>
+#include <ebgpart.h>
+#include <stdio.h>
+#include <env_disk_utils.h>
+#include <fake_devices.h>
+
+DEFINE_FFF_GLOBALS;
+
+Suite *ebg_test_suite(void);
+
+char *get_mountpoint_custom_fake(char *devpath);
+void delete_temp_files(void);
+
+bool __wrap_probe_config_file(CONFIG_PART *);
+bool __real_probe_config_file(CONFIG_PART *);
+int probe_config_file_call_count;
+
+char *fake_mountpoint = "/tmp/tmp.XXXXXX";
+
+struct stailhead *headp;
+struct fake_env_file_path {
+ char *path;
+ STAILQ_ENTRY(fake_env_file_path) fake_env_file_paths;
+};
+STAILQ_HEAD(stailhead, fake_env_file_path) head = STAILQ_HEAD_INITIALIZER(head);
+
+char *get_mountpoint_custom_fake(char *devpath)
+{
+ char *buff = NULL;
+ char *tmpdir = NULL;
+ char *tmpfile = NULL;
+
+ if (asprintf(&tmpdir, "%s", fake_mountpoint) == -1)
+ {
+ tmpdir = NULL;
+ goto fake_mountpoint_error;
+ }
+
+ tmpdir = mkdtemp(tmpdir);
+ if (!tmpdir)
+ goto fake_mountpoint_error;
+
+ if (asprintf(&buff, "%s", tmpdir) == -1) {
+ buff = NULL;
+ goto fake_mountpoint_error;
+ }
+
+ if (asprintf(&tmpfile, "%s/%s", tmpdir, FAT_ENV_FILENAME) == -1) {
+ tmpfile = NULL;
+ goto fake_mountpoint_error;
+ }
+
+ /* create a fake environment file
+ */
+ FILE *temp_env_file = fopen(tmpfile, "w");
+
+ BG_ENVDATA env_data;
+ memset(&env_data, 0, sizeof(BG_ENVDATA));
+ fwrite(&env_data, sizeof(BG_ENVDATA), 1, temp_env_file);
+ fclose(temp_env_file);
+
+ free(tmpfile);
+ free(tmpdir);
+
+ struct fake_env_file_path *fefp;
+ fefp = malloc(sizeof(struct fake_env_file_path));
+
+ /* If possibly store created temporary files and paths in a list to
+ * tidy up later. If not, the test should not fail because of this.
+ */
+ char *buffer_copy;
+ asprintf(&buffer_copy, "%s", buff);
+
+ if (fefp && buffer_copy) {
+ fefp->path = buffer_copy;
+ STAILQ_INSERT_TAIL(&head, fefp, fake_env_file_paths);
+ }
+ return buff;
+
+fake_mountpoint_error:
+ free(buff);
+ free(tmpdir);
+ return NULL;
+}
+
+bool __wrap_probe_config_file(CONFIG_PART *cp)
+{
+ bool ret;
+ probe_config_file_call_count++;
+
+ FILE *test_file = fopen("./test.txt", "a+");
+ if (!test_file)
+ return true;
+
+ fprintf(test_file, "probe_config_file(%s)\n", cp->devpath);
+ fclose(test_file);
+
+ if (asprintf(&cp->mountpoint, "tmpdir") == -1)
+ {
+ cp->not_mounted = true;
+ cp->mountpoint = NULL;
+ return false;
+ }
+ cp->not_mounted = false;
+ ret = __real_probe_config_file(cp);
+
+ free(cp->mountpoint);
+ return ret;
+}
+
+void delete_temp_files(void)
+{
+ char *buffer;
+ while (!STAILQ_EMPTY(&head)) {
+ struct fake_env_file_path *fefp = STAILQ_FIRST(&head);
+
+ if (asprintf(&buffer, "%s/BGENV.DAT", fefp->path) != -1) {
+ remove(buffer);
+ free(buffer);
+ }
+ rmdir(fefp->path);
+ free(fefp->path);
+
+ STAILQ_REMOVE_HEAD(&head, fake_env_file_paths);
+ free(fefp);
+ }
+}
+
+FAKE_VOID_FUNC(ped_device_probe_all);
+FAKE_VALUE_FUNC(PedDevice *, ped_device_get_next, const PedDevice *);
+FAKE_VALUE_FUNC(char *, get_mountpoint, char *);
+
+START_TEST(env_api_fat_test_probe_config_file)
+{
+ bool result;
+
+ RESET_FAKE(ped_device_probe_all);
+ RESET_FAKE(ped_device_get_next);
+ RESET_FAKE(get_mountpoint);
+
+ allocate_fake_devices(1);
+
+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {
+ add_fake_partition(0);
+ }
+
+ ped_device_get_next_fake.custom_fake = ped_device_get_next_custom_fake;
+ get_mountpoint_fake.custom_fake = get_mountpoint_custom_fake;
+ probe_config_file_call_count = 0;
+
+ STAILQ_INIT(&head);
+
+ result = bgenv_init();
+
+ delete_temp_files();
+
+ free_fake_devices();
+
+ ck_assert_int_eq(ped_device_probe_all_fake.call_count, 1);
+ ck_assert_int_eq(ped_device_get_next_fake.call_count, 2);
+ ck_assert_int_eq(probe_config_file_call_count, ENV_NUM_CONFIG_PARTS);
+ ck_assert_int_eq(get_mountpoint_fake.call_count, ENV_NUM_CONFIG_PARTS);
+ ck_assert(result == true);
+}
+END_TEST
+
+Suite *ebg_test_suite(void)
+{
+ Suite *s;
+ TCase *tc_core;
+
+ s = suite_create("env_api_fat");
+
+ tc_core = tcase_create("Core");
+ tcase_add_test(tc_core, env_api_fat_test_probe_config_file);
+ suite_add_tcase(s, tc_core);
+
+ return s;
+}
diff --git a/tools/tests/test_probe_config_partitions.c b/tools/tests/test_probe_config_partitions.c
new file mode 100644
index 0000000..8dacf14
--- /dev/null
+++ b/tools/tests/test_probe_config_partitions.c
@@ -0,0 +1,99 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Andreas Reichel <andreas.r...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#ifndef ENV_NUM_CONFIG_PARTITIONS
+# define ENV_NUM_CONFIG_PARTITIONS 2
+#endif
+
+#include <stdlib.h>
+#include <check.h>
+#include <fff.h>
+#include <env_api.h>
+#include <env_config_file.h>
+#include <env_config_partitions.h>
+#include <fake_devices.h>
+
+DEFINE_FFF_GLOBALS;
+
+Suite *ebg_test_suite(void);
+
+bool read_env_custom_fake(CONFIG_PART *cp, BG_ENVDATA *env);
+bool read_env(CONFIG_PART *part, BG_ENVDATA *env);
+
+bool read_env_custom_fake(CONFIG_PART *cp, BG_ENVDATA *env)
+{
+ if (!env) {
+ return false;
+ }
+ memset(env, 0, sizeof(BG_ENVDATA));
+ return true;
+}
+
+FAKE_VALUE_FUNC(bool, read_env, CONFIG_PART *, BG_ENVDATA *);
+FAKE_VOID_FUNC(ped_device_probe_all);
+FAKE_VALUE_FUNC(PedDevice *, ped_device_get_next, const PedDevice *);
+
+START_TEST(env_api_fat_test_probe_config_partitions)
+{
+ bool result;
+ /* In this unit test, contents of environment data are
+ * faked to be all zero
+ */
+
+ /* Test if bgenv_init fails if no block devices are found
+ */
+ RESET_FAKE(ped_device_probe_all);
+ RESET_FAKE(ped_device_get_next);
+
+ ped_device_get_next_fake.return_val = NULL;
+
+ result = bgenv_init();
+
+ ck_assert(ped_device_probe_all_fake.call_count == 1);
+ ck_assert(result == false);
+
+ /* Test if bgenv_init fails if a device with two partitions is found
+ * but now config file is there
+ */
+ RESET_FAKE(ped_device_probe_all);
+ RESET_FAKE(ped_device_get_next);
+
+ allocate_fake_devices(1);
+
+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {
+ add_fake_partition(0);
+ }
+
+ ped_device_get_next_fake.custom_fake = ped_device_get_next_custom_fake;
+ result = bgenv_init();
+
+ free_fake_devices();
+
+ ck_assert(ped_device_probe_all_fake.call_count == 1);
+ ck_assert(ped_device_get_next_fake.call_count == 2);
+ ck_assert(result == false);
+}
+END_TEST
+
+Suite *ebg_test_suite(void)
+{
+ Suite *s;
+ TCase *tc_core;
+
+ s = suite_create("env_api_fat");
+
+ tc_core = tcase_create("Core");
+ tcase_add_test(tc_core, env_api_fat_test_probe_config_partitions);
+ suite_add_tcase(s, tc_core);
+
+ return s;
+}
--
2.14.2

Claudius Heine

unread,
Oct 24, 2017, 10:04:55 AM10/24/17
to Andreas J. Reichel, efibootg...@googlegroups.com
Hi Andreas,
So this patch on its own would call apt-get install without any package
argument?

Claudius
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: c...@denx.de

PGP key: 6FF2 E59F 00C6 BC28 31D8 64C1 1173 CB19 9808 B153
Keyserver: hkp://pool.sks-keyservers.net
signature.asc

Claudius Heine

unread,
Oct 24, 2017, 10:07:35 AM10/24/17
to Andreas J. Reichel, efibootg...@googlegroups.com
On Tue, 2017-10-24 at 14:26 +0200, Andreas J. Reichel wrote:
That only needed for running the tests right? So its not a 'compile'
dependency.

Claudius

> ```
>
> ### Debian 8 ###
>
> ```
> -apt-get install gnu-efi libpci-dev
> +apt-get install gnu-efi libpci-dev check
> ```
>
> ## Compilation ##
> --
> 2.14.2
>
signature.asc

Claudius Heine

unread,
Oct 24, 2017, 10:08:49 AM10/24/17
to Andreas J. Reichel, efibootg...@googlegroups.com
Hi,

On Tue, 2017-10-24 at 14:26 +0200, Andreas J. Reichel wrote:
> From: Andreas Reichel <andreas.r...@siemens.com>
>
> Use function faking framework for mocking in unit tests.
> Integrate it into the project as git submodule.
>
> Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
> ---
> .gitmodules | 3 +++
> fff | 1 +
> tools/tests/Makefile.am | 3 ++-
> 3 files changed, 6 insertions(+), 1 deletion(-)
> create mode 100644 .gitmodules
> create mode 160000 fff
>
> diff --git a/.gitmodules b/.gitmodules
> new file mode 100644
> index 0000000..971d7e7
> --- /dev/null
> +++ b/.gitmodules
> @@ -0,0 +1,3 @@
> +[submodule "fff"]
> + path = fff
> + url = https://github.com/meekrosoft/fff

Maybe third_party/fff is a better place to put this?

Claudius

> diff --git a/fff b/fff
> new file mode 160000
> index 0000000..719dd8b
> --- /dev/null
> +++ b/fff
> @@ -0,0 +1 @@
> +Subproject commit 719dd8b6a39a34d0dbf71eeccd4bcd8c5e84c9b4
> diff --git a/tools/tests/Makefile.am b/tools/tests/Makefile.am
> index ec96981..30dbb85 100644
> --- a/tools/tests/Makefile.am
> +++ b/tools/tests/Makefile.am
> @@ -15,7 +15,8 @@ OBJCOPY ?= objcopy
> AM_CPPFLAGS = \
> -I$(top_srcdir)/include \
> -I$(top_srcdir)/swupdate-adapter \
> - -I$(top_srcdir)/tools
> + -I$(top_srcdir)/tools \
> + -I$(top_srcdir)/fff
>
> AM_CFLAGS = \
> -Wno-unused-parameter \
> --
> 2.14.2
>
signature.asc

Claudius Heine

unread,
Oct 24, 2017, 10:20:20 AM10/24/17
to Andreas J. Reichel, efibootg...@googlegroups.com
Hi,

since we seems to like clean structured patchesets in efibootguard,
this patch could be mostly merged into previous patches of this set.

On Tue, 2017-10-24 at 14:26 +0200, Andreas J. Reichel wrote:
> From: Andreas Reichel <andreas.r...@siemens.com>
>
> Install needed packages into build environment.
>
> Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
> ---
> .travis-build.sh | 30 +++++++++++++++---------------
> 1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/.travis-build.sh b/.travis-build.sh
> index 6e534cb..a7e6beb 100755
> --- a/.travis-build.sh
> +++ b/.travis-build.sh
> @@ -25,22 +25,14 @@ install_common_deps()
>
> install_native_deps()
> {
> - sudo apt-add-repository \
> - 'deb http://archive.ubuntu.com/ubuntu xenial universe'
> - sudo apt-get update -qq

=> patch 1

> sudo apt-get install --no-install-recommends \
> - --target-release xenial check
> + libz-dev check

=> patch 2

> }
>
> install_i586_deps()
> {
> sudo apt-get install --no-install-recommends \
> - libz-dev:i386
> - sudo apt-add-repository \
> - 'deb http://archive.ubuntu.com/ubuntu xenial universe'
> - sudo apt-get update -qq

=> patch 1

> - sudo apt-get install --no-install-recommends \
> - --target-release xenial check:i386
> + libz-dev:i386 check:i386

=> patch 2 and 1

> }
>
> prepare_build()
> @@ -86,14 +78,17 @@ case "$TARGET_EFFECTIVE" in
> install_i586_deps
> prepare_build
> enter_build
> + export PKG_CONFIG_DIR=
> + export PKG_CONFIG_PATH=/usr/lib/i386-linux-gnu/pkgconfig
> + export PKG_CONFIG_LIBDIR=/usr/lib/i386-linux-gnu

=> patch 2

> ../configure --with-gnuefi-lib-dir=/usr/lib32 CFLAGS=-m32 \
> host_alias=i586-linux
> exec make check
> ;;
>
> cppcheck)
> - install_common_deps
> - install_native_deps
> + install_common_deps
> + install_native_deps

=> new fix style patch?

> echo "Building and installing cppcheck..."
> if ! install_cppcheck >cppcheck_build.log 2>&1
> then
> @@ -103,10 +98,15 @@ case "$TARGET_EFFECTIVE" in
> prepare_build
> ./configure
>
> + # Ignore directories as they are not part of the project
> + ignore=""
> + ignore+=" -i fff"
> +

=> patch 3

> suppress=""
> # Justified suppressions:
> - # Not part of the project:
> - suppress+=" --
> suppress=variableScope:/usr/include/bits/stdlib-bsearch.h"
> + # Does not belong to the project
> + suppress+=" --suppress=*:/usr/include/*"
> + suppress+=" --suppress=*:/usr/include/bits/*"

=> patch 2 or 3 depending on what triggers these new supressions.

> # Function 'efi_main' is called by efi:
> suppress+=" --suppress=unusedFunction:main.c"
> # Some functions are defined for API only
> @@ -137,7 +137,7 @@ case "$TARGET_EFFECTIVE" in
> # Exit code '1' is returned if arguments are not valid or if
> no input
> # files are provided. Compare 'cppcheck --help'.
> exec cppcheck -f -q --error-exitcode=2 \
> - $enable $suppress $cpp_conf $includes .
> + $enable $suppress $ignore $cpp_conf $includes .

=> patch 3
> ;;
> coverity_prepare)
> install_common_deps
> --
> 2.14.2
>

It is good practice to try to fix broken stuff in the patches that
break it within one patchset.

Cheers,
Claudius
signature.asc

Claudius Heine

unread,
Oct 24, 2017, 10:27:58 AM10/24/17
to Andreas J. Reichel, efibootg...@googlegroups.com
Hi,

On Tue, 2017-10-24 at 14:26 +0200, Andreas J. Reichel wrote:
> From: Andreas Reichel <andreas.r...@siemens.com>
>
> For unit test support, code must be more modular.
> This also avoids inline functions and static functions where
> possible.

I skip reviewing this patch because I assume this patch does not modify
any code, just moves it into separate files? Otherwise I would rather
see one patch that changes stuff and one patch that moves stuff.

If you removed 'static' keywords, them maybe do it in a separate patch.
signature.asc

Claudius Heine

unread,
Oct 24, 2017, 11:23:17 AM10/24/17
to Andreas J. Reichel, efibootg...@googlegroups.com
On Tue, 2017-10-24 at 14:26 +0200, Andreas J. Reichel wrote:
> From: Andreas Reichel <andreas.r...@siemens.com>
>
> The new testcode initially provides tests for
> * internal userspace API core functions
> * external userspace API functions (libebgenv.a)
>
> They are devided into the following test-suits:
>
> * test_bgenv_init_retval.c, where partition probing must be
> faked and disks must be simualted.
> * test_probe_config_file.c, where the existence of environment
> data is faked for simulated config partitions
> * test_ebgenv_api_internal.c, core functions for userspace API
> * test_ebgenv_api.c, library API functions for userspace

This is a pretty large patch. Maybe split this patch up a bit? One
patch for the empty test setup (test_main and Makefile.am), one for
utility functions (fake_devices), and then one patch for each set of
tests.

Not sure about the removal of the static. I see two options, one patch
that removes all the 'static' or remove the 'static' within the patches
that use those functions.

Makes it easier to review.
Why is that needed? What has changed that CPPFLAGS are now CFLAGS? I
don't suppose anything in this patch changed this, so maybe move this
change into one of previous patches of this patch set.

> -Wno-unused-parameter \
> -Wmissing-prototypes \
> -fshort-wchar \
> @@ -26,11 +25,54 @@ AM_CFLAGS = \
> -D_GNU_SOURCE \
> -g
>
> +libtest_env_api_fat_a_SRC = \
> + ../../env/env_api.c \
> + ../../env/env_api_fat.c \
> + ../../tools/ebgpart.c \
> + ../../env/env_config_file.c \
> + ../../env/env_config_partitions.c \
> + ../../env/env_disk_utils.c \
> + ../../env/uservars.c
> +

If i understand it correctly, you are compiling the env api static
library with the fat backend here and then link to it via the test
cases.

I thought about your argument that you want to support the check of
multiple different configurations (backends) with one 'make check' and
I currently think that you should not bother about this at this level.
The intention of 'make check' is to test if the current configuration
is working and not if any other configuration works, so this should be
delegated to the CI tool.

Were there any other arguments for this?

> CLEANFILES =
>
> -test_api:
> - ln -sf /bin/true test_api
> +noinst_LIBRARIES = libtest_env_api_fat.a
> +libtest_env_api_fat_a_SOURCES = $(libtest_env_api_fat_a_SRC)
> +
> +libenvapi_testlib_fat.a: libtest_env_api_fat.a
> + $(OBJCOPY) --weaken $^ $@

;)

So libenvapi_testlib_fat.a is the same as libtest_env_api_fat.a but
with weakend symbols right? Why not call it
libtest_env_api_fat.weakend.a ? (Or if you remove the build of
'libtest_env_api_fat.a' call it 'libebgenv.weakend.a'.
Hmm... Now when I read this I occurs to me that you should maybe put
includes for those other 'env_config*' stuff into the 'env_api.h'. Not
really good if you need to import 3 files just to have the complete ebg
api.

Thats it for now. Will look at the rest later.

Cheers,
Claudius
signature.asc

Andreas Reichel

unread,
Oct 24, 2017, 11:35:29 AM10/24/17
to Claudius Heine, efibootg...@googlegroups.com
No, not right. Because make will compile the tests and fail without
check.

Andreas

> Claudius
>
> > ```
> >
> > ### Debian 8 ###
> >
> > ```
> > -apt-get install gnu-efi libpci-dev
> > +apt-get install gnu-efi libpci-dev check
> > ```
> >
> > ## Compilation ##
> > --
> > 2.14.2
> >
> --
> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: c...@denx.de
>
> PGP key: 6FF2 E59F 00C6 BC28 31D8 64C1 1173 CB19 9808 B153
> Keyserver: hkp://pool.sks-keyservers.net



--
Andreas Reichel
Dipl.-Phys. (Univ.)
Software Consultant

Andreas...@tngtech.com, +49-174-3180074
TNG Technology Consulting GmbH, Betastr. 13a, 85774 Unterfoehring
Geschaeftsfuehrer: Henrik Klagges, Dr. Robert Dahlke, Gerhard Mueller
Sitz: Unterfoehring * Amtsgericht Muenchen * HRB 135082

Andreas Reichel

unread,
Oct 24, 2017, 11:36:11 AM10/24/17
to Claudius Heine, efibootg...@googlegroups.com
Thanks, overlooked this one.
> --
> You received this message because you are subscribed to the Google Groups "EFI Boot Guard" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to efibootguard-d...@googlegroups.com.
> To post to this group, send email to efibootg...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/efibootguard-dev/1508853882.13007.22.camel%40denx.de.
> For more options, visit https://groups.google.com/d/optout.

Andreas Reichel

unread,
Oct 24, 2017, 11:41:57 AM10/24/17
to Claudius Heine, efibootg...@googlegroups.com
On Tue, Oct 24, 2017 at 04:08:42PM +0200, Claudius Heine wrote:
> Hi,
>
> On Tue, 2017-10-24 at 14:26 +0200, Andreas J. Reichel wrote:
> > From: Andreas Reichel <andreas.r...@siemens.com>
> >
> > Use function faking framework for mocking in unit tests.
> > Integrate it into the project as git submodule.
> >
> > Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
> > ---
> > .gitmodules | 3 +++
> > fff | 1 +
> > tools/tests/Makefile.am | 3 ++-
> > 3 files changed, 6 insertions(+), 1 deletion(-)
> > create mode 100644 .gitmodules
> > create mode 160000 fff
> >
> > diff --git a/.gitmodules b/.gitmodules
> > new file mode 100644
> > index 0000000..971d7e7
> > --- /dev/null
> > +++ b/.gitmodules
> > @@ -0,0 +1,3 @@
> > +[submodule "fff"]
> > + path = fff
> > + url = https://github.com/meekrosoft/fff
>
> Maybe third_party/fff is a better place to put this?
>
If you like this better... Jan what do you think?

Andreas

Andreas Reichel

unread,
Oct 24, 2017, 11:43:50 AM10/24/17
to Claudius Heine, efibootg...@googlegroups.com
On Tue, Oct 24, 2017 at 04:27:55PM +0200, Claudius Heine wrote:
> Hi,
>
> On Tue, 2017-10-24 at 14:26 +0200, Andreas J. Reichel wrote:
> > From: Andreas Reichel <andreas.r...@siemens.com>
> >
> > For unit test support, code must be more modular.
> > This also avoids inline functions and static functions where
> > possible.
>
> I skip reviewing this patch because I assume this patch does not modify
> any code, just moves it into separate files? Otherwise I would rather
> see one patch that changes stuff and one patch that moves stuff.
>
> If you removed 'static' keywords, them maybe do it in a separate patch.
>
Sorry this is complete nonsense to me. If I move local code into
separated translation units, it is implicit, that I have to remove
static keywords. There is no use in creating a patch overkill by just
removing static keywoards.

Andreas

> Cheers,
> Claudius
>
> --
> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: c...@denx.de
>
> PGP key: 6FF2 E59F 00C6 BC28 31D8 64C1 1173 CB19 9808 B153
> Keyserver: hkp://pool.sks-keyservers.net



Andreas Reichel

unread,
Oct 24, 2017, 11:47:54 AM10/24/17
to Claudius Heine, efibootg...@googlegroups.com
On Tue, Oct 24, 2017 at 05:23:07PM +0200, Claudius Heine wrote:
> On Tue, 2017-10-24 at 14:26 +0200, Andreas J. Reichel wrote:
> > From: Andreas Reichel <andreas.r...@siemens.com>
> >
> > The new testcode initially provides tests for
> > * internal userspace API core functions
> > * external userspace API functions (libebgenv.a)
> >
> > They are devided into the following test-suits:
> >
> > * test_bgenv_init_retval.c, where partition probing must be
> > faked and disks must be simualted.
> > * test_probe_config_file.c, where the existence of environment
> > data is faked for simulated config partitions
> > * test_ebgenv_api_internal.c, core functions for userspace API
> > * test_ebgenv_api.c, library API functions for userspace
>
> This is a pretty large patch. Maybe split this patch up a bit? One
> patch for the empty test setup (test_main and Makefile.am), one for
> utility functions (fake_devices), and then one patch for each set of
> tests.

Splitting the test code does not make it shorter. If a reviewer reads
2000 lines of test-code in one file or 2000 lines of test-code in
several files... I don't like to have 5 patch files because the test
code has to be applied finally. If it is not okay, I will fix it.

This patch does nothing else but add the tests that are based on the
patches before.


>
> Not sure about the removal of the static. I see two options, one patch
> that removes all the 'static' or remove the 'static' within the patches
> that use those functions.
>
I am quite sure. I remove the static keywoards in this patch, because
the tests need this. And the tests are introduced by this patch. Makes
> --
> You received this message because you are subscribed to the Google Groups "EFI Boot Guard" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to efibootguard-d...@googlegroups.com.
> To post to this group, send email to efibootg...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/efibootguard-dev/1508858587.13007.78.camel%40denx.de.
> For more options, visit https://groups.google.com/d/optout.



Andreas Reichel

unread,
Oct 24, 2017, 11:51:11 AM10/24/17
to Claudius Heine, efibootg...@googlegroups.com
On Tue, Oct 24, 2017 at 04:20:07PM +0200, Claudius Heine wrote:
> Hi,
>
> since we seems to like clean structured patchesets in efibootguard,
> this patch could be mostly merged into previous patches of this set.
Just because you disagree about the separation does not mean,
my patchset is unclean.
It just means, that I have collected all changes that touch the travis
script here and you would like to make travis work with every single
commit.

Claudius Heine

unread,
Oct 24, 2017, 12:20:55 PM10/24/17
to Andreas Reichel, efibootg...@googlegroups.com
Hi Andreas,

On Tue, 2017-10-24 at 17:50 +0200, Andreas Reichel wrote:
> On Tue, Oct 24, 2017 at 04:20:07PM +0200, Claudius Heine wrote:
> > Hi,
> >
> > since we seems to like clean structured patchesets in efibootguard,
> > this patch could be mostly merged into previous patches of this
> > set.
>
> Just because you disagree about the separation does not mean,
> my patchset is unclean.

Ok maybe 'clean' is not the right word because it contains a bias. I
don't disagree with your separation per se. I do it in my own projects
often enough. I just pointed out that there two (at least) different
methods to do these things. 1: patches represent the development
process and 2: patches introduce or change specific features ('cleaned
up', 'structured', 'post processed', ...).

IMO if one patch in a patchset breaks something (in this case travis or
in some other cases the documentation) and then a latter patch fixes
this again than this sounds more like method 1 to me.

I do see arguments to have a separate patch at the end of a patchset
that fixes the documentation, and that might be ok, because that is not
technical. But travis is.
signature.asc

Claudius Heine

unread,
Oct 24, 2017, 12:27:51 PM10/24/17
to Andreas Reichel, efibootg...@googlegroups.com
Hi,

On Tue, 2017-10-24 at 17:42 +0200, Andreas Reichel wrote:
> On Tue, Oct 24, 2017 at 04:27:55PM +0200, Claudius Heine wrote:
> > Hi,
> >
> > On Tue, 2017-10-24 at 14:26 +0200, Andreas J. Reichel wrote:
> > > From: Andreas Reichel <andreas.r...@siemens.com>
> > >
> > > For unit test support, code must be more modular.
> > > This also avoids inline functions and static functions where
> > > possible.
> >
> > I skip reviewing this patch because I assume this patch does not
> > modify
> > any code, just moves it into separate files? Otherwise I would
> > rather
> > see one patch that changes stuff and one patch that moves stuff.
> >
> > If you removed 'static' keywords, them maybe do it in a separate
> > patch.
> >
>
> Sorry this is complete nonsense to me. If I move local code into
> separated translation units, it is implicit, that I have to remove
> static keywords. There is no use in creating a patch overkill by just
> removing static keywoards.

IMO that eases review, does it not? 1 remove statics with a commit
messages that hints to the next patch and 2 split stuff up and don't
change code.

I don't think its good practice to try to have fewer patches by doing
multiple different things in one patch. But I get that you think that
those things aren't two steps and I think they are, so maybe Jan should
decide.
signature.asc

Claudius Heine

unread,
Oct 24, 2017, 5:23:14 PM10/24/17
to Andreas Reichel, efibootg...@googlegroups.com
Hi Andreas,

On Tue, 2017-10-24 at 17:47 +0200, Andreas Reichel wrote:
> On Tue, Oct 24, 2017 at 05:23:07PM +0200, Claudius Heine wrote:
> > On Tue, 2017-10-24 at 14:26 +0200, Andreas J. Reichel wrote:
> > > From: Andreas Reichel <andreas.r...@siemens.com>
> > >
> > > The new testcode initially provides tests for
> > > * internal userspace API core functions
> > > * external userspace API functions (libebgenv.a)
> > >
> > > They are devided into the following test-suits:
> > >
> > > * test_bgenv_init_retval.c, where partition probing must be
> > > faked and disks must be simualted.
> > > * test_probe_config_file.c, where the existence of environment
> > > data is faked for simulated config partitions
> > > * test_ebgenv_api_internal.c, core functions for userspace API
> > > * test_ebgenv_api.c, library API functions for userspace
> >
> > This is a pretty large patch. Maybe split this patch up a bit? One
> > patch for the empty test setup (test_main and Makefile.am), one for
> > utility functions (fake_devices), and then one patch for each set
> > of
> > tests.
>
> Splitting the test code does not make it shorter.

Agree.

> If a reviewer reads
> 2000 lines of test-code in one file or 2000 lines of test-code in
> several files...

Yes? What then?

Reading 2000 lines in several files is easier, because they are sorted
together, hopefully logical/structural. Also multiple patches that each
contain 1 header file and 1 implementation file is easier to read than
1 patch that contains multiple files because you don't have to jump
around much to see how they fit together.

> I don't like to have 5 patch files because the test
> code has to be applied finally. If it is not okay, I will fix it.

I don't understand you here. Could you elaborate?

> This patch does nothing else but add the tests that are based on the
> patches before.

They were rewritten to fit a new test infrastructure, were they not? So
its not just moving code around unchanged? If so they should be
reviewed.

You do know that I do this review not to anger you, but to help
accelerate this process. Jan does not always has time to fully review
everything, so I help you and him by doing some of it. This should
normally lead to a faster patch development cycle.

> >
> > Not sure about the removal of the static. I see two options, one
> > patch
> > that removes all the 'static' or remove the 'static' within the
> > patches
> > that use those functions.
> >
>
> I am quite sure. I remove the static keywoards in this patch, because
> the tests need this. And the tests are introduced by this patch.
> Makes
> it easier to review.

So you go with option 2 then. But if you split this patch up, then it
might be more work for you. Just saying...

Also I have some more review comments in the rest of the last mail that
might have gone unnoticed.

I will try to get through the rest of it at a later date.

Cheers,
Claudius
signature.asc

Andreas Reichel

unread,
Oct 25, 2017, 4:50:31 AM10/25/17
to Claudius Heine, efibootg...@googlegroups.com
I personally do not get this. For me it is always easier to see
everything in place instead of split up. But if you like it split up I
can split it up...

> > I don't like to have 5 patch files because the test
> > code has to be applied finally. If it is not okay, I will fix it.
>
> I don't understand you here. Could you elaborate?
>
It happens that patch series are applied partially. That was another
thought. But I'm okay with redoing this.

> > This patch does nothing else but add the tests that are based on the
> > patches before.
>
> They were rewritten to fit a new test infrastructure, were they not? So
> its not just moving code around unchanged? If so they should be
> reviewed.
>
Add code means "add new code". Of course, please.

> You do know that I do this review not to anger you, but to help
> accelerate this process. Jan does not always has time to fully review

I am neither upset nor angry nor anything else... I just do not agree
with some opinions :) If I would, there were no discussions and
solutions are never optimal without discussions.
I am always thankful for your reviews. But that does not change my
technical thoughts on certain things.

> everything, so I help you and him by doing some of it. This should
> normally lead to a faster patch development cycle.
>
> > >
> > > Not sure about the removal of the static. I see two options, one
> > > patch
> > > that removes all the 'static' or remove the 'static' within the
> > > patches
> > > that use those functions.
> > >
> >
> > I am quite sure. I remove the static keywoards in this patch, because
> > the tests need this. And the tests are introduced by this patch.
> > Makes
> > it easier to review.
>
> So you go with option 2 then. But if you split this patch up, then it
> might be more work for you. Just saying...
>
> Also I have some more review comments in the rest of the last mail that
> might have gone unnoticed.
>
Just a note: If I do not answer something, I usually agree or have not
yet come to an opinion.

> I will try to get through the rest of it at a later date.
>
Thank you!

Andreas

> Cheers,
> Claudius
>
> --
> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: c...@denx.de
>
> PGP key: 6FF2 E59F 00C6 BC28 31D8 64C1 1173 CB19 9808 B153
> Keyserver: hkp://pool.sks-keyservers.net



Andreas Reichel

unread,
Oct 25, 2017, 4:56:11 AM10/25/17
to Claudius Heine, efibootg...@googlegroups.com
On Tue, Oct 24, 2017 at 06:20:49PM +0200, Claudius Heine wrote:
> Hi Andreas,
>
> On Tue, 2017-10-24 at 17:50 +0200, Andreas Reichel wrote:
> > On Tue, Oct 24, 2017 at 04:20:07PM +0200, Claudius Heine wrote:
> > > Hi,
> > >
> > > since we seems to like clean structured patchesets in efibootguard,
> > > this patch could be mostly merged into previous patches of this
> > > set.
> >
> > Just because you disagree about the separation does not mean,
> > my patchset is unclean.
>
> Ok maybe 'clean' is not the right word because it contains a bias. I
> don't disagree with your separation per se. I do it in my own projects
> often enough. I just pointed out that there two (at least) different
> methods to do these things. 1: patches represent the development
> process and 2: patches introduce or change specific features ('cleaned
> up', 'structured', 'post processed', ...).
>
> IMO if one patch in a patchset breaks something (in this case travis or
> in some other cases the documentation) and then a latter patch fixes
> this again than this sounds more like method 1 to me.
>
Partially agree here (look below)... it is something between code
changes and documentation updates... For documentation purposes, also
collected updates were accepted in the past. However, strictly using
method 2, also the documentation would have to be updated strictly with
each code change and not just in the end of a series.

> I do see arguments to have a separate patch at the end of a patchset
> that fixes the documentation, and that might be ok, because that is not
> technical. But travis is.
>
My thought was, that neither the build itself, nor the functioning of
the software is disturbed but this sorting of the patches. And also,
there are no Travis builds of separate commits usually, because I would
have applied the patches, compiled the code, tested it and then uploaded
the branch. But I see your point to keep a strict line within every
single and smallest feature.

> Claudius
>
> --
> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: c...@denx.de
>
> PGP key: 6FF2 E59F 00C6 BC28 31D8 64C1 1173 CB19 9808 B153
> Keyserver: hkp://pool.sks-keyservers.net



Andreas Reichel

unread,
Oct 25, 2017, 4:57:43 AM10/25/17
to Claudius Heine, efibootg...@googlegroups.com
On Tue, Oct 24, 2017 at 06:27:48PM +0200, Claudius Heine wrote:
> Hi,
>
> On Tue, 2017-10-24 at 17:42 +0200, Andreas Reichel wrote:
> > On Tue, Oct 24, 2017 at 04:27:55PM +0200, Claudius Heine wrote:
> > > Hi,
> > >
> > > On Tue, 2017-10-24 at 14:26 +0200, Andreas J. Reichel wrote:
> > > > From: Andreas Reichel <andreas.r...@siemens.com>
> > > >
> > > > For unit test support, code must be more modular.
> > > > This also avoids inline functions and static functions where
> > > > possible.
> > >
> > > I skip reviewing this patch because I assume this patch does not
> > > modify
> > > any code, just moves it into separate files? Otherwise I would
> > > rather
> > > see one patch that changes stuff and one patch that moves stuff.
> > >
> > > If you removed 'static' keywords, them maybe do it in a separate
> > > patch.
> > >
> >
> > Sorry this is complete nonsense to me. If I move local code into
> > separated translation units, it is implicit, that I have to remove
> > static keywords. There is no use in creating a patch overkill by just
> > removing static keywoards.
>
> IMO that eases review, does it not? 1 remove statics with a commit
> messages that hints to the next patch and 2 split stuff up and don't
> change code.
>
Removing static keywoards is nothing that can break code and nothing
that needs deeper review.

> I don't think its good practice to try to have fewer patches by doing
> multiple different things in one patch. But I get that you think that
> those things aren't two steps and I think they are, so maybe Jan should
> decide.
These ARE things belonging together here :) These are not different things.

Yes, Jan please decide :)

Andreas
>
> Claudius
> --
> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: c...@denx.de
>
> PGP key: 6FF2 E59F 00C6 BC28 31D8 64C1 1173 CB19 9808 B153
> Keyserver: hkp://pool.sks-keyservers.net
>
> --
> You received this message because you are subscribed to the Google Groups "EFI Boot Guard" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to efibootguard-d...@googlegroups.com.
> To post to this group, send email to efibootg...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/efibootguard-dev/1508862468.13007.95.camel%40denx.de.
> For more options, visit https://groups.google.com/d/optout.



Jan Kiszka

unread,
Oct 30, 2017, 11:53:53 AM10/30/17
to [ext] Andreas Reichel, Claudius Heine, efibootg...@googlegroups.com
I would prefer to have a correlation with the use case of this:
something like "testing/fff" or - if this will remain tools-only - even
tools/tests/fff.

Jan

--
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

Jan Kiszka

unread,
Oct 30, 2017, 12:30:14 PM10/30/17
to [ext] Andreas Reichel, Claudius Heine, efibootg...@googlegroups.com
Assuming that the patch is really only removing 'static' from the
functions when moving them around, that is fine.

Andreas J. Reichel

unread,
Nov 2, 2017, 11:57:40 AM11/2/17
to efibootg...@googlegroups.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>

This patch series contains a complete replacement for the current
test-suite, because cmocka is not compatible with efibootguard's
license.

Instead, fff (fake function framework) and check is used.
* https://github.com/meekrosoft/fff (MIT license)
* https://libcheck.github.io/check (LGPL)

Additional tests for user variables are included in the
upcoming patch series after this one has been merged.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>

Andreas Reichel (9):
Remove cmocka test suite
Add libcheck unit testing framework
Add function faking framework as submodule
Make code more modular
tests: Add test for bgenv_init
tests: Add test for config partition probing
tests: Add test for config file probing
tests: Add tests for internal environment API functions
tests: Add tests for environment API

.gitmodules | 3 +
.travis-build.sh | 32 +-
Makefile.am | 4 +-
configure.ac | 3 +
docs/COMPILE.md | 4 +-
env/env_api_fat.c | 230 +----------
env/env_config_file.c | 90 ++++
env/env_config_partitions.c | 83 ++++
env/env_disk_utils.c | 99 +++++
include/env_config_file.h | 20 +
include/env_config_partitions.h | 18 +
include/env_disk_utils.h | 20 +
include/test-interface.h | 2 +
testing/fff | 1 +
tools/tests/Makefile.am | 67 ++-
tools/tests/fake_devices.c | 94 +++++
tools/tests/fake_devices.h | 28 ++
tools/tests/test_api.c | 315 --------------
tools/tests/test_bgenv_init_retval.c | 96 +++++
tools/tests/test_ebgenv_api.c | 635 +++++++++++++++++++++++++++++
tools/tests/test_ebgenv_api_internal.c | 403 ++++++++++++++++++
tools/tests/test_environment.c | 119 ------
tools/tests/test_main.c | 34 ++
tools/tests/test_partitions.c | 154 -------
tools/tests/test_probe_config_file.c | 191 +++++++++
tools/tests/test_probe_config_partitions.c | 95 +++++
26 files changed, 1995 insertions(+), 845 deletions(-)
create mode 100644 .gitmodules
create mode 100644 env/env_config_file.c
create mode 100644 env/env_config_partitions.c
create mode 100644 env/env_disk_utils.c
create mode 100644 include/env_config_file.h
create mode 100644 include/env_config_partitions.h
create mode 100644 include/env_disk_utils.h
create mode 160000 testing/fff
create mode 100644 tools/tests/fake_devices.c
create mode 100644 tools/tests/fake_devices.h
delete mode 100644 tools/tests/test_api.c
create mode 100644 tools/tests/test_bgenv_init_retval.c
create mode 100644 tools/tests/test_ebgenv_api.c
create mode 100644 tools/tests/test_ebgenv_api_internal.c
delete mode 100644 tools/tests/test_environment.c
create mode 100644 tools/tests/test_main.c
delete mode 100644 tools/tests/test_partitions.c
create mode 100644 tools/tests/test_probe_config_file.c
create mode 100644 tools/tests/test_probe_config_partitions.c

--
2.14.2

Andreas J. Reichel

unread,
Nov 2, 2017, 11:57:40 AM11/2/17
to efibootg...@googlegroups.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>

Test all everything provided by environment API core functions except
user variables.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---
tools/tests/Makefile.am | 6 +-
tools/tests/test_ebgenv_api_internal.c | 403 +++++++++++++++++++++++++++++++++
2 files changed, 408 insertions(+), 1 deletion(-)
create mode 100644 tools/tests/test_ebgenv_api_internal.c

diff --git a/tools/tests/Makefile.am b/tools/tests/Makefile.am
index 957a044..0d21673 100644
--- a/tools/tests/Makefile.am
+++ b/tools/tests/Makefile.am
@@ -46,7 +46,8 @@ libenvapi_testlib_fat.a: libtest_env_api_fat.a

check_PROGRAMS = test_bgenv_init_retval \
test_probe_config_partitions \
- test_probe_config_file
+ test_probe_config_file \
+ test_ebgenv_api_internal

FAT_TESTLIB=libenvapi_testlib_fat.a

@@ -67,5 +68,8 @@ test_probe_config_file_SOURCES = test_probe_config_file.c fake_devices.c \
$(SRC_TEST_COMMON)
test_probe_config_file_LDADD = $(FAT_TESTLIB) $(LIBCHECK_LIBS)

+test_ebgenv_api_internal_CFLAGS = $(AM_CFLAGS)
+test_ebgenv_api_internal_SOURCES = test_ebgenv_api_internal.c $(SRC_TEST_COMMON)
+test_ebgenv_api_internal_LDADD = $(FAT_TESTLIB) $(LIBCHECK_LIBS)

TESTS = $(check_PROGRAMS)
diff --git a/tools/tests/test_ebgenv_api_internal.c b/tools/tests/test_ebgenv_api_internal.c
new file mode 100644
index 0000000..1f2995b
--- /dev/null
+++ b/tools/tests/test_ebgenv_api_internal.c
@@ -0,0 +1,403 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Andreas Reichel <andreas.r...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#include <stdlib.h>
+#include <check.h>
+#include <fff.h>
+Suite *ebg_test_suite(void)
+{
+ Suite *s;
+ TCase *tc_core;
+
+ s = suite_create("ebgenv_api");
+
+ TFun tfuncs[] = {
+ ebgenv_api_internal_strXtoY,
+ ebgenv_api_internal_bgenv_str2enum,
+ ebgenv_api_internal_bgenv_open_by_index,
+ ebgenv_api_internal_bgenv_open_oldest,
+ ebgenv_api_internal_bgenv_open_latest,
+ ebgenv_api_internal_bgenv_write,
+ ebgenv_api_internal_bgenv_read,
+ ebgenv_api_internal_bgenv_create_new,
+ ebgenv_api_internal_bgenv_get,
+ ebgenv_api_internal_bgenv_set
+ };
+
+ tc_core = tcase_create("Core");
+
+ for (int i = 0; i < sizeof(tfuncs)/sizeof(void *); i++) {
+ tcase_add_test(tc_core, tfuncs[i]);
+ }
+
+ suite_add_tcase(s, tc_core);
+
+ return s;
+}
--
2.14.2

Andreas J. Reichel

unread,
Nov 2, 2017, 11:57:40 AM11/2/17
to efibootg...@googlegroups.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>

Due to license incompatbility, remove cmocka from project.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---
.travis-build.sh | 11 +-
tools/tests/Makefile.am | 22 +--
tools/tests/test_api.c | 315 -----------------------------------------
tools/tests/test_environment.c | 119 ----------------
tools/tests/test_partitions.c | 154 --------------------
5 files changed, 5 insertions(+), 616 deletions(-)
delete mode 100644 tools/tests/test_api.c
delete mode 100644 tools/tests/test_environment.c
delete mode 100644 tools/tests/test_partitions.c

diff --git a/.travis-build.sh b/.travis-build.sh
index 6dc6c0c..7e79e9a 100755
--- a/.travis-build.sh
+++ b/.travis-build.sh
@@ -25,22 +25,13 @@ install_common_deps()

install_native_deps()
{
- sudo apt-add-repository \
- 'deb http://archive.ubuntu.com/ubuntu xenial universe'
- sudo apt-get update -qq
- sudo apt-get install --no-install-recommends \
- --target-release xenial libcmocka-dev
+ true
}

install_i586_deps()
{
sudo apt-get install --no-install-recommends \
libz-dev:i386
- sudo apt-add-repository \
- 'deb http://archive.ubuntu.com/ubuntu xenial universe'
- sudo apt-get update -qq
- sudo apt-get install --no-install-recommends \
- --target-release xenial libcmocka-dev:i386
}

prepare_build()
diff --git a/tools/tests/Makefile.am b/tools/tests/Makefile.am
index dd4a249..ec96981 100644
--- a/tools/tests/Makefile.am
+++ b/tools/tests/Makefile.am
TESTS = $(check_PROGRAMS)
- * Copyright (c) Siemens AG, 2017
- *
- * Authors:
- * Andreas Reichel <andreas.r...@siemens.com>
- *

Andreas J. Reichel

unread,
Nov 2, 2017, 11:57:40 AM11/2/17
to efibootg...@googlegroups.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>

Add tests for API functions that are available through libebgenv.a
except user variables.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---
.travis-build.sh | 1 +
tools/tests/Makefile.am | 7 +-
tools/tests/test_ebgenv_api.c | 635 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 642 insertions(+), 1 deletion(-)
create mode 100644 tools/tests/test_ebgenv_api.c

diff --git a/.travis-build.sh b/.travis-build.sh
index 4e58407..11acb93 100755
--- a/.travis-build.sh
+++ b/.travis-build.sh
@@ -116,6 +116,7 @@ case "$TARGET_EFFECTIVE" in
suppress+=" --suppress=unusedFunction:env/env_api_fat.c"
# Some functions are used by linker wrapping
suppress+=" --suppress=unusedFunction:tools/tests/test_probe_config_file.c"
+ suppress+=" --suppress=unusedFunction:tools/tests/test_ebgenv_api.c"
# EFI uses void* as ImageBase needed for further calculations
suppress+=" --suppress=arithOperationsOnVoidPointer:main.c"

diff --git a/tools/tests/Makefile.am b/tools/tests/Makefile.am
index 0d21673..e370158 100644
--- a/tools/tests/Makefile.am
+++ b/tools/tests/Makefile.am
@@ -47,7 +47,8 @@ libenvapi_testlib_fat.a: libtest_env_api_fat.a
check_PROGRAMS = test_bgenv_init_retval \
test_probe_config_partitions \
test_probe_config_file \
- test_ebgenv_api_internal
+ test_ebgenv_api_internal \
+ test_ebgenv_api

FAT_TESTLIB=libenvapi_testlib_fat.a

@@ -72,4 +73,8 @@ test_ebgenv_api_internal_CFLAGS = $(AM_CFLAGS)
test_ebgenv_api_internal_SOURCES = test_ebgenv_api_internal.c $(SRC_TEST_COMMON)
test_ebgenv_api_internal_LDADD = $(FAT_TESTLIB) $(LIBCHECK_LIBS)

+test_ebgenv_api_CFLAGS = $(AM_CFLAGS) -Wl,--wrap=bgenv_set -Wl,--wrap=bgenv_get
+test_ebgenv_api_SOURCES = test_ebgenv_api.c $(SRC_TEST_COMMON)
+test_ebgenv_api_LDADD = $(FAT_TESTLIB) $(LIBCHECK_LIBS)
+
TESTS = $(check_PROGRAMS)
diff --git a/tools/tests/test_ebgenv_api.c b/tools/tests/test_ebgenv_api.c
new file mode 100644
index 0000000..3f44ea9
--- /dev/null
+++ b/tools/tests/test_ebgenv_api.c
@@ -0,0 +1,635 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Andreas Reichel <andreas.r...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#include <stdlib.h>
+#include <check.h>
+#include <fff.h>
+#include <env_api.h>
+#include <ebgenv.h>
+#include <env_config_file.h>
+#include <env_config_partitions.h>
+
+DEFINE_FFF_GLOBALS;
+
+static char *devpath = "/dev/nobrain";
+
+Suite *ebg_test_suite(void);
+
+extern bool write_env(CONFIG_PART *part, BG_ENVDATA *env);
+CONFIG_PART config_parts[ENV_NUM_CONFIG_PARTS];
+BG_ENVDATA envdata[ENV_NUM_CONFIG_PARTS];
+
+START_TEST(ebgenv_api_ebg_env_create_new)
+{
+ ebgenv_t e;
+ int ret;
+ wchar_t bufferw[10];
+ char buffer[10];
+ char *kernelfile = "kernel123";
+ char *kernelparams = "param456";
+
+ memset(&e, 0, sizeof(e));
+
+ /* Test if ebg_env_create_new returns EIO if bgenv_init
+ * returns false
+ */
+ bgenv_init_fake.return_val = false;
+ ret = ebg_env_create_new(&e);
+ ck_assert_int_eq(ret, EIO);
+
+ /* Test if errno is returned by ebg_env_created, if the bgenv pointer
+ * is NULL but ebg_new_env_created is true, which is contradictory.
+ * Also, ebg_new_env_created must be reset to false.
+ */
+ bgenv_init_fake.return_val = true;
+ bgenv_close_fake.return_val = true;
+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++)
+ {
+ envdata[i].revision = i;
+ }
+ e.ebg_new_env_created = true;
+ e.bgenv = NULL;
+ errno = 3044;
+
+ ret = ebg_env_create_new(&e);
+
+ ck_assert_int_eq(ret, 3044);
+ ck_assert(e.ebg_new_env_created == false);
+
+
+ /* Test if ebg_env_open_current returns EIO if bgenv_init returns false
+ */
+ bgenv_init_fake.return_val = false;
+ ret = ebg_env_open_current(&e);
+
+ ck_assert_int_eq(ret, EIO);
+
+#if ENV_NUM_CONFIG_PARTS > 1
+
+ /* Test if ebg_env_open_current opens the environment with the highest
+ * revision
+ */
+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {
+ envdata[i].revision = i + 1;
+ }
+
+ bgenv_init_fake.return_val = true;
+ ret = ebg_env_open_current(&e);
+
+ ck_assert_int_eq(ret, 0);
+ ck_assert(((BGENV *)e.bgenv)->data == &envdata[ENV_NUM_CONFIG_PARTS-1]);
+
+ (void)ebg_env_close(&e);
+
+ envdata[0].revision = 0xFFFF;
+
+ ret = ebg_env_open_current(&e);
+
+ ck_assert_int_eq(ret, 0);
+ ck_assert(((BGENV *)e.bgenv)->data == &envdata[0]);
+
+ (void)ebg_env_close(&e);
+#endif
+
+}
+END_TEST
+
+START_TEST(ebgenv_api_ebg_env_get)
+{
+ ebgenv_t e;
+ memset(&e, 0, sizeof(e));
+ int ret;
+ char buffer[1];
+
+ /* Test if ebg_env_get calls bg_env_get correctly and that it returns
+ * -EINVAL if no key is provided
+ */
+ bgenv_get_call_count = 0;
+
+ ret = ebg_env_get(&e, NULL, NULL);
+ ck_assert_int_eq(ret, -EINVAL);
+
+ ck_assert(bgenv_get_call_count == 1);
+ ck_assert(bgenv_getset_arg0 == e.bgenv);
+ ck_assert(bgenv_getset_arg1 == NULL);
+ ck_assert(bgenv_getset_arg2 == NULL);
+
+ /* Test if ebg_env_get retrieves correct data if given a valid
+ * environment handle.
+ */
+ e.bgenv = (BGENV *)calloc(1, sizeof(BGENV));
+ ck_assert(e.bgenv != NULL);
+
+ ((BGENV *)e.bgenv)->data = (BG_ENVDATA *)calloc(1, sizeof(BG_ENVDATA));
+ ck_assert(((BGENV *)e.bgenv)->data != NULL);
+
+ ck_assert(((BGENV *)e.bgenv)->data != NULL);
+
+ ck_assert(((BGENV *)e.bgenv)->data != NULL);
+
+ ck_assert(((BGENV *)e.bgenv)->data != NULL);
+
+ ck_assert(((BGENV *)e.bgenv)->data != NULL);
+
+ ret = ebg_env_user_free(&e);
+ ck_assert_int_eq(ret, ENV_MEM_USERVARS);
+
+ free(((BGENV *)e.bgenv)->data);
+ free(e.bgenv);
+}
+END_TEST
+
+START_TEST(ebgenv_api_ebg_env_getglobalstate)
+{
+#if ENV_NUM_CONFIG_PARTS > 1
+ ebgenv_t e;
+ uint16_t state;
+ memset(&e, 0, sizeof(e));
+
+ /* Test if ebg_env_getglobalstate returns OK if current environment
+ * is set to OK
+ */
+ e.bgenv = (BGENV *)calloc(1, sizeof(BGENV));
+ ck_assert(e.bgenv != NULL);
+
+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {
+ envdata[i].revision = i + 1;
+ }
+
+ envdata[1].revision = 0;
+ envdata[1].ustate = USTATE_OK;
+
+ state = ebg_env_getglobalstate(&e);
+ ck_assert_int_eq(state, USTATE_OK);
+
+ /* Test if ebg_env_getglobalstate returns FAILED if current environment
+ * is set to FAILED with revision 0
+ */
+ envdata[1].revision = 0;
+ envdata[1].ustate = USTATE_FAILED;
+
+ state = ebg_env_getglobalstate(&e);
+ ck_assert_int_eq(state, USTATE_FAILED);
+
+ /* Test if ebg_env_getglobalstate returns FAILED if current environment
+ * is set to FAILED with non-zero revision
+ */
+ envdata[1].revision = 15;
+
+ state = ebg_env_getglobalstate(&e);
+ ck_assert_int_eq(state, USTATE_FAILED);
+
+ /* Test if ebg_env_getglobalstate returns INSTALLED if current
+ * environment is set to INSTALLED
+ */
+ envdata[1].revision = 15;
+ envdata[1].ustate = USTATE_INSTALLED;
+
+ state = ebg_env_getglobalstate(&e);
+ ck_assert_int_eq(state, USTATE_INSTALLED);
+
+ /* Test if ebg_env_getglobalstate returns FAILED if current environment
+ * is set to OK and any other is set to FAILED
+ */
+ envdata[1].ustate = USTATE_OK;
+ envdata[0].revision = 0;
+ envdata[0].ustate = USTATE_FAILED;
+
+ state = ebg_env_getglobalstate(&e);
+ ck_assert_int_eq(state, USTATE_FAILED);
+
+ /* Test if ebg_env_getglobalstate returns OK if current environment is
+ * set to OK and any other is set to INSTALLED
+ */
+ envdata[0].ustate = USTATE_INSTALLED;
+
+ state = ebg_env_getglobalstate(&e);
+ ck_assert_int_eq(state, USTATE_OK);
+
+ /* Test if ebg_env_getglobalstate returns TESTING if current
+ * environment is set to TESTING and none is FAILED
+ */
+ envdata[0].ustate = USTATE_OK;
+ envdata[1].ustate = USTATE_TESTING;
+
+ state = ebg_env_getglobalstate(&e);
+ ck_assert_int_eq(state, USTATE_TESTING);
+
+ /* Test if ebg_env_getglobalstate returns OK if current environment is
+ * set to OK and none is TESTING
+ */
+ envdata[0].ustate = USTATE_TESTING;
+ envdata[1].ustate = USTATE_OK;
+
+ state = ebg_env_getglobalstate(&e);
+ ck_assert_int_eq(state, USTATE_OK);
+
+ free(e.bgenv);
+#endif
+}
+END_TEST
+
+START_TEST(ebgenv_api_ebg_env_setglobalstate)
+{
+#if ENV_NUM_CONFIG_PARTS > 1
+ ebgenv_t e;
+ int ret;
+ memset(&e, 0, sizeof(e));
+
+ /* Test if ebg_env_setglobalstate sets only current to FAILED
+ */
+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {
+ envdata[i].revision = i + 1;
+ }
+
+ bgenv_init_fake.return_val = true;
+
+ ret = ebg_env_open_current(&e);
+
+ ck_assert_int_eq(ret, 0);
+ ck_assert(((BGENV *)e.bgenv)->data == &envdata[ENV_NUM_CONFIG_PARTS-1]);
+
+
+ /* Test if ebg_env_setglobalstate sets ALL environments to OK
+ */
+ envdata[0].ustate = USTATE_FAILED;
+ envdata[1].ustate = USTATE_FAILED;
+
+ bgenv_write_fake.return_val = true;
+ bgenv_close_fake.return_val = true;
+
+ ret = ebg_env_setglobalstate(&e, USTATE_OK);
+
+ ck_assert_int_eq(ret, 0);
+ ck_assert_int_eq(envdata[0].ustate, USTATE_OK);
+ ck_assert_int_eq(envdata[1].ustate, USTATE_OK);
+
+ /* Test if ebg_env_setglobalstate sets current environment to TESTING
+ */
+ envdata[0].ustate = USTATE_INSTALLED;
+ envdata[1].ustate = USTATE_INSTALLED;
+
+ ret = ebg_env_setglobalstate(&e, USTATE_TESTING);
+
+ ck_assert_int_eq(ret, 0);
+ ck_assert_int_eq(envdata[0].ustate, USTATE_TESTING);
+ ck_assert_int_eq(envdata[1].ustate, USTATE_INSTALLED);
+
+ /* Test if ebg_env_setglobalstate fails and returns EIO if bgenv_write
+ * fails
+ */
+ bgenv_write_fake.return_val = false;
+ bgenv_close_fake.return_val = true;
+
+ ret = ebg_env_setglobalstate(&e, USTATE_OK);
+
+ ck_assert_int_eq(ret, EIO);
+
+ /* Test if ebg_env_setglobalstate fails and returns EIO if bgenv_close
+ * fails
+ */
+ bgenv_write_fake.return_val = true;
+ bgenv_close_fake.return_val = false;
+
+ ret = ebg_env_setglobalstate(&e, USTATE_OK);
+
+ ck_assert_int_eq(ret, EIO);
+
+ (void)ebg_env_close(&e);
+#endif
+}
+END_TEST
+
+START_TEST(ebgenv_api_ebg_env_close)
+{
+ ebgenv_t e;
+ int ret;
+ memset(&e, 0, sizeof(e));
+
+ /* Test if ebg_env_close fails with invalid context and returns EIO
+ */
+ ret = ebg_env_close(&e);
+ ck_assert_int_eq(ret, EIO);
+
+ /* Test if ebg_env_close fails and returns EIO if bgenv_write fails
+ */
+ e.bgenv = calloc(1, sizeof(BGENV));
+ ck_assert(e.bgenv != NULL);
+
+ ((BGENV *)e.bgenv)->data = calloc(1, sizeof(BG_ENVDATA));
+ bgenv_write_fake.return_val = false;
+ bgenv_close_fake.return_val = true;
+ ret = ebg_env_close(&e);
+
+ ck_assert_int_eq(ret, EIO);
+
+ /* Test if ebg_env_close fails and returns EIO if bgenv_close fails.
+ */
+ bgenv_write_fake.return_val = true;
+ bgenv_close_fake.return_val = false;
+ ret = ebg_env_close(&e);
+
+ ck_assert_int_eq(ret, EIO);
+
+ /* Test if ebg_env_close is successful if all prerequisites are met
+ */
+ bgenv_write_fake.return_val = true;
+ bgenv_close_fake.return_val = true;
+ BGENV *save_ptr = e.bgenv;
+ ret = ebg_env_close(&e);
+
+ ck_assert_int_eq(ret, 0);
+ ck_assert(e.bgenv == NULL);
+
+ free(save_ptr->data);
+ free(save_ptr);
+}
+END_TEST
+
+Suite *ebg_test_suite(void)
+{
+ Suite *s;
+ TCase *tc_core;
+
+ s = suite_create("ebgenv_api");
+
+ TFun tfuncs[] = {
+ ebgenv_api_ebg_env_create_new,
+ ebgenv_api_ebg_env_open_current,
+ ebgenv_api_ebg_env_get,
+ ebgenv_api_ebg_env_set,
+ ebgenv_api_ebg_env_set_ex,
+ ebgenv_api_ebg_env_get_ex,
+ ebgenv_api_ebg_env_user_free,
+ ebgenv_api_ebg_env_getglobalstate,
+ ebgenv_api_ebg_env_setglobalstate,
+ ebgenv_api_ebg_env_close

Andreas J. Reichel

unread,
Nov 2, 2017, 11:57:40 AM11/2/17
to efibootg...@googlegroups.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>

Emulate config files to test calls to probe_config_file.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---
.travis-build.sh | 2 +
tools/tests/Makefile.am | 9 +-
tools/tests/test_probe_config_file.c | 191 +++++++++++++++++++++++++++++++++++
3 files changed, 201 insertions(+), 1 deletion(-)
create mode 100644 tools/tests/test_probe_config_file.c

diff --git a/.travis-build.sh b/.travis-build.sh
index 6c8d79c..4e58407 100755
--- a/.travis-build.sh
+++ b/.travis-build.sh
@@ -114,6 +114,8 @@ case "$TARGET_EFFECTIVE" in
suppress+=" --suppress=unusedFunction:env/fatvars.c"
suppress+=" --suppress=unusedFunction:tools/tests/test_environment.c"
suppress+=" --suppress=unusedFunction:env/env_api_fat.c"
+ # Some functions are used by linker wrapping
+ suppress+=" --suppress=unusedFunction:tools/tests/test_probe_config_file.c"
# EFI uses void* as ImageBase needed for further calculations
suppress+=" --suppress=arithOperationsOnVoidPointer:main.c"

diff --git a/tools/tests/Makefile.am b/tools/tests/Makefile.am
index 72e9a49..957a044 100644
--- a/tools/tests/Makefile.am
+++ b/tools/tests/Makefile.am
@@ -45,7 +45,8 @@ libenvapi_testlib_fat.a: libtest_env_api_fat.a
$(OBJCOPY) --weaken $^ $@

check_PROGRAMS = test_bgenv_init_retval \
- test_probe_config_partitions
+ test_probe_config_partitions \
+ test_probe_config_file

FAT_TESTLIB=libenvapi_testlib_fat.a

@@ -61,4 +62,10 @@ test_probe_config_partitions_SOURCES = test_probe_config_partitions.c \
$(SRC_TEST_COMMON)
test_probe_config_partitions_LDADD = $(FAT_TESTLIB) $(LIBCHECK_LIBS)

+test_probe_config_file_CFLAGS = $(AM_CFLAGS) -Wl,--wrap=probe_config_file
+test_probe_config_file_SOURCES = test_probe_config_file.c fake_devices.c \
+ $(SRC_TEST_COMMON)
+test_probe_config_file_LDADD = $(FAT_TESTLIB) $(LIBCHECK_LIBS)
+
+
TESTS = $(check_PROGRAMS)
diff --git a/tools/tests/test_probe_config_file.c b/tools/tests/test_probe_config_file.c
new file mode 100644
index 0000000..c32fb61
--- /dev/null
+++ b/tools/tests/test_probe_config_file.c
@@ -0,0 +1,191 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Andreas Reichel <andreas.r...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#include <stdlib.h>
+#include <errno.h>
+#include <sys/queue.h>
+#include <check.h>
+#include <fff.h>
+#include <env_api.h>
+
+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {
+ add_fake_partition(0);
+ }
+
+ ped_device_get_next_fake.custom_fake = ped_device_get_next_custom_fake;
+ get_mountpoint_fake.custom_fake = get_mountpoint_custom_fake;
+ probe_config_file_call_count = 0;
+
+ STAILQ_INIT(&head);
+
+ result = bgenv_init();
+
+ delete_temp_files();
+
+ free_fake_devices();
+
+ ck_assert_int_eq(ped_device_probe_all_fake.call_count, 1);
+ ck_assert_int_eq(ped_device_get_next_fake.call_count, 2);
+ ck_assert_int_eq(probe_config_file_call_count, ENV_NUM_CONFIG_PARTS);
+ ck_assert_int_eq(get_mountpoint_fake.call_count, ENV_NUM_CONFIG_PARTS);
+ ck_assert(result == true);
+}
+END_TEST
+
+Suite *ebg_test_suite(void)
+{
+ Suite *s;
+ TCase *tc_core;
+
+ s = suite_create("env_api_fat");
+
+ tc_core = tcase_create("Core");
+ tcase_add_test(tc_core, env_api_fat_test_probe_config_file);

Andreas J. Reichel

unread,
Nov 2, 2017, 11:57:40 AM11/2/17
to efibootg...@googlegroups.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>

Use libcheck as new unit testing framework.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---
.travis-build.sh | 17 +++++++++++------
configure.ac | 3 +++
docs/COMPILE.md | 4 ++--
3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/.travis-build.sh b/.travis-build.sh
index 7e79e9a..4e9e3a9 100755
--- a/.travis-build.sh
+++ b/.travis-build.sh
@@ -25,13 +25,14 @@ install_common_deps()

install_native_deps()
{
- true
+ sudo apt-get install --no-install-recommends \
+ libz-dev check
}

install_i586_deps()
{
sudo apt-get install --no-install-recommends \
- libz-dev:i386
+ libz-dev:i386 check:i386
}

prepare_build()
@@ -77,14 +78,17 @@ case "$TARGET_EFFECTIVE" in
install_i586_deps
prepare_build
enter_build
+ export PKG_CONFIG_DIR=
+ export PKG_CONFIG_PATH=/usr/lib/i386-linux-gnu/pkgconfig
+ export PKG_CONFIG_LIBDIR=/usr/lib/i386-linux-gnu
../configure --with-gnuefi-lib-dir=/usr/lib32 CFLAGS=-m32 \
host_alias=i586-linux
exec make check
;;

cppcheck)
- install_common_deps
- install_native_deps
+ install_common_deps
+ install_native_deps
echo "Building and installing cppcheck..."
if ! install_cppcheck >cppcheck_build.log 2>&1
then
@@ -96,8 +100,9 @@ case "$TARGET_EFFECTIVE" in

suppress=""
# Justified suppressions:
- # Not part of the project:
- suppress+=" --suppress=variableScope:/usr/include/bits/stdlib-bsearch.h"
+ # Does not belong to the project
+ suppress+=" --suppress=*:/usr/include/*"
+ suppress+=" --suppress=*:/usr/include/bits/*"
# Function 'efi_main' is called by efi:
suppress+=" --suppress=unusedFunction:main.c"
# Some functions are defined for API only
diff --git a/configure.ac b/configure.ac
index d1dd082..7e8e7e9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -163,6 +163,9 @@ AC_ARG_WITH([mem-uservars],
])

AC_DEFINE_UNQUOTED([ENV_MEM_USERVARS], [${ENV_MEM_USERVARS}], [Reserved memory for user variables])
+
+PKG_PROG_PKG_CONFIG()
+PKG_CHECK_MODULES(LIBCHECK, check)
# ------------------------------------------------------------------------------
AC_CONFIG_FILES([
Makefile
diff --git a/docs/COMPILE.md b/docs/COMPILE.md
index 0429d81..e194601 100644
--- a/docs/COMPILE.md
+++ b/docs/COMPILE.md
@@ -5,13 +5,13 @@
### Arch Linux ###

```
-pacman -S gnu-efi-libs pciutils
+pacman -S gnu-efi-libs pciutils check

Andreas J. Reichel

unread,
Nov 2, 2017, 11:57:41 AM11/2/17
to efibootg...@googlegroups.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>

Use function faking framework for mocking in unit tests.
Integrate it into the project as git submodule.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---
.gitmodules | 3 +++
.travis-build.sh | 5 ++++-
testing/fff | 1 +
tools/tests/Makefile.am | 7 +++----
4 files changed, 11 insertions(+), 5 deletions(-)
create mode 100644 .gitmodules
create mode 160000 testing/fff

diff --git a/.gitmodules b/.gitmodules
new file mode 100644
index 0000000..2412404
--- /dev/null
+++ b/.gitmodules
@@ -0,0 +1,3 @@
+[submodule "testing/fff"]
+ path = testing/fff
+ url = https://github.com/meekrosoft/fff
diff --git a/.travis-build.sh b/.travis-build.sh
index 4e9e3a9..6c8d79c 100755
--- a/.travis-build.sh
+++ b/.travis-build.sh
@@ -98,6 +98,9 @@ case "$TARGET_EFFECTIVE" in
prepare_build
./configure

+ ignore=""
+ ignore+=" -i testing/fff"
+
suppress=""
# Justified suppressions:
# Does not belong to the project
@@ -133,7 +136,7 @@ case "$TARGET_EFFECTIVE" in
# Exit code '1' is returned if arguments are not valid or if no input
# files are provided. Compare 'cppcheck --help'.
exec cppcheck -f -q --error-exitcode=2 \
- $enable $suppress $cpp_conf $includes .
+ $enable $suppress $ignore $cpp_conf $includes .
;;
coverity_prepare)
install_common_deps
diff --git a/testing/fff b/testing/fff
new file mode 160000
index 0000000..719dd8b
--- /dev/null
+++ b/testing/fff
@@ -0,0 +1 @@
+Subproject commit 719dd8b6a39a34d0dbf71eeccd4bcd8c5e84c9b4
diff --git a/tools/tests/Makefile.am b/tools/tests/Makefile.am
index ec96981..9b2344f 100644
--- a/tools/tests/Makefile.am
+++ b/tools/tests/Makefile.am
@@ -12,12 +12,11 @@

OBJCOPY ?= objcopy

-AM_CPPFLAGS = \
+AM_CFLAGS = \
-I$(top_srcdir)/include \
-I$(top_srcdir)/swupdate-adapter \
- -I$(top_srcdir)/tools
-
-AM_CFLAGS = \
+ -I$(top_srcdir)/tools \
+ -I$(top_srcdir)/testing/fff \
-Wno-unused-parameter \
-Wmissing-prototypes \
-fshort-wchar \
--
2.14.2

Andreas J. Reichel

unread,
Nov 2, 2017, 11:57:41 AM11/2/17
to efibootg...@googlegroups.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>

Add test for bgenv_init faking probe_config_partition.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---
env/env_api_fat.c | 6 +--
include/test-interface.h | 2 +
tools/tests/Makefile.am | 29 +++++++++--
tools/tests/test_bgenv_init_retval.c | 96 ++++++++++++++++++++++++++++++++++++
tools/tests/test_main.c | 34 +++++++++++++
5 files changed, 161 insertions(+), 6 deletions(-)
create mode 100644 tools/tests/test_bgenv_init_retval.c
create mode 100644 tools/tests/test_main.c

diff --git a/env/env_api_fat.c b/env/env_api_fat.c
index a344fcb..8eb1454 100644
--- a/env/env_api_fat.c
+++ b/env/env_api_fat.c
@@ -20,7 +20,7 @@

bool bgenv_verbosity = false;

-static EBGENVKEY bgenv_str2enum(char *key)
+EBGENVKEY bgenv_str2enum(char *key)
{
if (strncmp(key, "kernelfile", strlen("kernelfile") + 1) == 0) {
return EBGENV_KERNELFILE;
@@ -120,8 +120,8 @@ bool write_env(CONFIG_PART *part, BG_ENVDATA *env)
return result;
}

-static CONFIG_PART config_parts[ENV_NUM_CONFIG_PARTS];
-static BG_ENVDATA envdata[ENV_NUM_CONFIG_PARTS];
+CONFIG_PART config_parts[ENV_NUM_CONFIG_PARTS];
+BG_ENVDATA envdata[ENV_NUM_CONFIG_PARTS];

bool bgenv_init()
{
diff --git a/include/test-interface.h b/include/test-interface.h
index 36b0a62..bd2572d 100644
--- a/include/test-interface.h
+++ b/include/test-interface.h
@@ -22,4 +22,6 @@ bool probe_config_file(CONFIG_PART *cfgpart);
bool probe_config_partitions(CONFIG_PART *cfgparts);
bool mount_partition(CONFIG_PART *cfgpart);

+EBGENVKEY bgenv_str2enum(char *key);
+
#endif // __TEST_INTERFACE_H__
diff --git a/tools/tests/Makefile.am b/tools/tests/Makefile.am
index 9b2344f..f5bc4a4 100644
--- a/tools/tests/Makefile.am
+++ b/tools/tests/Makefile.am
@@ -16,7 +16,10 @@ AM_CFLAGS = \
-I$(top_srcdir)/include \
-I$(top_srcdir)/swupdate-adapter \
-I$(top_srcdir)/tools \
+ -I$(top_srcdir)/tools/tests \
-I$(top_srcdir)/testing/fff \
+ -I$(top_srcdir) \
+ -include config.h \
-Wno-unused-parameter \
-Wmissing-prototypes \
-fshort-wchar \
@@ -24,11 +27,31 @@ AM_CFLAGS = \
-D_GNU_SOURCE \
-g

+libtest_env_api_fat_a_SRC = \
+ ../../env/env_api.c \
+ ../../env/env_api_fat.c \
+ ../../tools/ebgpart.c \
+ ../../env/env_config_file.c \
+ ../../env/env_config_partitions.c \
+ ../../env/env_disk_utils.c \
+ ../../env/uservars.c
+
CLEANFILES =

-test_api:
- ln -sf /bin/true test_api
+noinst_LIBRARIES = libtest_env_api_fat.a
+libtest_env_api_fat_a_SOURCES = $(libtest_env_api_fat_a_SRC)
+
+libenvapi_testlib_fat.a: libtest_env_api_fat.a
+ $(OBJCOPY) --weaken $^ $@
+
+check_PROGRAMS = test_bgenv_init_retval
+
+FAT_TESTLIB=libenvapi_testlib_fat.a
+
+SRC_TEST_COMMON=test_main.c

-check_PROGRAMS = test_api
+test_bgenv_init_retval_CFLAGS = $(AM_CFLAGS)
+test_bgenv_init_retval_SOURCES = test_bgenv_init_retval.c $(SRC_TEST_COMMON)
+test_bgenv_init_retval_LDADD = $(FAT_TESTLIB) $(LIBCHECK_LIBS)

TESTS = $(check_PROGRAMS)
diff --git a/tools/tests/test_bgenv_init_retval.c b/tools/tests/test_bgenv_init_retval.c
new file mode 100644
index 0000000..27324c9
--- /dev/null
+++ b/tools/tests/test_bgenv_init_retval.c
@@ -0,0 +1,96 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Andreas Reichel <andreas.r...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#include <stdlib.h>
+#include <check.h>
+#include <fff.h>
+#include <env_api.h>
+#include <env_config_file.h>
+#include <env_config_partitions.h>
+
+DEFINE_FFF_GLOBALS;
+
+static char *devpath = "/dev/nobrain";
+
+bool read_env(CONFIG_PART *part, BG_ENVDATA *env);
+
+Suite *env_api_fat_suite(void);
+bool probe_config_partitions_custom_fake(CONFIG_PART *cfgpart);
+bool read_env_custom_fake(CONFIG_PART *cp, BG_ENVDATA *env);
+
+Suite *ebg_test_suite(void);
+
+bool probe_config_partitions_custom_fake(CONFIG_PART *cfgpart)
+{
+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {
+ cfgpart[i].devpath = devpath;
+ }
+ return true;
+}
+
+bool read_env_custom_fake(CONFIG_PART *cp, BG_ENVDATA *env)
+{
+ if (!env) {
+ return false;
+ }
+ memset(env, 0, sizeof(BG_ENVDATA));
+ return true;
+}
+
+FAKE_VALUE_FUNC(bool, probe_config_partitions, CONFIG_PART *);
+FAKE_VALUE_FUNC(bool, read_env, CONFIG_PART *, BG_ENVDATA *);
+
+START_TEST(env_api_fat_test_bgenv_init_retval)
+{
+ bool result;
+ /* In this unit test, contents of environment data are
+ * faked to be all zero
+ */
+
+ /* Test if bgenv_init fails if no config partitions are found
+ */
+ RESET_FAKE(probe_config_partitions);
+
+ probe_config_partitions_fake.return_val = false;
+
+ result = bgenv_init();
+
+ ck_assert(probe_config_partitions_fake.call_count == 1);
+ ck_assert(result == false);
+
+ /* Test if bgenv_init succeeds if config partitions are found
+ */
+ RESET_FAKE(probe_config_partitions);
+
+ probe_config_partitions_fake.custom_fake = probe_config_partitions_custom_fake;
+ read_env_fake.custom_fake = read_env_custom_fake;
+ result = bgenv_init();
+
+ ck_assert(probe_config_partitions_fake.call_count == 1);
+ ck_assert(read_env_fake.call_count == ENV_NUM_CONFIG_PARTS);
+ ck_assert(result == true);
+}
+END_TEST
+
+Suite *ebg_test_suite(void)
+{
+ Suite *s;
+ TCase *tc_core;
+
+ s = suite_create("env_api_fat");
+
+ tc_core = tcase_create("Core");
+ tcase_add_test(tc_core, env_api_fat_test_bgenv_init_retval);
+ suite_add_tcase(s, tc_core);
+
+ return s;
+}
diff --git a/tools/tests/test_main.c b/tools/tests/test_main.c
new file mode 100644
index 0000000..a25e092
--- /dev/null
+++ b/tools/tests/test_main.c
@@ -0,0 +1,34 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Andreas Reichel <andreas.r...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#include <stdlib.h>
+#include <check.h>
+#include <fff.h>
+
+extern Suite *ebg_test_suite(void);
+
+int main(void)
+{
+ int number_failed;
+
+ Suite *s;
+ SRunner *sr;
+
+ s = ebg_test_suite();
+ sr = srunner_create(s);
+
+ srunner_run_all(sr, CK_NORMAL);
+ number_failed = srunner_ntests_failed(sr);
+ srunner_free(sr);
+
+ return (number_failed == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
+}
--
2.14.2

Andreas J. Reichel

unread,
Nov 2, 2017, 11:57:41 AM11/2/17
to efibootg...@googlegroups.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>

For unit test support, code must be more modular.
This also avoids inline functions and static functions where possible.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---
Makefile.am | 4 +-
env/env_api_fat.c | 224 ++--------------------------------------
env/env_config_file.c | 90 ++++++++++++++++
env/env_config_partitions.c | 83 +++++++++++++++
env/env_disk_utils.c | 99 ++++++++++++++++++
include/env_config_file.h | 20 ++++
include/env_config_partitions.h | 18 ++++
include/env_disk_utils.h | 20 ++++
8 files changed, 339 insertions(+), 219 deletions(-)
create mode 100644 env/env_config_file.c
create mode 100644 env/env_config_partitions.c
create mode 100644 env/env_disk_utils.c
create mode 100644 include/env_config_file.h
create mode 100644 include/env_config_partitions.h
create mode 100644 include/env_disk_utils.h

diff --git a/Makefile.am b/Makefile.am
index 51e2658..1e0dc1c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -72,6 +72,9 @@ lib_LIBRARIES = libebgenv.a
libebgenv_a_SOURCES = \
env/@env_api_file@.c \
env/env_api.c \
+ env/env_config_file.c \
+ env/env_config_partitions.c \
+ env/env_disk_utils.c \
env/uservars.c \
tools/ebgpart.c

@@ -91,7 +94,6 @@ pkginclude_HEADERS = \
bin_PROGRAMS = bg_setenv

bg_setenv_SOURCES = \
- tools/ebgpart.c \
tools/bg_setenv.c

bg_setenv_CFLAGS = \
diff --git a/env/env_api_fat.c b/env/env_api_fat.c
index 653d3f8..a344fcb 100644
--- a/env/env_api_fat.c
+++ b/env/env_api_fat.c
@@ -11,11 +11,12 @@
*/

#include "env_api.h"
-#include "ebgpart.h"
+#include "env_disk_utils.h"
+#include "env_config_partitions.h"
+#include "env_config_file.h"
#include "uservars.h"
#include "test-interface.h"
-
-const char *tmp_mnt_dir = "/tmp/mnt-XXXXXX";
+#include "ebgpart.h"

bool bgenv_verbosity = false;

@@ -46,219 +47,6 @@ void bgenv_be_verbose(bool v)
ebgpart_beverbose(v);
}

-static char *get_mountpoint(char *devpath)
-{
- struct mntent *part = NULL;
- FILE *mtab = NULL;
-
- if ((mtab = setmntent("/proc/mounts", "r")) == NULL)
- return NULL;
-
- while ((part = getmntent(mtab)) != NULL) {
- if ((part->mnt_fsname != NULL) &&
- (strcmp(part->mnt_fsname, devpath)) == 0) {
- char *mntpoint;
-
- if (!(mntpoint =
- malloc(strlen(part->mnt_dir) + 1))) {
- break;
- };
- strncpy(mntpoint, part->mnt_dir,
- strlen(part->mnt_dir) + 1);
- return mntpoint;
- }
- }
- endmntent(mtab);
-
- return NULL;
-}
-
-__attribute__((noinline))
-bool mount_partition(CONFIG_PART *cfgpart)
-{
- char tmpdir_template[256];
- char *mountpoint;
- (void)snprintf(tmpdir_template, 256, "%s", tmp_mnt_dir);
- if (!cfgpart) {
- return false;
- }
- if (!cfgpart->devpath) {
- return false;
- }
- if (!(mountpoint = mkdtemp(tmpdir_template))) {
- VERBOSE(stderr, "Error creating temporary mount point.\n");
- return false;
- }
- if (mount(cfgpart->devpath, mountpoint, "vfat", 0, "")) {
- VERBOSE(stderr, "Error mounting to temporary mount point.\n");
- if (rmdir(tmpdir_template)) {
- VERBOSE(stderr,
- "Error deleting temporary directory.\n");
- }
- return false;
- }
- cfgpart->mountpoint = (char *)malloc(strlen(mountpoint) + 1);
- if (!cfgpart->mountpoint) {
- VERBOSE(stderr, "Error, out of memory.\n");
- return false;
- }
- strncpy(cfgpart->mountpoint, mountpoint, strlen(mountpoint) + 1);
- return true;
-}
-
-static void unmount_partition(CONFIG_PART *cfgpart)
-{
- if (!cfgpart) {
- return;
- }
- if (!cfgpart->mountpoint) {
- return;
- }
- if (umount(cfgpart->mountpoint)) {
- VERBOSE(stderr, "Error unmounting temporary mountpoint %s.\n",
- cfgpart->mountpoint);
- }
- if (rmdir(cfgpart->mountpoint)) {
- VERBOSE(stderr, "Error deleting temporary directory %s.\n",
- cfgpart->mountpoint);
- }
- free(cfgpart->mountpoint);
- cfgpart->mountpoint = NULL;
-}
-
-static FILE *open_config_file(CONFIG_PART *cfgpart, char *mode)
-{
- char *configfilepath;
- configfilepath = (char *)malloc(strlen(FAT_ENV_FILENAME) +
- strlen(cfgpart->mountpoint) + 2);
- if (!configfilepath) {
- return NULL;
- }
- strncpy(configfilepath, cfgpart->mountpoint,
- strlen(cfgpart->mountpoint) + 1);
- strncat(configfilepath, "/", 1);
- strncat(configfilepath, FAT_ENV_FILENAME, strlen(FAT_ENV_FILENAME));
- VERBOSE(stdout, "Probing config file at %s.\n", configfilepath);
- FILE *config = fopen(configfilepath, mode);
- free(configfilepath);
- return config;
-}
-
-__attribute__((noinline))
-bool probe_config_file(CONFIG_PART *cfgpart)
-{
- bool do_unmount = false;
- if (!cfgpart) {
- return false;
- }
- printf_debug("Checking device: %s\n", cfgpart->devpath);
- if (!(cfgpart->mountpoint = get_mountpoint(cfgpart->devpath))) {
- /* partition is not mounted */
- cfgpart->not_mounted = true;
- VERBOSE(stdout, "Partition %s is not mounted.\n",
- cfgpart->devpath);
- if (!mount_partition(cfgpart)) {
- return false;
- }
- do_unmount = true;
- } else {
- cfgpart->not_mounted = false;
- }
-
- if (cfgpart->mountpoint) {
- /* partition is mounted to mountpoint, either before or by this
- * program */
- VERBOSE(stdout, "Partition %s is mounted to %s.\n",
- cfgpart->devpath, cfgpart->mountpoint);
- bool result = false;
- FILE *config;
- if (!(config = open_config_file(cfgpart, "rb"))) {
- printf_debug(
- "Could not open config file on partition %s.\n",
- FAT_ENV_FILENAME);
- } else {
- result = true;
- if (fclose(config)) {
- VERBOSE(stderr, "Error closing config file on "
- "partition %s.\n",
- cfgpart->devpath);
- }
- }
- if (do_unmount) {
- unmount_partition(cfgpart);
- }
- return result;
- }
- return false;
-}
-
-bool probe_config_partitions(CONFIG_PART *cfgpart)
-{
- PedDevice *dev = NULL;
- char devpath[4096];
- int count = 0;
-
- if (!cfgpart) {
- return false;
- }
-
- ped_device_probe_all();
-
- while ((dev = ped_device_get_next(dev))) {
- printf_debug("Device: %s\n", dev->model);
- PedDisk *pd = ped_disk_new(dev);
- if (!pd) {
- continue;
- }
- PedPartition *part = pd->part_list;
- while (part) {
- if (!part->fs_type || !part->fs_type->name ||
- (strcmp(part->fs_type->name, "fat12") != 0 &&
- strcmp(part->fs_type->name, "fat16") != 0 &&
- strcmp(part->fs_type->name, "fat32") != 0)) {
- part = ped_disk_next_partition(pd, part);
- continue;
- }
- if (strncmp("/dev/mmcblk", dev->path, 11) == 0) {
- (void)snprintf(devpath, 4096, "%sp%u",
- dev->path, part->num);
- } else {
- (void)snprintf(devpath, 4096, "%s%u",
- dev->path, part->num);
- }
- if (!cfgpart[count].devpath) {
- cfgpart[count].devpath =
- malloc(strlen(devpath) + 1);
- if (!cfgpart[count].devpath) {
- VERBOSE(stderr, "Out of memory.");
- return false;
- }
- }
- strncpy(cfgpart[count].devpath, devpath,
- strlen(devpath) + 1);
- if (probe_config_file(&cfgpart[count])) {
- printf_debug("%s", "Environment file found.\n");
- if (count >= ENV_NUM_CONFIG_PARTS) {
- VERBOSE(stderr, "Error, there are "
- "more than %d config "
- "partitions.\n",
- ENV_NUM_CONFIG_PARTS);
- return false;
- }
- count++;
- }
- part = ped_disk_next_partition(pd, part);
- }
- }
- if (count < ENV_NUM_CONFIG_PARTS) {
- VERBOSE(stderr,
- "Error, less than %d config partitions exist.\n",
- ENV_NUM_CONFIG_PARTS);
- return false;
- }
- return true;
-}
-
bool read_env(CONFIG_PART *part, BG_ENVDATA *env)
{
if (!part) {
@@ -286,7 +74,7 @@ bool read_env(CONFIG_PART *part, BG_ENVDATA *env)
}
result = false;
}
- if (fclose(config)) {
+ if (close_config_file(config)) {
VERBOSE(stderr,
"Error closing environment file after reading.\n");
};
@@ -321,7 +109,7 @@ bool write_env(CONFIG_PART *part, BG_ENVDATA *env)
part->devpath);
result = false;
}
- if (fclose(config)) {
+ if (close_config_file(config)) {
VERBOSE(stderr,
"Error closing environment file after writing.\n");
result = false;
diff --git a/env/env_config_file.c b/env/env_config_file.c
new file mode 100644
index 0000000..3a802f2
--- /dev/null
+++ b/env/env_config_file.c
@@ -0,0 +1,90 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Andreas Reichel <andreas.r...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#include <stdlib.h>
+#include <stdio.h>
+#include "env_api.h"
+#include "env_disk_utils.h"
+#include "env_config_file.h"
+
+FILE *open_config_file(CONFIG_PART *cfgpart, char *mode)
+{
+ char *configfilepath;
+ configfilepath = (char *)malloc(strlen(FAT_ENV_FILENAME) +
+ strlen(cfgpart->mountpoint) + 2);
+ if (!configfilepath) {
+ return NULL;
+ }
+ strncpy(configfilepath, cfgpart->mountpoint,
+ strlen(cfgpart->mountpoint) + 1);
+ strncat(configfilepath, "/", 1);
+ strncat(configfilepath, FAT_ENV_FILENAME, strlen(FAT_ENV_FILENAME));
+ VERBOSE(stdout, "Probing config file at %s.\n", configfilepath);
+ FILE *config = fopen(configfilepath, mode);
+ free(configfilepath);
+ return config;
+}
+
+int close_config_file(FILE *config_file_handle)
+{
+ if (config_file_handle)
+ {
+ return fclose(config_file_handle);
+ }
+}
+
+bool probe_config_file(CONFIG_PART *cfgpart)
+{
+ bool do_unmount = false;
+ if (!cfgpart) {
+ return false;
+ }
+ printf_debug("Checking device: %s\n", cfgpart->devpath);
+ if (!(cfgpart->mountpoint = get_mountpoint(cfgpart->devpath))) {
+ /* partition is not mounted */
+ cfgpart->not_mounted = true;
+ VERBOSE(stdout, "Partition %s is not mounted.\n",
+ cfgpart->devpath);
+ if (!mount_partition(cfgpart)) {
+ return false;
+ }
+ do_unmount = true;
+ } else {
+ cfgpart->not_mounted = false;
+ }
+
+ if (cfgpart->mountpoint) {
+ /* partition is mounted to mountpoint, either before or by this
+ * program */
+ VERBOSE(stdout, "Partition %s is mounted to %s.\n",
+ cfgpart->devpath, cfgpart->mountpoint);
+ bool result = false;
+ FILE *config;
+ if (!(config = open_config_file(cfgpart, "rb"))) {
+ printf_debug(
+ "Could not open config file on partition %s.\n",
+ FAT_ENV_FILENAME);
+ } else {
+ result = true;
+ if (fclose(config)) {
+ VERBOSE(stderr, "Error closing config file on "
+ "partition %s.\n",
+ cfgpart->devpath);
+ }
+ }
+ if (do_unmount) {
+ unmount_partition(cfgpart);
+ }
+ return result;
+ }
+ return false;
+}
diff --git a/env/env_config_partitions.c b/env/env_config_partitions.c
new file mode 100644
index 0000000..b4ffc6c
--- /dev/null
+++ b/env/env_config_partitions.c
@@ -0,0 +1,83 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Andreas Reichel <andreas.r...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#include "env_api.h"
+#include "ebgpart.h"
+#include "env_config_partitions.h"
+#include "env_config_file.h"
+
+bool probe_config_partitions(CONFIG_PART *cfgpart)
+{
+ PedDevice *dev = NULL;
+ char devpath[4096];
+ int count = 0;
+
+ if (!cfgpart) {
+ return false;
+ }
+
+ ped_device_probe_all();
+
+ while ((dev = ped_device_get_next(dev))) {
+ printf_debug("Device: %s\n", dev->model);
+ PedDisk *pd = ped_disk_new(dev);
+ if (!pd) {
+ continue;
+ }
+ PedPartition *part = pd->part_list;
+ while (part) {
+ if (!part->fs_type || !part->fs_type->name ||
+ (strcmp(part->fs_type->name, "fat12") != 0 &&
+ strcmp(part->fs_type->name, "fat16") != 0 &&
+ strcmp(part->fs_type->name, "fat32") != 0)) {
+ part = ped_disk_next_partition(pd, part);
+ continue;
+ }
+ if (strncmp("/dev/mmcblk", dev->path, 11) == 0) {
+ (void)snprintf(devpath, 4096, "%sp%u",
+ dev->path, part->num);
+ } else {
+ (void)snprintf(devpath, 4096, "%s%u",
+ dev->path, part->num);
+ }
+ if (!cfgpart[count].devpath) {
+ cfgpart[count].devpath =
+ malloc(strlen(devpath) + 1);
+ if (!cfgpart[count].devpath) {
+ VERBOSE(stderr, "Out of memory.");
+ return false;
+ }
+ }
+ strncpy(cfgpart[count].devpath, devpath,
+ strlen(devpath) + 1);
+ if (probe_config_file(&cfgpart[count])) {
+ printf_debug("%s", "Environment file found.\n");
+ if (count >= ENV_NUM_CONFIG_PARTS) {
+ VERBOSE(stderr, "Error, there are "
+ "more than %d config "
+ "partitions.\n",
+ ENV_NUM_CONFIG_PARTS);
+ return false;
+ }
+ count++;
+ }
+ part = ped_disk_next_partition(pd, part);
+ }
+ }
+ if (count < ENV_NUM_CONFIG_PARTS) {
+ VERBOSE(stderr,
+ "Error, less than %d config partitions exist.\n",
+ ENV_NUM_CONFIG_PARTS);
+ return false;
+ }
+ return true;
+}
diff --git a/env/env_disk_utils.c b/env/env_disk_utils.c
new file mode 100644
index 0000000..344cd59
--- /dev/null
+++ b/env/env_disk_utils.c
@@ -0,0 +1,99 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Andreas Reichel <andreas.r...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#include <stdlib.h>
+#include <mntent.h>
+#include <string.h>
+#include "env_api.h"
+#include "env_disk_utils.h"
+
+const char *tmp_mnt_dir = "/tmp/mnt-XXXXXX";
+
+char *get_mountpoint(char *devpath)
+{
+ struct mntent *part = NULL;
+ FILE *mtab = NULL;
+
+ if ((mtab = setmntent("/proc/mounts", "r")) == NULL)
+ return NULL;
+
+ while ((part = getmntent(mtab)) != NULL) {
+ if ((part->mnt_fsname != NULL) &&
+ (strcmp(part->mnt_fsname, devpath)) == 0) {
+ char *mntpoint;
+
+ if (!(mntpoint =
+ malloc(strlen(part->mnt_dir) + 1))) {
+ break;
+ };
+ strncpy(mntpoint, part->mnt_dir,
+ strlen(part->mnt_dir) + 1);
+ return mntpoint;
+ }
+ }
+ endmntent(mtab);
+
+ return NULL;
+}
+
+bool mount_partition(CONFIG_PART *cfgpart)
+{
+ char tmpdir_template[256];
+ char *mountpoint;
+ (void)snprintf(tmpdir_template, 256, "%s", tmp_mnt_dir);
+ if (!cfgpart) {
+ return false;
+ }
+ if (!cfgpart->devpath) {
+ return false;
+ }
+ if (!(mountpoint = mkdtemp(tmpdir_template))) {
+ VERBOSE(stderr, "Error creating temporary mount point.\n");
+ return false;
+ }
+ if (mount(cfgpart->devpath, mountpoint, "vfat", 0, "")) {
+ VERBOSE(stderr, "Error mounting to temporary mount point.\n");
+ if (rmdir(tmpdir_template)) {
+ VERBOSE(stderr,
+ "Error deleting temporary directory.\n");
+ }
+ return false;
+ }
+ cfgpart->mountpoint = (char *)malloc(strlen(mountpoint) + 1);
+ if (!cfgpart->mountpoint) {
+ VERBOSE(stderr, "Error, out of memory.\n");
+ return false;
+ }
+ strncpy(cfgpart->mountpoint, mountpoint, strlen(mountpoint) + 1);
+ return true;
+}
+
+void unmount_partition(CONFIG_PART *cfgpart)
+{
+ if (!cfgpart) {
+ return;
+ }
+ if (!cfgpart->mountpoint) {
+ return;
+ }
+ if (umount(cfgpart->mountpoint)) {
+ VERBOSE(stderr, "Error unmounting temporary mountpoint %s.\n",
+ cfgpart->mountpoint);
+ }
+ if (rmdir(cfgpart->mountpoint)) {
+ VERBOSE(stderr, "Error deleting temporary directory %s.\n",
+ cfgpart->mountpoint);
+ }
+ free(cfgpart->mountpoint);
+ cfgpart->mountpoint = NULL;
+}
+
diff --git a/include/env_config_file.h b/include/env_config_file.h
new file mode 100644
index 0000000..b172142
--- /dev/null
+++ b/include/env_config_file.h
@@ -0,0 +1,20 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Andreas Reichel <andreas.r...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#ifndef __ENV_CONFIG_FILE_H__
+#define __ENV_CONFIG_FILE_H__
+
+FILE *open_config_file(CONFIG_PART *cfgpart, char *mode);
+int close_config_file(FILE *config_file_handle);
+bool probe_config_file(CONFIG_PART *cfgpart);
+
+#endif // __ENV_CONFIG_FILE_H__
diff --git a/include/env_config_partitions.h b/include/env_config_partitions.h
new file mode 100644
index 0000000..01f8d94
--- /dev/null
+++ b/include/env_config_partitions.h
@@ -0,0 +1,18 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Andreas Reichel <andreas.r...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#ifndef __ENV_CONFIG_PARTITIONS_H__
+#define __ENV_CONFIG_PARTITIONS_H__
+
+bool probe_config_partitions(CONFIG_PART *cfgpart);
+
+#endif // __ENV_CONFIG_PARTITIONS_H__
diff --git a/include/env_disk_utils.h b/include/env_disk_utils.h
new file mode 100644
index 0000000..9ceb019
--- /dev/null
+++ b/include/env_disk_utils.h
@@ -0,0 +1,20 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Andreas Reichel <andreas.r...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#ifndef __ENV_DISK_UTILS_H__
+#define __ENV_DISK_UTILS_H__
+
+char *get_mountpoint(char *devpath);
+bool mount_partition(CONFIG_PART *cfgpart);
+void unmount_partition(CONFIG_PART *cfgpart);
+
+#endif // __ENV_DISK_UTILS_H__
--
2.14.2

Andreas J. Reichel

unread,
Nov 2, 2017, 11:57:41 AM11/2/17
to efibootg...@googlegroups.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>

Generate fake devices and fake device enumeration to test config
partition probing.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---
tools/tests/Makefile.am | 9 ++-
tools/tests/fake_devices.c | 94 +++++++++++++++++++++++++++++
tools/tests/fake_devices.h | 28 +++++++++
tools/tests/test_probe_config_partitions.c | 95 ++++++++++++++++++++++++++++++
4 files changed, 225 insertions(+), 1 deletion(-)
create mode 100644 tools/tests/fake_devices.c
create mode 100644 tools/tests/fake_devices.h
create mode 100644 tools/tests/test_probe_config_partitions.c

diff --git a/tools/tests/Makefile.am b/tools/tests/Makefile.am
index f5bc4a4..72e9a49 100644
--- a/tools/tests/Makefile.am
+++ b/tools/tests/Makefile.am
@@ -44,7 +44,8 @@ libtest_env_api_fat_a_SOURCES = $(libtest_env_api_fat_a_SRC)
libenvapi_testlib_fat.a: libtest_env_api_fat.a
$(OBJCOPY) --weaken $^ $@

-check_PROGRAMS = test_bgenv_init_retval
+check_PROGRAMS = test_bgenv_init_retval \
+ test_probe_config_partitions

FAT_TESTLIB=libenvapi_testlib_fat.a

@@ -54,4 +55,10 @@ test_bgenv_init_retval_CFLAGS = $(AM_CFLAGS)
test_bgenv_init_retval_SOURCES = test_bgenv_init_retval.c $(SRC_TEST_COMMON)
test_bgenv_init_retval_LDADD = $(FAT_TESTLIB) $(LIBCHECK_LIBS)

+test_probe_config_partitions_CFLAGS = $(AM_CFLAGS)
+test_probe_config_partitions_SOURCES = test_probe_config_partitions.c \
+ fake_devices.c \
+ $(SRC_TEST_COMMON)
+test_probe_config_partitions_LDADD = $(FAT_TESTLIB) $(LIBCHECK_LIBS)
+
TESTS = $(check_PROGRAMS)
diff --git a/tools/tests/fake_devices.c b/tools/tests/fake_devices.c
new file mode 100644
index 0000000..58a47bd
--- /dev/null
+++ b/tools/tests/fake_devices.c
@@ -0,0 +1,94 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Andreas Reichel <andreas.r...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#include <stdlib.h>
+#include <env_api.h>
+#include <env_config_file.h>
+#include <env_config_partitions.h>
+
+ for (int i = 0; i < num_fake_devices; i++) {
+ if (fake_devices[i].model)
+ free(fake_devices[i].model);
+ if (fake_devices[i].path)
+ free(fake_devices[i].path);
+ if (fake_devices[i].part_list)
+ remove_fake_partitions(i);
+ }
+
+ free(fake_devices);
+}
+
+PedDevice *ped_device_get_next_custom_fake(const PedDevice *dev)
+{
+ if (!dev)
+ return fake_devices;
+
+ return dev->next;
+}
diff --git a/tools/tests/fake_devices.h b/tools/tests/fake_devices.h
new file mode 100644
index 0000000..aa5f423
--- /dev/null
+++ b/tools/tests/fake_devices.h
@@ -0,0 +1,28 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Andreas Reichel <andreas.r...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#ifndef __FAKE_DEVICES_H__
+#define __FAKE_DEVICES_H__
+
+#include <ebgpart.h>
+
+extern PedDevice *fake_devices;
+extern int num_fake_devices;
+
+void allocate_fake_devices(int n);
+void add_fake_partition(int devnum);
+void remove_fake_partitions(int n);
+void free_fake_devices(void);
+
+PedDevice *ped_device_get_next_custom_fake(const PedDevice *dev);
+
+#endif // __FAKE_DEVICES_H__
diff --git a/tools/tests/test_probe_config_partitions.c b/tools/tests/test_probe_config_partitions.c
new file mode 100644
index 0000000..4536aea
--- /dev/null
+++ b/tools/tests/test_probe_config_partitions.c
@@ -0,0 +1,95 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Andreas Reichel <andreas.r...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#include <stdlib.h>
+#include <check.h>
+#include <fff.h>
+#include <env_api.h>
+#include <env_config_file.h>
+#include <env_config_partitions.h>
+#include <fake_devices.h>
+
+DEFINE_FFF_GLOBALS;
+
+Suite *ebg_test_suite(void);
+
+bool read_env_custom_fake(CONFIG_PART *cp, BG_ENVDATA *env);
+bool read_env(CONFIG_PART *part, BG_ENVDATA *env);
+
+bool read_env_custom_fake(CONFIG_PART *cp, BG_ENVDATA *env)
+{
+ if (!env) {
+ return false;
+ }
+ memset(env, 0, sizeof(BG_ENVDATA));
+ return true;
+}
+
+FAKE_VALUE_FUNC(bool, read_env, CONFIG_PART *, BG_ENVDATA *);
+FAKE_VOID_FUNC(ped_device_probe_all);
+FAKE_VALUE_FUNC(PedDevice *, ped_device_get_next, const PedDevice *);
+
+START_TEST(env_api_fat_test_probe_config_partitions)
+{
+ bool result;
+ /* In this unit test, contents of environment data are
+ * faked to be all zero
+ */
+
+ /* Test if bgenv_init fails if no block devices are found
+ */
+ RESET_FAKE(ped_device_probe_all);
+ RESET_FAKE(ped_device_get_next);
+
+ ped_device_get_next_fake.return_val = NULL;
+
+ result = bgenv_init();
+
+ ck_assert(ped_device_probe_all_fake.call_count == 1);
+ ck_assert(result == false);
+
+ /* Test if bgenv_init fails if a device with two partitions is found
+ * but now config file is there
+ */
+ RESET_FAKE(ped_device_probe_all);
+ RESET_FAKE(ped_device_get_next);
+
+ allocate_fake_devices(1);
+
+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {
+ add_fake_partition(0);
+ }
+
+ ped_device_get_next_fake.custom_fake = ped_device_get_next_custom_fake;
+ result = bgenv_init();
+
+ free_fake_devices();
+
+ ck_assert(ped_device_probe_all_fake.call_count == 1);
+ ck_assert(ped_device_get_next_fake.call_count == 2);
+ ck_assert(result == false);
+}
+END_TEST
+
+Suite *ebg_test_suite(void)
+{
+ Suite *s;
+ TCase *tc_core;
+
+ s = suite_create("env_api_fat");
+
+ tc_core = tcase_create("Core");
+ tcase_add_test(tc_core, env_api_fat_test_probe_config_partitions);
+ suite_add_tcase(s, tc_core);
+
+ return s;
+}
--
2.14.2

Jan Kiszka

unread,
Nov 2, 2017, 2:22:28 PM11/2/17
to [ext] Andreas J. Reichel, efibootg...@googlegroups.com
Looks all good and seems to work. I've decided last-minute to rename
testing/fff to tests/fff (to be consistent with other test-related
folders) and merged things to next.

Thanks,
Reply all
Reply to author
Forward
0 new messages