From 52e736eaa2cba88397ab5e5197e704e89a36ddca Mon Sep 17 00:00:00 2001 From: "Berk D. Demir" Date: Wed, 9 Jul 2025 22:05:32 -0700 Subject: [PATCH 1/8] Fix compiler warnings: Unused functions Move function definitions in include/blisp_util.h to lib/blisp_util.c. Defining them as static leads to internal linkage, accessible in that translation unit, hence the warning. Their references are to be handled in other translation units, so they shouldn't be static. --- CMakeLists.txt | 4 +++- include/blisp_util.h | 46 +++++++----------------------------------- lib/blisp_util.c | 48 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 40 deletions(-) create mode 100644 lib/blisp_util.c diff --git a/CMakeLists.txt b/CMakeLists.txt index 7a2a972..43462e8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -11,8 +11,10 @@ option(COMPILE_TESTS "Compile the tests" OFF) add_library(libblisp_obj OBJECT lib/blisp.c + lib/blisp_easy.c + lib/blisp_util.c lib/chip/blisp_chip_bl60x.c - lib/chip/blisp_chip_bl70x.c lib/blisp_easy.c) + lib/chip/blisp_chip_bl70x.c) target_include_directories(libblisp_obj PRIVATE ${CMAKE_SOURCE_DIR}/include/) diff --git a/include/blisp_util.h b/include/blisp_util.h index f1f4d8f..1f00480 100644 --- a/include/blisp_util.h +++ b/include/blisp_util.h @@ -2,35 +2,18 @@ #ifndef _BLISP_UTIL_H #define _BLISP_UTIL_H -#include -#include +#include #ifdef WIN32 -#include +# include #else -#include +# include #endif -static void blisp_dlog(const char* format, ...) -{ - fflush(stdout); - va_list args; - va_start(args, format); - vfprintf(stderr, format, args); - va_end(args); - fputc('\n', stderr); -} +void blisp_dlog(const char* format, ...); +void sleep_ms(int milliseconds); -static void sleep_ms(int milliseconds) { -#ifdef WIN32 - Sleep(milliseconds); -#else - struct timespec ts; - ts.tv_sec = milliseconds / 1000; - ts.tv_nsec = (milliseconds % 1000) * 1000000; - nanosleep(&ts, NULL); -#endif -} +uint32_t crc32_calculate(const void *data, size_t data_len); /** * * Generated on Mon Jan 9 19:56:36 2023 @@ -80,19 +63,4 @@ static const uint32_t crc_table[256] = { 0xb3667a2e, 0xc4614ab8, 0x5d681b02, 0x2a6f2b94, 0xb40bbe37, 0xc30c8ea1, 0x5a05df1b, 0x2d02ef8d }; -static uint32_t crc32_calculate(const void *data, size_t data_len) -{ - uint32_t crc = 0xffffffff; - const unsigned char *d = (const unsigned char *)data; - unsigned int tbl_idx; - - while (data_len--) { - tbl_idx = (crc ^ *d) & 0xff; - crc = (crc_table[tbl_idx] ^ (crc >> 8)) & 0xffffffff; - d++; - } - return (crc & 0xffffffff) ^ 0xffffffff; -} - - -#endif \ No newline at end of file +#endif diff --git a/lib/blisp_util.c b/lib/blisp_util.c new file mode 100644 index 0000000..27bca4f --- /dev/null +++ b/lib/blisp_util.c @@ -0,0 +1,48 @@ +// SPDX-License-Identifier: MIT +#include +#include +#include + +#ifdef WIN32 +# include +#else +# include +#endif + +#include "blisp_util.h" + +void blisp_dlog(const char* format, ...) +{ + fflush(stdout); + va_list args; + va_start(args, format); + vfprintf(stderr, format, args); + va_end(args); + fputc('\n', stderr); +} + + +void sleep_ms(int milliseconds) { +#ifdef WIN32 + Sleep(milliseconds); +#else + struct timespec ts; + ts.tv_sec = milliseconds / 1000; + ts.tv_nsec = (milliseconds % 1000) * 1000000; + nanosleep(&ts, NULL); +#endif +} + +uint32_t crc32_calculate(const void *data, size_t data_len) +{ + uint32_t crc = 0xffffffff; + const unsigned char *d = (const unsigned char *)data; + unsigned int tbl_idx; + + while (data_len--) { + tbl_idx = (crc ^ *d) & 0xff; + crc = (crc_table[tbl_idx] ^ (crc >> 8)) & 0xffffffff; + d++; + } + return (crc & 0xffffffff) ^ 0xffffffff; +} From 77df78c1affd305a10355329a84c32383fe4896a Mon Sep 17 00:00:00 2001 From: "Berk D. Demir" Date: Wed, 9 Jul 2025 22:09:37 -0700 Subject: [PATCH 2/8] Fix compiler warnings: Unused function params We could use C23's `[[maybe_unused]]` but MSVC doesn't support it. So we void cast them with an accompanying "unused" comment. --- lib/blisp.c | 4 +++- lib/chip/blisp_chip_bl60x.c | 2 ++ lib/chip/blisp_chip_bl70x.c | 2 ++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/blisp.c b/lib/blisp.c index b3d8f42..58f0ff2 100644 --- a/lib/blisp.c +++ b/lib/blisp.c @@ -16,6 +16,8 @@ static void drain(struct sp_port* port) { #if defined(__APPLE__) || defined(__FreeBSD__) sp_drain(port); +#else + (void)port; // unused #endif } @@ -414,4 +416,4 @@ blisp_return_t blisp_device_reset(struct blisp_device* device) { void blisp_device_close(struct blisp_device* device) { struct sp_port* serial_port = device->serial_port; sp_close(serial_port); -} \ No newline at end of file +} diff --git a/lib/chip/blisp_chip_bl60x.c b/lib/chip/blisp_chip_bl60x.c index e8ae033..8639390 100644 --- a/lib/chip/blisp_chip_bl60x.c +++ b/lib/chip/blisp_chip_bl60x.c @@ -6,6 +6,8 @@ int64_t blisp_chip_bl60x_get_eflash_loader(uint8_t clk_type, uint8_t** firmware_buf_ptr) { + (void)clk_type; // unused + uint8_t* firmware_buf = malloc(sizeof(bl60x_eflash_loader_bin)); memcpy(firmware_buf, bl60x_eflash_loader_bin, sizeof(bl60x_eflash_loader_bin)); *(firmware_buf + 0xE0) = 4; // TODO: 40 MHz clock diff --git a/lib/chip/blisp_chip_bl70x.c b/lib/chip/blisp_chip_bl70x.c index a380720..0b7360f 100644 --- a/lib/chip/blisp_chip_bl70x.c +++ b/lib/chip/blisp_chip_bl70x.c @@ -6,6 +6,8 @@ int64_t blisp_chip_bl70x_get_eflash_loader(uint8_t clk_type, uint8_t** firmware_buf_ptr) { + (void) clk_type; // ununsed + uint8_t* firmware_buf = malloc(sizeof(bl70x_eflash_loader_bin)); memcpy(firmware_buf, bl70x_eflash_loader_bin, sizeof(bl70x_eflash_loader_bin)); *(firmware_buf + 0xE0) = 1; // TODO: 32 MHz clock From 847be6c690573d6953e64f84f7d0ca169bcc5e7a Mon Sep 17 00:00:00 2001 From: "Berk D. Demir" Date: Wed, 9 Jul 2025 21:08:58 -0700 Subject: [PATCH 3/8] Fix compiler warnings: enum-int-mismatch blisp.h and common.h declares signatures of functions that return blisp_return_t as int32_t. --- include/blisp.h | 28 ++++++++++++++-------------- tools/blisp/src/common.h | 4 ++-- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/include/blisp.h b/include/blisp.h index 16cd2e2..8a89ffc 100644 --- a/include/blisp.h +++ b/include/blisp.h @@ -31,34 +31,34 @@ struct blisp_boot_info { // TODO: Refactor variable names, so all will follow same semantic, like // image_run, image_check etc. -int32_t blisp_device_init(struct blisp_device* device, struct blisp_chip* chip); -int32_t blisp_device_open(struct blisp_device* device, const char* port_name); -int32_t blisp_device_handshake(struct blisp_device* device, bool in_ef_loader); -int32_t blisp_device_get_boot_info(struct blisp_device* device, +blisp_return_t blisp_device_init(struct blisp_device* device, struct blisp_chip* chip); +blisp_return_t blisp_device_open(struct blisp_device* device, const char* port_name); +blisp_return_t blisp_device_handshake(struct blisp_device* device, bool in_ef_loader); +blisp_return_t blisp_device_get_boot_info(struct blisp_device* device, struct blisp_boot_info* boot_info); -int32_t blisp_device_load_boot_header(struct blisp_device* device, +blisp_return_t blisp_device_load_boot_header(struct blisp_device* device, uint8_t* boot_header); -int32_t blisp_device_load_segment_header( +blisp_return_t blisp_device_load_segment_header( struct blisp_device* device, struct blisp_segment_header* segment_header); -int32_t blisp_device_load_segment_data(struct blisp_device* device, +blisp_return_t blisp_device_load_segment_data(struct blisp_device* device, uint8_t* segment_data, uint32_t segment_data_length); -int32_t blisp_device_write_memory(struct blisp_device* device, +blisp_return_t blisp_device_write_memory(struct blisp_device* device, uint32_t address, uint32_t value, bool wait_for_res); -int32_t blisp_device_check_image(struct blisp_device* device); -int32_t blisp_device_run_image(struct blisp_device* device); -int32_t blisp_device_flash_erase(struct blisp_device* device, +blisp_return_t blisp_device_check_image(struct blisp_device* device); +blisp_return_t blisp_device_run_image(struct blisp_device* device); +blisp_return_t blisp_device_flash_erase(struct blisp_device* device, uint32_t start_address, uint32_t end_address); -int32_t blisp_device_flash_write(struct blisp_device* device, +blisp_return_t blisp_device_flash_write(struct blisp_device* device, uint32_t start_address, uint8_t* payload, uint32_t payload_size); -int32_t blisp_device_program_check(struct blisp_device* device); -int32_t blisp_device_reset(struct blisp_device* device); +blisp_return_t blisp_device_program_check(struct blisp_device* device); +blisp_return_t blisp_device_reset(struct blisp_device* device); void blisp_device_close(struct blisp_device* device); #endif \ No newline at end of file diff --git a/tools/blisp/src/common.h b/tools/blisp/src/common.h index e6519e7..257208e 100644 --- a/tools/blisp/src/common.h +++ b/tools/blisp/src/common.h @@ -6,8 +6,8 @@ #include #include -int32_t blisp_common_prepare_flash(struct blisp_device* device); +blisp_return_t blisp_common_prepare_flash(struct blisp_device* device); void blisp_common_progress_callback(uint32_t current_value, uint32_t max_value); -int32_t blisp_common_init_device(struct blisp_device* device, struct arg_str* port_name, struct arg_str* chip_type); +blisp_return_t blisp_common_init_device(struct blisp_device* device, struct arg_str* port_name, struct arg_str* chip_type); #endif // BLISP_COMMON_H From 77410adbee1c27a5d66178a18a1625757632618a Mon Sep 17 00:00:00 2001 From: "Berk D. Demir" Date: Wed, 9 Jul 2025 21:18:08 -0700 Subject: [PATCH 4/8] Fix compiler warnings: enum-conversion sp_* functions from libserialport return `enum sp_return`, not `blisp_return_t`. --- lib/blisp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/blisp.c b/lib/blisp.c index 58f0ff2..5516ef0 100644 --- a/lib/blisp.c +++ b/lib/blisp.c @@ -30,7 +30,7 @@ blisp_return_t blisp_device_init(struct blisp_device* device, blisp_return_t blisp_device_open(struct blisp_device* device, const char* port_name) { - blisp_return_t ret; + enum sp_return ret; struct sp_port* serial_port = NULL; if (port_name != NULL) { From 20270e520144ae3c503706fe02b81862b366dd96 Mon Sep 17 00:00:00 2001 From: "Berk D. Demir" Date: Wed, 9 Jul 2025 21:39:14 -0700 Subject: [PATCH 5/8] Fix compiler warnings: blisp_flash_firmware Improve blisp_flash_firmware error handling: - Address never checked return of `parse_firmware_file` - Ensure the function always returns the correct blisp_return_t value - Make error checking syntactic dance consistent --- tools/blisp/src/cmd/write.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/tools/blisp/src/cmd/write.c b/tools/blisp/src/cmd/write.c index a53390b..d28f48a 100644 --- a/tools/blisp/src/cmd/write.c +++ b/tools/blisp/src/cmd/write.c @@ -168,31 +168,35 @@ void fill_up_boot_header(struct bfl_boot_header* boot_header) { blisp_return_t blisp_flash_firmware() { struct blisp_device device; - blisp_return_t ret; + blisp_return_t ret = BLISP_OK; if (access(binary_to_write->filename[0], R_OK) != 0) { // File not accessible, error out. fprintf(stderr, "Input firmware not found: %s\n", binary_to_write->filename[0]); cmd_write_args_print_glossary(); /* Print help to assist user */ /* No need to free memory, will now exit with ret code 1 */ - return 1; + return BLISP_ERR_CANT_OPEN_FILE; } ret = blisp_common_init_device(&device, port_name, chip_type); - - if (ret != 0) { + if (ret != BLISP_OK) { return ret; } - if (blisp_common_prepare_flash(&device) != 0) { + ret = blisp_common_prepare_flash(&device); + if (ret != BLISP_OK) { // TODO: Error handling goto exit1; } parsed_firmware_file_t parsed_file; memset(&parsed_file, 0, sizeof(parsed_file)); - int parsed_result = - parse_firmware_file(binary_to_write->filename[0], &parsed_file); + if (parse_firmware_file(binary_to_write->filename[0], &parsed_file) < 0) { + // `parse_firmware_file` doesn't return `blisp_return_t` + // so we default to the generic error. + ret = BLISP_ERR_UNKNOWN; + goto exit1; + } // If we are injecting a bootloader section, make it, erase flash, and flash // it. Then when we do firmware later on; it will be located afterwards @@ -249,7 +253,7 @@ blisp_return_t blisp_flash_firmware() { &device, &data_transport, parsed_file.payload_address, parsed_file.payload_length, blisp_common_progress_callback); - if (ret < BLISP_OK) { + if (ret != BLISP_OK) { fprintf(stderr, "Failed to write app to flash.\n"); goto exit2; } @@ -275,6 +279,8 @@ blisp_return_t blisp_flash_firmware() { free(parsed_file.payload); exit1: blisp_device_close(&device); + + return ret; } blisp_return_t cmd_write_args_init() { From cb89bdb53735b571d0c183112fcbfcd2627ebbee Mon Sep 17 00:00:00 2001 From: "Berk D. Demir" Date: Wed, 9 Jul 2025 22:59:07 -0700 Subject: [PATCH 6/8] Fix compiler warnings: format string `z` is the length field for `size_t`. --- tools/blisp/src/cmd/write.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/blisp/src/cmd/write.c b/tools/blisp/src/cmd/write.c index d28f48a..fde093f 100644 --- a/tools/blisp/src/cmd/write.c +++ b/tools/blisp/src/cmd/write.c @@ -237,13 +237,13 @@ blisp_return_t blisp_flash_firmware() { if (ret != BLISP_OK) { fprintf(stderr, - "Failed to erase flash. Tried to erase from 0x%08lu to 0x%08lu\n", + "Failed to erase flash. Tried to erase from 0x%08zx to 0x%08zx\n", parsed_file.payload_address, parsed_file.payload_address + parsed_file.payload_length + 1); goto exit2; } - printf("Flashing the firmware %lu bytes @ 0x%08lu...\n", + printf("Flashing the firmware %zu bytes @ 0x%08zx...\n", parsed_file.payload_length, parsed_file.payload_address); struct blisp_easy_transport data_transport = blisp_easy_transport_new_from_memory(parsed_file.payload, From d60e0046a12b03c78abfffd7fbb225b345d209c7 Mon Sep 17 00:00:00 2001 From: "Berk D. Demir" Date: Wed, 9 Jul 2025 22:14:34 -0700 Subject: [PATCH 7/8] Enable all compiler warnings --- CMakeLists.txt | 9 +++++++++ tools/blisp/CMakeLists.txt | 10 ++++++++++ 2 files changed, 19 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 43462e8..3f6642a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -17,6 +17,15 @@ add_library(libblisp_obj OBJECT lib/chip/blisp_chip_bl70x.c) target_include_directories(libblisp_obj PRIVATE ${CMAKE_SOURCE_DIR}/include/) +if (NOT CMAKE_C_COMPILER_ID MATCHES "MSVC") + target_compile_options(libblisp_obj PRIVATE -Wall -Wextra -Wpedantic) +else() + # MSVC does not support 'extra' and 'pedantic' levels to warnings. + # `/Wall` seems to generate way too many non-actionable output marked as warnings. + # We settle for `/W4`. + # https://learn.microsoft.com/en-us/cpp/build/reference/compiler-option-warning-level?view=msvc-170 + target_compile_options(libblisp_obj PRIVATE -W4) +endif() set_property(TARGET libblisp_obj PROPERTY POSITION_INDEPENDENT_CODE 1) diff --git a/tools/blisp/CMakeLists.txt b/tools/blisp/CMakeLists.txt index 66cbb40..3af2488 100644 --- a/tools/blisp/CMakeLists.txt +++ b/tools/blisp/CMakeLists.txt @@ -23,6 +23,16 @@ target_link_libraries(blisp PRIVATE argtable3::argtable3 libblisp_static file_parsers) +if (NOT CMAKE_C_COMPILER_ID MATCHES "MSVC") + target_compile_options(libblisp_obj PRIVATE -Wall -Wextra -Wpedantic) +else() + # MSVC does not support 'extra' and 'pedantic' levels to warnings. + # `/Wall` seems to generate way too many non-actionable output marked as warnings. + # We settle for `/W4`. + # https://learn.microsoft.com/en-us/cpp/build/reference/compiler-option-warning-level?view=msvc-170 + target_compile_options(libblisp_obj PRIVATE -W4) +endif() + if (WIN32) target_link_libraries(blisp PRIVATE Setupapi.lib) elseif (APPLE) From ee10c51970209ebe09cde7633cd5a130dd69354b Mon Sep 17 00:00:00 2001 From: "Berk D. Demir" Date: Thu, 17 Jul 2025 20:35:55 -0700 Subject: [PATCH 8/8] Fix compiler warnings: Explicit cast where needed --- lib/blisp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/blisp.c b/lib/blisp.c index 5516ef0..a483392 100644 --- a/lib/blisp.c +++ b/lib/blisp.c @@ -192,8 +192,8 @@ blisp_return_t blisp_device_handshake(struct blisp_device* device, sleep_ms(50); // Wait a bit so BootROM can init } - uint32_t bytes_count = device->chip->handshake_byte_multiplier * - (float)device->current_baud_rate / 10.0f; + uint32_t bytes_count = (uint32_t)(device->chip->handshake_byte_multiplier * + (float)device->current_baud_rate / 10.0f); if (bytes_count > 600) bytes_count = 600; memset(handshake_buffer, 'U', bytes_count);