[PATCH 0/7] cppcheck fixes / CI refactoring / separate driver helpers

[Jan Kiszka]

This addresses the new cppcheck finding plus some reported locally. It
also helps to perform such local checks more easily in the future by
pulling out a cppcheck script.

Finally, this moves driver helper functions under drivers/utils and
avoids including them when there is no driver that needs them.

Jan

Jan Kiszka (7):
scripts: Factor out cppcheck.sh from CI job
cppcheck: Add exception for new warning in w83627hf_wdt.c
watchdog: ipmi_wdt: Avoid pointless check at loop entry
libebgenv: Address cppcheck style complaint
cppcheck: Suppress unusedStructMember in kernel-stub/fdt.c
drivers: Move smbios helper out of utils into separate folder
drivers: utils: Relocate simatic.c

.github/workflows/main.yaml | 54 +--------------------
Makefile.am | 8 +++-
simatic.c => drivers/utils/simatic.c | 1 +
drivers/utils/smbios.c | 41 ++++++++++++++++
drivers/watchdog/ipmi_wdt.c | 7 +--
include/smbios.h | 21 ++++++++
include/utils.h | 3 --
scripts/cppcheck.sh | 72 ++++++++++++++++++++++++++++
tools/ebgpart.c | 7 ++-
utils.c | 25 ----------
10 files changed, 149 insertions(+), 90 deletions(-)
rename simatic.c => drivers/utils/simatic.c (98%)
create mode 100644 drivers/utils/smbios.c
create mode 100644 include/smbios.h
create mode 100755 scripts/cppcheck.sh

--
2.35.3

[Jan Kiszka]

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

This is only needed by x86 drivers.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---
Makefile.am | 2 +-
simatic.c => drivers/utils/simatic.c | 0
2 files changed, 1 insertion(+), 1 deletion(-)
rename simatic.c => drivers/utils/simatic.c (100%)

diff --git a/Makefile.am b/Makefile.am
index 28848e6..e95bae7 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -175,6 +175,7 @@ efi_sources_watchdogs = \
drivers/watchdog/ipmi_wdt.c \
drivers/watchdog/itco.c \
drivers/watchdog/hpwdt.c \
+ drivers/utils/simatic.c \
drivers/utils/smbios.c
else
efi_sources_watchdogs =
@@ -188,7 +189,6 @@ efi_sources = \
env/fatvars.c \
utils.c \
loader_interface.c \
- simatic.c \
bootguard.c \
main.c

diff --git a/simatic.c b/drivers/utils/simatic.c
similarity index 100%
rename from simatic.c
rename to drivers/utils/simatic.c
--
2.35.3

[Jan Kiszka]

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

tools/ebgpart.c:380:11: style: Condition 'devfile' is always true [knownConditionTrueFalse]
} while (devfile);
^

While resolving this, also reduce the scope of devfile (would be the
next warning).

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

tools/ebgpart.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/tools/ebgpart.c b/tools/ebgpart.c
index fba7fb2..b6384f2 100644
--- a/tools/ebgpart.c
+++ b/tools/ebgpart.c
@@ -358,9 +358,8 @@ static int scan_devdir(unsigned int fmajor, unsigned int fminor, char *fullname,
VERBOSE(stderr, "Failed to open %s\n", DEVDIR);
return result;
}
- struct dirent *devfile;
- do {
- devfile = readdir(devdir);
+ while (true) {
+ struct dirent *devfile = readdir(devdir);
if (!devfile) {
break;
}
@@ -377,7 +376,7 @@ static int scan_devdir(unsigned int fmajor, unsigned int fminor, char *fullname,
result = 0;
break;
}
- } while (devfile);
+ }
closedir(devdir);

return result;
--
2.35.3

[Jan Kiszka]

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

These are there because of the file format.

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

scripts/cppcheck.sh | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/cppcheck.sh b/scripts/cppcheck.sh
index b01421f..af5f337 100755
--- a/scripts/cppcheck.sh
+++ b/scripts/cppcheck.sh
@@ -40,8 +40,9 @@ suppress+=" --suppress=comparePointers:main.c"
suppress+=" --suppress=unusedFunction:drivers/watchdog/amdfch_wdt.c"
# False positive, noreturn is not recognized
suppress+=" --suppress=nullPointerRedundantCheck:kernel-stub/main.c"
-# Avoid noise regarding Ignore* fields
+# Avoid noise regarding Ignore* or otherwise unused fields
suppress+=" --suppress=unusedStructMember:kernel-stub/main.c"
+suppress+=" --suppress=unusedStructMember:kernel-stub/fdt.c"
# Not applicable because of API requirements
suppress+=" --suppress=constParameter:drivers/watchdog/ipc4x7e_wdt.c"
suppress+=" --suppress=constParameter:drivers/watchdog/w83627hf_wdt.c"
--
2.35.3

[Jan Kiszka]

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

This is analogous to ipc4x7e_wdt.c.

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

scripts/cppcheck.sh | 1 +
1 file changed, 1 insertion(+)

diff --git a/scripts/cppcheck.sh b/scripts/cppcheck.sh
index 1267f21..b01421f 100755
--- a/scripts/cppcheck.sh
+++ b/scripts/cppcheck.sh
@@ -44,6 +44,7 @@ suppress+=" --suppress=nullPointerRedundantCheck:kernel-stub/main.c"
suppress+=" --suppress=unusedStructMember:kernel-stub/main.c"

# Not applicable because of API requirements
suppress+=" --suppress=constParameter:drivers/watchdog/ipc4x7e_wdt.c"

+suppress+=" --suppress=constParameter:drivers/watchdog/w83627hf_wdt.c"
suppress+=" --suppress=constParameter:kernel-stub/initrd.c"

enable="--enable=warning \
--
2.35.3

[Jan Kiszka]

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

This allows to pull in smbios_find_struct only into builds which need
them (x86). And it also avoids filling the main folder with driver-only
code.

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

Makefile.am | 6 +++++-
drivers/utils/smbios.c | 41 +++++++++++++++++++++++++++++++++++++
drivers/watchdog/ipmi_wdt.c | 1 +
include/smbios.h | 21 +++++++++++++++++++
include/utils.h | 3 ---
simatic.c | 1 +
utils.c | 25 ----------------------
7 files changed, 69 insertions(+), 29 deletions(-)

create mode 100644 drivers/utils/smbios.c
create mode 100644 include/smbios.h

diff --git a/Makefile.am b/Makefile.am
index b7b3936..28848e6 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -174,7 +174,8 @@ efi_sources_watchdogs = \
drivers/watchdog/w83627hf_wdt.c \
drivers/watchdog/ipmi_wdt.c \
drivers/watchdog/itco.c \
- drivers/watchdog/hpwdt.c
+ drivers/watchdog/hpwdt.c \
+ drivers/utils/smbios.c
else
efi_sources_watchdogs =
endif
@@ -284,6 +285,9 @@ $(top_builddir)/drivers/watchdog/%.o: $(top_srcdir)/drivers/watchdog/%.c
$(top_builddir)/drivers/watchdog/%.o: $(top_srcdir)/drivers/watchdog/%.S
$(call gnuefi_compile)

+$(top_builddir)/drivers/utils/%.o: $(top_srcdir)/drivers/utils/%.c
+ $(call gnuefi_compile)
+
$(top_builddir)/kernel-stub/%.o: $(top_srcdir)/kernel-stub/%.c
$(call gnuefi_compile)

diff --git a/drivers/utils/smbios.c b/drivers/utils/smbios.c
new file mode 100644
index 0000000..a765288
--- /dev/null
+++ b/drivers/utils/smbios.c
@@ -0,0 +1,41 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2020-2023
+ *
+ * Authors:
+ * Dr. Johann Pfefferl <johann....@siemens.com>
+ * Jan Kiszka <jan.k...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#include <smbios.h>
+
+SMBIOS_STRUCTURE_POINTER smbios_find_struct(SMBIOS_STRUCTURE_TABLE *table,
+ UINT16 type)
+{
+ SMBIOS_STRUCTURE_POINTER strct;
+ UINT8 *str;
+ UINTN n;
+
+ strct.Raw = (UINT8 *)(uintptr_t)table->TableAddress;
+
+ for (n = 0; n < table->NumberOfSmbiosStructures; n++) {
+ if (strct.Hdr->Type == type) {
+ return strct;
+ }
+ /* Read over any appended strings. */
+ str = strct.Raw + strct.Hdr->Length;
+ while (str[0] != 0 || str[1] != 0) {
+ str++;
+ }
+ strct.Raw = str + 2;
+ }
+
+ strct.Raw = NULL;
+ return strct;
+}
diff --git a/drivers/watchdog/ipmi_wdt.c b/drivers/watchdog/ipmi_wdt.c
index 55d9623..19e7813 100644
--- a/drivers/watchdog/ipmi_wdt.c
+++ b/drivers/watchdog/ipmi_wdt.c
@@ -15,6 +15,7 @@
#include <efi.h>
#include <pci/header.h>
#include <sys/io.h>
+#include "smbios.h"
#include "utils.h"

#define SMBIOS_TYPE_IPMI_KCS 38
diff --git a/include/smbios.h b/include/smbios.h
new file mode 100644
index 0000000..edc7c39
--- /dev/null
+++ b/include/smbios.h
@@ -0,0 +1,21 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2020-2023
+ *
+ * Authors:
+ * Jan Kiszka <jan.k...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#pragma once
+
+#include <efi.h>
+#include <efilib.h>
+
+SMBIOS_STRUCTURE_POINTER smbios_find_struct(SMBIOS_STRUCTURE_TABLE *table,
+ UINT16 type);
diff --git a/include/utils.h b/include/utils.h
index 15d60fb..084796e 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -56,6 +56,3 @@ VOID PrintC(const UINT8 color, const CHAR16 *fmt, ...);

#define INFO(fmt, ...) \
PrintC(EFI_LIGHTGRAY, fmt, ##__VA_ARGS__)
-
-SMBIOS_STRUCTURE_POINTER smbios_find_struct(SMBIOS_STRUCTURE_TABLE *table,
- UINT16 type);
diff --git a/simatic.c b/simatic.c
index 7309cdf..a1c63cb 100644
--- a/simatic.c
+++ b/simatic.c
@@ -17,6 +17,7 @@
#include <efi.h>
#include <efilib.h>
#include "simatic.h"
+#include "smbios.h"
#include "utils.h"

static UINT32 get_station_id(SMBIOS_STRUCTURE_POINTER oem_strct)
diff --git a/utils.c b/utils.c
index e065b6f..0ae5a5e 100644
--- a/utils.c
+++ b/utils.c
@@ -295,28 +295,3 @@ CHAR16 *GetBootMediumPath(CHAR16 *input)

return dst;
}
-
-SMBIOS_STRUCTURE_POINTER smbios_find_struct(SMBIOS_STRUCTURE_TABLE *table,
- UINT16 type)
-{
- SMBIOS_STRUCTURE_POINTER strct;
- UINT8 *str;
- UINTN n;
-
- strct.Raw = (UINT8 *)(uintptr_t)table->TableAddress;
-
- for (n = 0; n < table->NumberOfSmbiosStructures; n++) {
- if (strct.Hdr->Type == type) {
- return strct;
- }
- /* Read over any appended strings. */
- str = strct.Raw + strct.Hdr->Length;
- while (str[0] != 0 || str[1] != 0) {
- str++;
- }
- strct.Raw = str + 2;
- }
-
- strct.Raw = NULL;
- return strct;
-}
--
2.35.3

[Jan Kiszka]

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

This allows the local execution of cppcheck with the same parameters
used in CI, specifically the same exceptions.

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

.github/workflows/main.yaml | 54 +---------------------------
scripts/cppcheck.sh | 70 +++++++++++++++++++++++++++++++++++++
2 files changed, 71 insertions(+), 53 deletions(-)
create mode 100755 scripts/cppcheck.sh

diff --git a/.github/workflows/main.yaml b/.github/workflows/main.yaml
index 2b3a89e..11a3194 100644
--- a/.github/workflows/main.yaml
+++ b/.github/workflows/main.yaml
@@ -163,56 +163,4 @@ jobs:
if: ${{ matrix.target == 'cppcheck' }}
run: |
./configure
-
- ignore=""
- ignore+=" -i tests/fff"
-
- suppress=""
- # Justified suppressions:
- # 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"
- suppress+=" --suppress=unusedFunction:kernel-stub/main.c"
- # Some functions are defined for API only
- suppress+=" --suppress=unusedFunction:utils.c"
- suppress+=" --suppress=unusedFunction:env/env_api.c"
- 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"
- # 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"
- # False positive, noreturn is not recognized
- suppress+=" --suppress=nullPointerRedundantCheck:kernel-stub/main.c"
- # Avoid noise regarding Ignore* fields
- suppress+=" --suppress=unusedStructMember:kernel-stub/main.c"
- # Not applicable because of API requirements
- suppress+=" --suppress=constParameter:drivers/watchdog/ipc4x7e_wdt.c"
- suppress+=" --suppress=constParameter:kernel-stub/initrd.c"
-
- enable="--enable=warning \
- --enable=style \
- --enable=performance \
- --enable=portability \
- --enable=unusedFunction"
-
- includes="-I . \
- -I include \
- -I /usr/include \
- -I /usr/include/linux \
- -I /usr/include/efi \
- -I /usr/include/efi/x86_64 \
- -I /usr/include/x86_64-linux-gnu \
- -I /usr/lib/gcc/x86_64-linux-gnu/9/include"
-
- cpp_conf="-U__WINT_TYPE__ -U__GNUC__"
- # Exit code '1' is returned if arguments are not valid or if no input
- # files are provided. Compare 'cppcheck --help'.
- cppcheck -f -q --error-exitcode=2 $enable $suppress $ignore \
- $cpp_conf $includes .
+ scripts/cppcheck.sh
diff --git a/scripts/cppcheck.sh b/scripts/cppcheck.sh
new file mode 100755
index 0000000..1267f21
--- /dev/null
+++ b/scripts/cppcheck.sh
@@ -0,0 +1,70 @@
+#!/bin/bash
+#
+# EFI Boot Guard
+#
+# Copyright (c) Siemens AG, 2021-2023
+#
+# Authors:
+# Claudius Heine <c...@denx.de>
+# Jan Kiszka <jan.k...@siemens.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2. See
+# the COPYING file in the top-level directory.
+#
+# SPDX-License-Identifier: GPL-2.0
+#
+
+ignore=""
+ignore+=" -i tests/fff"
+
+suppress=""
+# Justified suppressions:
+# 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"
+suppress+=" --suppress=unusedFunction:kernel-stub/main.c"
+# Some functions are defined for API only
+suppress+=" --suppress=unusedFunction:utils.c"
+suppress+=" --suppress=unusedFunction:env/env_api.c"
+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"
+# 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"
+# False positive, noreturn is not recognized
+suppress+=" --suppress=nullPointerRedundantCheck:kernel-stub/main.c"
+# Avoid noise regarding Ignore* fields
+suppress+=" --suppress=unusedStructMember:kernel-stub/main.c"
+# Not applicable because of API requirements
+suppress+=" --suppress=constParameter:drivers/watchdog/ipc4x7e_wdt.c"
+suppress+=" --suppress=constParameter:kernel-stub/initrd.c"
+
+enable="--enable=warning \
+ --enable=style \
+ --enable=performance \
+ --enable=portability \
+ --enable=unusedFunction"
+
+includes="-I . \
+ -I include \
+ -I /usr/include \
+ -I /usr/include/linux \
+ -I /usr/include/efi \
+ -I /usr/include/efi/x86_64 \
+ -I /usr/include/x86_64-linux-gnu \
+ -I /usr/lib/gcc/x86_64-linux-gnu/9/include"
+
+cpp_conf="-U__WINT_TYPE__ -U__GNUC__"
+path=${1-.}
+
+# Exit code '1' is returned if arguments are not valid or if no input
+# files are provided. Compare 'cppcheck --help'.
+cppcheck -f -q --error-exitcode=2 $enable $suppress $ignore \
+ $cpp_conf $includes $path "$@"
--
2.35.3

[JEMS EBERHARD HORBEL]

DIRECT SENDER IS HERE LETS DEAL.

JENS EBERHARD



MT103/202 DIRECT WIRE TRANSFER
PAYPAL TRANSFER
CASHAPP TRANSFER
ZELLE TRANSFER
TRANSFER WISE
WESTERN UNION TRANSFER
BITCOIN FLASHING 
BANK ACCOUNT LOADING/FLASHING
IBAN TO IBAN TRANSFER
MONEYGRAM TRANSFER
SLBC PROVIDER
CREDIT CARD TOP UP
SEPA TRANSFER
WIRE TRANSFER
GLOBALPAY INC US

Thanks.


NOTE; ONLY SERIOUS / RELIABLE RECEIVERS CAN CONTACT.

DM ME ON WHATSAPP FOR A SERIOUS DEAL.

+447405129573