Skip to content

Refactor to address compiler warnings #38

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jul 18, 2025
Merged

Refactor to address compiler warnings #38

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
52e736e
Fix compiler warnings: Unused functions
bdd Jul 10, 2025
77df78c
Fix compiler warnings: Unused function params
bdd Jul 10, 2025
847be6c
Fix compiler warnings: enum-int-mismatch
bdd Jul 10, 2025
77410ad
Fix compiler warnings: enum-conversion
bdd Jul 10, 2025
20270e5
Fix compiler warnings: blisp_flash_firmware
bdd Jul 10, 2025
cb89bdb
Fix compiler warnings: format string
bdd Jul 10, 2025
d60e004
Enable all compiler warnings
bdd Jul 10, 2025
ee10c51
Fix compiler warnings: Explicit cast where needed
bdd Jul 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,21 @@ 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/)
if (NOT CMAKE_C_COMPILER_ID MATCHES "MSVC")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing this isn't working with the build failures?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is working. there's a missing one. i'm working on sending something that'll address the build issue but MSVC gives a useful warning neither gcc nor clang gave.

blisp_send_command receives a uint16_t type payload_size, but in various places what is passed to it is uint32_t. I was reading the call sites and went down the rabbit hole of Bouffalo Lab docs.

Let me update this real quick, retaining the warnings. They can be worked on later.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh makes sense. No pressure/rush was trying to help 😓

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything is green right now, but I think MSVC's warning highlights a possibly unintended but currently benign type down conversion issue.

From: https://github.com/pine64/blisp/actions/runs/16361436024/job/46229915025?pr=38#step:4:35

[...]\lib\blisp.c(196,59): warning C4244: 'initializing': conversion from 'float' to 'uint32_t', possible loss of data
[...]\lib\blisp.c(286,56): warning C4244: 'function': conversion from 'uint32_t' to 'uint16_t', possible loss of data
[...]\lib\blisp.c(385,61): warning C4244: 'function': conversion from 'uint32_t' to 'uint16_t', possible loss of data

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, for the first one, float to uint32_t, what is missing is an explicit conversion. I will add it as a separate commit. The other two, are about the uint16_t payload_size parameter of blisp_send_command.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I'm done :)
If you think it's OK to merge with the two warnings only MSVC raises, I'm OK too.

I can work on these warnings in the near future. I promise it won't take me 2 years to come back to it (check the age of this PR 😅).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also to blame for the age, no one was really monitoring PR's well and I forgot about it 😓

I think its good to go indeeed

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)

Expand Down
28 changes: 14 additions & 14 deletions include/blisp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
46 changes: 7 additions & 39 deletions include/blisp_util.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,35 +2,18 @@
#ifndef _BLISP_UTIL_H
#define _BLISP_UTIL_H

#include <stdarg.h>
#include <stdio.h>
#include <stdint.h>
#ifdef WIN32
#include <windows.h>
# include <windows.h>
#else
#include <time.h>
# include <time.h>
#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
Expand Down Expand Up @@ -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
#endif
10 changes: 6 additions & 4 deletions lib/blisp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
static void drain(struct sp_port* port) {
#if defined(__APPLE__) || defined(__FreeBSD__)
sp_drain(port);
#else
(void)port; // unused
#endif
}

Expand All @@ -28,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) {
Expand Down Expand Up @@ -190,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);
Expand Down Expand Up @@ -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);
}
}
48 changes: 48 additions & 0 deletions lib/blisp_util.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// SPDX-License-Identifier: MIT
#include <stdarg.h>
#include <stdint.h>
#include <stdio.h>

#ifdef WIN32
# include <windows.h>
#else
# include <time.h>
#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;
}
2 changes: 2 additions & 0 deletions lib/chip/blisp_chip_bl60x.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions lib/chip/blisp_chip_bl70x.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions tools/blisp/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
26 changes: 16 additions & 10 deletions tools/blisp/src/cmd/write.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -233,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,
Expand All @@ -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;
}
Expand All @@ -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() {
Expand Down
4 changes: 2 additions & 2 deletions tools/blisp/src/common.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
#include <blisp.h>
#include <argtable3.h>

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
Loading