diff --git a/.clang-tidy b/.clang-tidy new file mode 100644 index 0000000..96fd3da --- /dev/null +++ b/.clang-tidy @@ -0,0 +1,16 @@ +--- +Checks: > + modernize-*, + performance-*, + readability-*, + bugprone-*, + -bugprone-easily-swappable-parameters, + -readability-braces-around-statements, + -readability-magic-numbers, + -readability-function-cognitive-complexity, + -readability-identifier-length +... + +WarningsAsErrors: '*' +HeaderFilterRegex: '' +FormatStyle: none diff --git a/CMakeLists.txt b/CMakeLists.txt index 0acfa32..c2c7eff 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -2,7 +2,7 @@ cmake_minimum_required(VERSION 3.16) project(nanomodbus C) set(CMAKE_C_STANDARD 99) -set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wextra -pedantic") +set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wextra -pedantic -Wswitch-enum -Wcast-qual") set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} -O0 -g") set(CMAKE_C_FLAGS_RELEASE "${CMAKE_C_FLAGS_RELEASE} -O3") diff --git a/examples/linux/platform.h b/examples/linux/platform.h index 4461cbf..62dd98e 100644 --- a/examples/linux/platform.h +++ b/examples/linux/platform.h @@ -23,7 +23,8 @@ int client_connection = -1; void* connect_tcp(const char* address, const char* port) { struct addrinfo ainfo = {0}; - struct addrinfo *results, *rp; + struct addrinfo *results; + struct addrinfo *rp; int fd; ainfo.ai_family = AF_INET; @@ -40,8 +41,8 @@ void* connect_tcp(const char* address, const char* port) { ret = connect(fd, rp->ai_addr, rp->ai_addrlen); if (ret == 0) break; - else - close(fd); + + close(fd); } freeaddrinfo(results); @@ -67,7 +68,8 @@ void close_server_on_exit(int sig) { int create_tcp_server(const char* address, const char* port) { struct addrinfo ainfo = {0}; - struct addrinfo *results, *rp; + struct addrinfo *results; + struct addrinfo *rp; int fd = -1; ainfo.ai_family = AF_INET; @@ -117,7 +119,7 @@ int create_tcp_server(const char* address, const char* port) { } -void* server_poll() { +void* server_poll(void) { fd_set read_fd_set; FD_ZERO(&read_fd_set); @@ -162,7 +164,7 @@ void disconnect(void* conn) { } -void close_server() { +void close_server(void) { close(server_fd); printf("Server closed\n"); } @@ -184,24 +186,25 @@ int32_t read_fd_linux(uint8_t* buf, uint16_t count, int32_t timeout_ms, void* ar if (timeout_ms >= 0) { tv_p = &tv; tv.tv_sec = timeout_ms / 1000; - tv.tv_usec = (timeout_ms % 1000) * 1000; + tv.tv_usec = (int64_t) (timeout_ms % 1000) * 1000; } int ret = select(fd + 1, &rfds, NULL, NULL, tv_p); if (ret == 0) { return total; } - else if (ret == 1) { + + if (ret == 1) { ssize_t r = read(fd, buf + total, 1); if (r == 0) { disconnect(arg); return -1; } - else if (r < 0) + + if (r < 0) return -1; - else { - total += r; - } + + total += r; } else return -1; @@ -225,24 +228,25 @@ int32_t write_fd_linux(const uint8_t* buf, uint16_t count, int32_t timeout_ms, v if (timeout_ms >= 0) { tv_p = &tv; tv.tv_sec = timeout_ms / 1000; - tv.tv_usec = (timeout_ms % 1000) * 1000; + tv.tv_usec = (int64_t) (timeout_ms % 1000) * 1000; } int ret = select(fd + 1, NULL, &wfds, NULL, tv_p); if (ret == 0) { return 0; } - else if (ret == 1) { + + if (ret == 1) { ssize_t w = write(fd, buf + total, count); if (w == 0) { disconnect(arg); return -1; } - else if (w <= 0) + + if (w <= 0) return -1; - else { - total += (int32_t) w; - } + + total += (int32_t) w; } else return -1; diff --git a/nanomodbus.c b/nanomodbus.c index 409d54b..7f7f8b1 100644 --- a/nanomodbus.c +++ b/nanomodbus.c @@ -50,19 +50,19 @@ #endif #endif -static uint8_t get_1(nmbs_t *m) { - uint8_t result = m->msg.buf[m->msg.buf_idx]; - m->msg.buf_idx++; +static uint8_t get_1(nmbs_t *nmbs) { + uint8_t result = nmbs->msg.buf[nmbs->msg.buf_idx]; + nmbs->msg.buf_idx++; return result; } -static void put_1(nmbs_t *m, uint8_t b) { - m->msg.buf[(m)->msg.buf_idx] = b; - m->msg.buf_idx++; +static void put_1(nmbs_t *nmbs, uint8_t data) { + nmbs->msg.buf[nmbs->msg.buf_idx] = data; + nmbs->msg.buf_idx++; } -static void discard_1(nmbs_t *m) { - m->msg.buf_idx++; +static void discard_1(nmbs_t *nmbs) { + nmbs->msg.buf_idx++; } #ifdef NMBS_BIG_ENDIAN @@ -80,16 +80,16 @@ static void put_2(nmbs_t *m, uint16_t w) { #else -static uint16_t get_2(nmbs_t *m) { - uint16_t result = ((uint16_t) (m->msg.buf[m->msg.buf_idx + 1])) | (((uint16_t) m->msg.buf[m->msg.buf_idx] << 8)); - m->msg.buf_idx += 2; +static uint16_t get_2(nmbs_t *nmbs) { + uint16_t result = ((uint16_t) (nmbs->msg.buf[nmbs->msg.buf_idx + 1])) | (((uint16_t) nmbs->msg.buf[nmbs->msg.buf_idx] << 8)); + nmbs->msg.buf_idx += 2; return result; } -static void put_2(nmbs_t *m, uint16_t w) { - m->msg.buf[m->msg.buf_idx] = ((uint8_t) ((((uint16_t) (w)) & 0xFF00) >> 8)); - m->msg.buf[m->msg.buf_idx + 1] = ((uint8_t) (((uint16_t) (w)) & 0x00FF)); - m->msg.buf_idx += 2; +static void put_2(nmbs_t *nmbs, uint16_t data) { + nmbs->msg.buf[nmbs->msg.buf_idx] = ((uint8_t) ((((uint16_t) (data)) & 0xFF00) >> 8)); + nmbs->msg.buf[nmbs->msg.buf_idx + 1] = ((uint8_t) (((uint16_t) (data)) & 0x00FF)); + nmbs->msg.buf_idx += 2; } #endif @@ -383,14 +383,12 @@ static nmbs_error recv_res_header(nmbs_t* nmbs) { if (exception < 1 || exception > 4) return NMBS_ERROR_INVALID_RESPONSE; - else { - DEBUG("exception %d\n", exception); - return exception; - } - } - else { - return NMBS_ERROR_INVALID_RESPONSE; + + DEBUG("exception %d\n", exception); + return exception; } + + return NMBS_ERROR_INVALID_RESPONSE; } DEBUG("NMBS res <- fc %d\t", nmbs->msg.fc); @@ -482,13 +480,13 @@ static nmbs_error handle_read_discrete(nmbs_t* nmbs, nmbs_error (*callback)(uint return send_exception_msg(nmbs, NMBS_EXCEPTION_ILLEGAL_DATA_ADDRESS); if (callback) { - nmbs_bitfield bf = {0}; - err = callback(address, quantity, bf, nmbs->platform.arg); + nmbs_bitfield bitfield = {0}; + err = callback(address, quantity, bitfield, nmbs->platform.arg); if (err != NMBS_ERROR_NONE) { if (nmbs_error_is_exception(err)) return send_exception_msg(nmbs, err); - else - return send_exception_msg(nmbs, NMBS_EXCEPTION_SERVER_DEVICE_FAILURE); + + return send_exception_msg(nmbs, NMBS_EXCEPTION_SERVER_DEVICE_FAILURE); } if (!nmbs->msg.broadcast) { @@ -501,8 +499,8 @@ static nmbs_error handle_read_discrete(nmbs_t* nmbs, nmbs_error (*callback)(uint DEBUG("coils "); for (int i = 0; i < discrete_bytes; i++) { - put_1(nmbs, bf[i]); - DEBUG("%d", bf[i]); + put_1(nmbs, bitfield[i]); + DEBUG("%d", bitfield[i]); } err = send_msg(nmbs); @@ -548,8 +546,8 @@ static nmbs_error handle_read_registers(nmbs_t* nmbs, nmbs_error (*callback)(uin if (err != NMBS_ERROR_NONE) { if (nmbs_error_is_exception(err)) return send_exception_msg(nmbs, err); - else - return send_exception_msg(nmbs, NMBS_EXCEPTION_SERVER_DEVICE_FAILURE); + + return send_exception_msg(nmbs, NMBS_EXCEPTION_SERVER_DEVICE_FAILURE); } if (!nmbs->msg.broadcast) { @@ -633,8 +631,8 @@ static nmbs_error handle_write_single_coil(nmbs_t* nmbs) { if (err != NMBS_ERROR_NONE) { if (nmbs_error_is_exception(err)) return send_exception_msg(nmbs, err); - else - return send_exception_msg(nmbs, NMBS_EXCEPTION_SERVER_DEVICE_FAILURE); + + return send_exception_msg(nmbs, NMBS_EXCEPTION_SERVER_DEVICE_FAILURE); } if (!nmbs->msg.broadcast) { @@ -680,8 +678,8 @@ static nmbs_error handle_write_single_register(nmbs_t* nmbs) { if (err != NMBS_ERROR_NONE) { if (nmbs_error_is_exception(err)) return send_exception_msg(nmbs, err); - else - return send_exception_msg(nmbs, NMBS_EXCEPTION_SERVER_DEVICE_FAILURE); + + return send_exception_msg(nmbs, NMBS_EXCEPTION_SERVER_DEVICE_FAILURE); } if (!nmbs->msg.broadcast) { @@ -750,8 +748,8 @@ static nmbs_error handle_write_multiple_coils(nmbs_t* nmbs) { if (err != NMBS_ERROR_NONE) { if (nmbs_error_is_exception(err)) return send_exception_msg(nmbs, err); - else - return send_exception_msg(nmbs, NMBS_EXCEPTION_SERVER_DEVICE_FAILURE); + + return send_exception_msg(nmbs, NMBS_EXCEPTION_SERVER_DEVICE_FAILURE); } if (!nmbs->msg.broadcast) { @@ -820,8 +818,8 @@ static nmbs_error handle_write_multiple_registers(nmbs_t* nmbs) { if (err != NMBS_ERROR_NONE) { if (nmbs_error_is_exception(err)) return send_exception_msg(nmbs, err); - else - return send_exception_msg(nmbs, NMBS_EXCEPTION_SERVER_DEVICE_FAILURE); + + return send_exception_msg(nmbs, NMBS_EXCEPTION_SERVER_DEVICE_FAILURE); } if (!nmbs->msg.broadcast) { @@ -918,8 +916,8 @@ nmbs_error nmbs_server_poll(nmbs_t* nmbs) { if (err != NMBS_ERROR_NONE) { if (!first_byte_received && err == NMBS_ERROR_TIMEOUT) return NMBS_ERROR_NONE; - else - return err; + + return err; } #ifdef NMBS_DEBUG diff --git a/tests/client_disabled.c b/tests/client_disabled.c index 10a2bd6..583fa99 100644 --- a/tests/client_disabled.c +++ b/tests/client_disabled.c @@ -3,8 +3,8 @@ #define UNUSED_PARAM(x) ((x) = (x)) -int32_t read_empty(uint8_t* b, uint16_t count, int32_t timeout, void* arg) { - UNUSED_PARAM(b); +int32_t read_empty(uint8_t* data, uint16_t count, int32_t timeout, void* arg) { + UNUSED_PARAM(data); UNUSED_PARAM(count); UNUSED_PARAM(timeout); UNUSED_PARAM(arg); @@ -12,8 +12,8 @@ int32_t read_empty(uint8_t* b, uint16_t count, int32_t timeout, void* arg) { } -int32_t write_empty(const uint8_t* b, uint16_t count, int32_t timeout, void* arg) { - UNUSED_PARAM(b); +int32_t write_empty(const uint8_t* data, uint16_t count, int32_t timeout, void* arg) { + UNUSED_PARAM(data); UNUSED_PARAM(count); UNUSED_PARAM(timeout); UNUSED_PARAM(arg); @@ -21,7 +21,10 @@ int32_t write_empty(const uint8_t* b, uint16_t count, int32_t timeout, void* arg } -int main() { +int main(int argc, char* argv[]) { + UNUSED_PARAM(argc); + UNUSED_PARAM(argv); + nmbs_t nmbs; nmbs_platform_conf platform_conf_empty = { diff --git a/tests/nanomodbus_tests.c b/tests/nanomodbus_tests.c index a18452c..77d8808 100644 --- a/tests/nanomodbus_tests.c +++ b/tests/nanomodbus_tests.c @@ -1,12 +1,11 @@ #include "nanomodbus_tests.h" -#include "nanomodbus.h" #include #include #include -int32_t read_empty(uint8_t* b, uint16_t count, int32_t timeout, void* arg) { - UNUSED_PARAM(b); +int32_t read_empty(uint8_t* data, uint16_t count, int32_t timeout, void* arg) { + UNUSED_PARAM(data); UNUSED_PARAM(count); UNUSED_PARAM(timeout); UNUSED_PARAM(arg); @@ -14,8 +13,8 @@ int32_t read_empty(uint8_t* b, uint16_t count, int32_t timeout, void* arg) { } -int32_t write_empty(const uint8_t* b, uint16_t count, int32_t timeout, void* arg) { - UNUSED_PARAM(b); +int32_t write_empty(const uint8_t* data, uint16_t count, int32_t timeout, void* arg) { + UNUSED_PARAM(data); UNUSED_PARAM(count); UNUSED_PARAM(timeout); UNUSED_PARAM(arg); @@ -54,21 +53,21 @@ void test_server_create(nmbs_transport transport) { expect(err == NMBS_ERROR_NONE); reset(nmbs); - nmbs_platform_conf p = platform_conf_empty; - p.transport = 3; - err = nmbs_server_create(&nmbs, 0, &p, &callbacks_empty); + nmbs_platform_conf conf = platform_conf_empty; + conf.transport = 3; + err = nmbs_server_create(&nmbs, 0, &conf, &callbacks_empty); expect(err == NMBS_ERROR_INVALID_ARGUMENT); reset(nmbs); - p = platform_conf_empty; - p.read = NULL; - err = nmbs_server_create(&nmbs, 0, &p, &callbacks_empty); + conf = platform_conf_empty; + conf.read = NULL; + err = nmbs_server_create(&nmbs, 0, &conf, &callbacks_empty); expect(err == NMBS_ERROR_INVALID_ARGUMENT); reset(nmbs); - p = platform_conf_empty; - p.write = NULL; - err = nmbs_server_create(&nmbs, 0, &p, &callbacks_empty); + conf = platform_conf_empty; + conf.write = NULL; + err = nmbs_server_create(&nmbs, 0, &conf, &callbacks_empty); expect(err == NMBS_ERROR_INVALID_ARGUMENT); } @@ -826,7 +825,10 @@ void for_transports(void (*test_fn)(nmbs_transport), const char* should_str) { } } -int main() { +int main(int argc, char* argv[]) { + UNUSED_PARAM(argc); + UNUSED_PARAM(argv); + for_transports(test_server_create, "create a modbus server"); for_transports(test_server_receive_base, "receive no messages without failing"); diff --git a/tests/server_disabled.c b/tests/server_disabled.c index 633a655..d634dbd 100644 --- a/tests/server_disabled.c +++ b/tests/server_disabled.c @@ -3,8 +3,8 @@ #define UNUSED_PARAM(x) ((x) = (x)) -int32_t read_empty(uint8_t* b, uint16_t count, int32_t timeout, void* arg) { - UNUSED_PARAM(b); +int32_t read_empty(uint8_t* data, uint16_t count, int32_t timeout, void* arg) { + UNUSED_PARAM(data); UNUSED_PARAM(count); UNUSED_PARAM(timeout); UNUSED_PARAM(arg); @@ -12,8 +12,8 @@ int32_t read_empty(uint8_t* b, uint16_t count, int32_t timeout, void* arg) { } -int32_t write_empty(const uint8_t* b, uint16_t count, int32_t timeout, void* arg) { - UNUSED_PARAM(b); +int32_t write_empty(const uint8_t* data, uint16_t count, int32_t timeout, void* arg) { + UNUSED_PARAM(data); UNUSED_PARAM(count); UNUSED_PARAM(timeout); UNUSED_PARAM(arg); @@ -21,7 +21,10 @@ int32_t write_empty(const uint8_t* b, uint16_t count, int32_t timeout, void* arg } -int main() { +int main(int argc, char* argv[]) { + UNUSED_PARAM(argc); + UNUSED_PARAM(argv); + nmbs_t nmbs; nmbs_platform_conf platform_conf_empty = {