[PATCH 0/5] cppcheck update fallouts

[Jan Kiszka]

Caching of the old cppcheck version unfortunately papered over the fact
that the update broke its build. And then the new version found new old
issues. All addressed in the following.

Jan

Jan Kiszka (5):
ci: Update cppcheck build to the chosen release
env: Fix reporting of BG_CONFIG_PARTIALLY_CORRUPTED from load_config
Avoid shadowing of variables or function names
tests: Improve result evaluations
ci: Suppress false positives of cppcheck

.travis-build.sh | 8 ++++++--
env/env_api.c | 2 +-
env/fatvars.c | 3 ++-
tools/ebgpart.c | 12 ++++++------
tools/tests/test_ebgenv_api.c | 1 +
tools/tests/test_ebgenv_api_internal.c | 4 ++++
tools/tests/test_probe_config_file.c | 26 +++++++++++++-------------
7 files changed, 33 insertions(+), 23 deletions(-)

--
2.26.2

[Jan Kiszka]

From: Jan Kiszka <jan.k...@siemens.com>

Further cppcheck findings, but all harmless ones.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---
env/env_api.c | 2 +-
tools/ebgpart.c | 12 ++++++------
tools/tests/test_probe_config_file.c | 26 +++++++++++++-------------
3 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/env/env_api.c b/env/env_api.c
index 4fa759d..3e39084 100644
--- a/env/env_api.c
+++ b/env/env_api.c
@@ -148,7 +148,7 @@ uint16_t ebg_env_getglobalstate(ebgenv_t *e)

/* find all environments with revision 0 */
for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {
- BGENV *env = bgenv_open_by_index(i);
+ env = bgenv_open_by_index(i);

if (!env) {
continue;
diff --git a/tools/ebgpart.c b/tools/ebgpart.c
index fc3bf3b..5607465 100644
--- a/tools/ebgpart.c
+++ b/tools/ebgpart.c
@@ -393,13 +393,13 @@ static int scan_devdir(unsigned int fmajor, unsigned int fminor, char *fullname,
}
(void)snprintf(fullname, maxlen, "%s/%s", DEVDIR,
devfile->d_name);
- struct stat fstat;
- if (stat(fullname, &fstat) == -1) {
+ struct stat statbuf;
+ if (stat(fullname, &statbuf) == -1) {
VERBOSE(stderr, "stat failed on %s\n", fullname);
break;
}
- if (major(fstat.st_rdev) == fmajor &&
- minor(fstat.st_rdev) == fminor) {
+ if (major(statbuf.st_rdev) == fmajor &&
+ minor(statbuf.st_rdev) == fminor) {
VERBOSE(stdout, "Node found: %s\n", fullname);
result = 0;
break;
@@ -462,8 +462,8 @@ void ped_device_probe_all(void)
/* Check if this file is really in the dev directory */
(void)snprintf(fullname, sizeof(fullname), "%s/%s", DEVDIR,
sysblockfile->d_name);
- struct stat fstat;
- if (stat(fullname, &fstat) == -1) {
+ struct stat statbuf;
+ if (stat(fullname, &statbuf) == -1) {
/* Node with same name not found in /dev, thus search
* for node with identical Major and Minor revision */
if (scan_devdir(fmajor, fminor, fullname,
diff --git a/tools/tests/test_probe_config_file.c b/tools/tests/test_probe_config_file.c
index 439d5d4..fb5055b 100644
--- a/tools/tests/test_probe_config_file.c
+++ b/tools/tests/test_probe_config_file.c
@@ -48,40 +48,40 @@ 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;
+ char *tempdir = NULL;
+ char *tempfile = NULL;

- if (asprintf(&tmpdir, "%s", fake_mountpoint) == -1) {
- tmpdir = NULL;
+ if (asprintf(&tempdir, "%s", fake_mountpoint) == -1) {
+ tempdir = NULL;
goto fake_mountpoint_error;
}

- tmpdir = mkdtemp(tmpdir);
- if (!tmpdir) {
+ tempdir = mkdtemp(tempdir);
+ if (!tempdir) {
goto fake_mountpoint_error;
}

- if (asprintf(&buff, "%s", tmpdir) == -1) {
+ if (asprintf(&buff, "%s", tempdir) == -1) {
buff = NULL;
goto fake_mountpoint_error;
}

- if (asprintf(&tmpfile, "%s/%s", tmpdir, FAT_ENV_FILENAME) == -1) {
- tmpfile = NULL;
+ if (asprintf(&tempfile, "%s/%s", tempdir, FAT_ENV_FILENAME) == -1) {
+ tempfile = NULL;
goto fake_mountpoint_error;
}

/* create a fake environment file
*/
- FILE *temp_env_file = fopen(tmpfile, "w");
+ FILE *temp_env_file = fopen(tempfile, "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);
+ free(tempfile);
+ free(tempdir);

struct fake_env_file_path *fefp;
fefp = malloc(sizeof(struct fake_env_file_path));
@@ -103,7 +103,7 @@ char *get_mountpoint_custom_fake(char *devpath)
fake_mountpoint_error:
free(fefp);
free(buff);
- free(tmpdir);
+ free(tempdir);
return NULL;
}

--
2.26.2

[Jan Kiszka]

From: Jan Kiszka <jan.k...@siemens.com>

The init_array walk in probe_watchdog() is absolutely correct, cppcheck
just cannot see that compatible pointers are compared.

It's unclear why cppcheck misses the fact that init() functions are
declared as constructors, thus are used.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

.travis-build.sh | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/.travis-build.sh b/.travis-build.sh
index 4a7242f..5eeb8d1 100755
--- a/.travis-build.sh
+++ b/.travis-build.sh
@@ -127,6 +127,10 @@ case "$TARGET_EFFECTIVE" in
suppress+=" --suppress=unusedFunction:tools/tests/test_ebgenv_api.c"
# EFI uses void* as ImageBase needed for further calculations
suppress+=" --suppress=arithOperationsOnVoidPointer:main.c"
+ # False positive on init_array iteration
+ suppress+=" --suppress=comparePointers:main.c"
+ # False positive on constructors, first hit
+ suppress+=" --suppress=unusedFunction:drivers/watchdog/amdfch_wdt.c"

enable="--enable=warning \
--enable=style \
--
2.26.2

[Jan Kiszka]

From: Jan Kiszka <jan.k...@siemens.com>

cppcheck noted that some results are not properly evaluated. Easy to
fix.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

tools/tests/test_ebgenv_api.c | 1 +
tools/tests/test_ebgenv_api_internal.c | 4 ++++

2 files changed, 5 insertions(+)

diff --git a/tools/tests/test_ebgenv_api.c b/tools/tests/test_ebgenv_api.c
index 12716a7..ea4523f 100644
--- a/tools/tests/test_ebgenv_api.c
+++ b/tools/tests/test_ebgenv_api.c
@@ -147,6 +147,7 @@ START_TEST(ebgenv_api_ebg_env_create_new)
*/
ret = ebg_env_create_new(&e);

+ ck_assert_int_eq(ret, 0);
ck_assert(((BGENV *)e.bgenv)->data == &envdata[0]);
ck_assert_int_eq(((BGENV *)e.bgenv)->data->ustate, USTATE_OK);
ck_assert_int_eq(
diff --git a/tools/tests/test_ebgenv_api_internal.c b/tools/tests/test_ebgenv_api_internal.c
index 1a5dfa0..795375f 100644
--- a/tools/tests/test_ebgenv_api_internal.c
+++ b/tools/tests/test_ebgenv_api_internal.c
@@ -306,10 +306,14 @@ START_TEST(ebgenv_api_internal_bgenv_get)
/* Test if bgenv_get returns the correct value
*/
res = bgenv_get(handle, "kernelfile", NULL, buffera, res);
+ ck_assert_int_eq(res, 0);
ck_assert_int_eq(strcmp(buffera, test_strings[0]), 0);

res = bgenv_get(handle, "kernelparams", NULL, NULL, 1000);
+ ck_assert_int_eq(res, strlen(test_strings[1]) + 1);
+
res = bgenv_get(handle, "kernelparams", NULL, buffera, res);
+ ck_assert_int_eq(res, 0);
ck_assert_int_eq(strcmp(buffera, test_strings[1]), 0);

free(handle);
--
2.26.2

[Jan Kiszka]

From: Jan Kiszka <jan.k...@siemens.com>

This was apparently papered over by caching in CI: Some variables have
changed that we need to provide to the build.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

.travis-build.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/.travis-build.sh b/.travis-build.sh
index cce8adf..4a7242f 100755
--- a/.travis-build.sh
+++ b/.travis-build.sh
@@ -64,8 +64,8 @@ install_cppcheck()
{
git clone https://github.com/danmar/cppcheck.git
git -C cppcheck checkout 2.3
- make -C cppcheck SRCDIR=build \
- CFGDIR=/opt/cppcheck/cfg \
+ make -C cppcheck MATCHCOMPILER=yes \
+ FILESDIR=/opt/cppcheck \
PREFIX=/opt/cppcheck \
HAVE_RULES=no install -j2 || \
return -1
--
2.26.2

[Jan Kiszka]

From: Jan Kiszka <jan.k...@siemens.com>

That error code was overwritting by the concluding BG_SUCCESS setting so
far. Found by cppcheck.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

env/fatvars.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/env/fatvars.c b/env/fatvars.c
index 99ac37e..81b4432 100644
--- a/env/fatvars.c
+++ b/env/fatvars.c
@@ -111,6 +111,8 @@ BG_STATUS load_config(BG_LOADER_PARAMS *bglp)
goto lc_cleanup;
}

+ result = BG_SUCCESS;
+
if (numHandles < ENV_NUM_CONFIG_PARTS) {
Print(L"Warning, too few config partitions: found: "
L"%d, but expected %d.\n",
@@ -227,7 +229,6 @@ BG_STATUS load_config(BG_LOADER_PARAMS *bglp)
Print(L" args: %s\n", bglp->payload_options);
Print(L" timeout: %d seconds\n", bglp->timeout);

- result = BG_SUCCESS;
lc_cleanup:
mfree(config_volumes);
return result;
--
2.26.2