From c166ffe0b4580c6d1b857bca51c61e565b0ebb57 Mon Sep 17 00:00:00 2001 From: Utkarsh Verma Date: Thu, 16 Nov 2023 09:08:29 +0530 Subject: [PATCH] refactor: Add clang-tidy checks and simplify codebase --- .clang-tidy | 30 ++++++++++++++++++++++++++ config.h | 5 ++++- include/block.h | 5 ++++- include/cli.h | 5 ++++- include/main.h | 5 ++++- include/signal-handler.h | 5 ++++- include/status.h | 5 ++++- include/timer.h | 5 ++++- include/util.h | 14 ++++++++---- include/watcher.h | 21 +++++++++--------- include/x11.h | 5 ++++- src/block.c | 2 +- src/main.c | 38 ++++++++++----------------------- src/status.c | 13 ++++++------ src/util.c | 2 +- src/watcher.c | 46 +++++++++++++++++++++++----------------- src/x11.c | 2 +- 17 files changed, 130 insertions(+), 78 deletions(-) create mode 100644 .clang-tidy diff --git a/.clang-tidy b/.clang-tidy new file mode 100644 index 0000000..17ce268 --- /dev/null +++ b/.clang-tidy @@ -0,0 +1,30 @@ +Checks: | + -*, + abseil-*, + bugprone-*, + clang-analyzer-*, + misc-*, + modernize-*, + performance-*, + portability-*, + readability-*, + llvm-*, + -bugprone-easily-swappable-parameters, + -readability-avoid-const-params-in-decls, + -readability-identifier-length + +CheckOptions: + - key: readability-inconsistent-declaration-parameter-name.Strict + value: true + - key: readability-identifier-naming.StructCase + value: lower_case + - key: readability-identifier-naming.FunctionCase + value: lower_case + - key: readability-identifier-naming.VariableCase + value: lower_case + - key: readability-identifier-naming.EnumConstantCase + value: UPPER_CASE + - key: readability-identifier-naming.MacroDefinitionCase + value: UPPER_CASE + - key: readability-function-cognitive-complexity.Threshold + value: 15 diff --git a/config.h b/config.h index 4e619d8..a02c0ca 100644 --- a/config.h +++ b/config.h @@ -1,4 +1,5 @@ -#pragma once +#ifndef CONFIG_H +#define CONFIG_H // String used to delimit block outputs in the status. #define DELIMITER " " @@ -27,3 +28,5 @@ X("sb-volume", 0, 8) \ X("sb-battery", 5, 9) \ X("sb-date", 1, 10) + +#endif // CONFIG_H diff --git a/include/block.h b/include/block.h index 1f2efe1..bb87bcb 100644 --- a/include/block.h +++ b/include/block.h @@ -1,4 +1,5 @@ -#pragma once +#ifndef BLOCK_H +#define BLOCK_H #include #include @@ -24,3 +25,5 @@ int block_deinit(block *const block); int block_execute(block *const block, const uint8_t button); int block_update(block *const block); bool block_must_run(const block *const block, const unsigned int time); + +#endif // BLOCK_H diff --git a/include/cli.h b/include/cli.h index 7cb218d..3d2d5a5 100644 --- a/include/cli.h +++ b/include/cli.h @@ -1,4 +1,5 @@ -#pragma once +#ifndef CLI_H +#define CLI_H #include @@ -8,3 +9,5 @@ typedef struct { int cli_init(cli_arguments* const args, const char* const argv[], const int argc); + +#endif // CLI_H diff --git a/include/main.h b/include/main.h index d5d5bfb..b37a6b1 100644 --- a/include/main.h +++ b/include/main.h @@ -1,4 +1,5 @@ -#pragma once +#ifndef MAIN_H +#define MAIN_H #include @@ -11,3 +12,5 @@ #define X(...) "." enum { BLOCK_COUNT = LEN(BLOCKS(X)) - 1 }; #undef X + +#endif // MAIN_H diff --git a/include/signal-handler.h b/include/signal-handler.h index cd7c877..dc70433 100644 --- a/include/signal-handler.h +++ b/include/signal-handler.h @@ -1,4 +1,5 @@ -#pragma once +#ifndef SIGNAL_HANDLER_H +#define SIGNAL_HANDLER_H #include @@ -28,3 +29,5 @@ signal_handler signal_handler_new( int signal_handler_init(signal_handler* const handler); int signal_handler_deinit(signal_handler* const handler); int signal_handler_process(signal_handler* const handler, timer* const timer); + +#endif // SIGNAL_HANDLER_H diff --git a/include/status.h b/include/status.h index 08aba35..48fb3d8 100644 --- a/include/status.h +++ b/include/status.h @@ -1,4 +1,5 @@ -#pragma once +#ifndef STATUS_H +#define STATUS_H #include @@ -26,3 +27,5 @@ status status_new(const block* const blocks, const unsigned short block_count); bool status_update(status* const status); int status_write(const status* const status, const bool is_debug_mode, x11_connection* const connection); + +#endif // STATUS_H diff --git a/include/timer.h b/include/timer.h index 26a2c73..eccf4a1 100644 --- a/include/timer.h +++ b/include/timer.h @@ -1,4 +1,5 @@ -#pragma once +#ifndef TIMER_H +#define TIMER_H #include @@ -14,3 +15,5 @@ typedef struct { timer timer_new(const block *const blocks, const unsigned short block_count); int timer_arm(timer *const timer); + +#endif // TIMER_H diff --git a/include/util.h b/include/util.h index 692d1ce..a3bdcce 100644 --- a/include/util.h +++ b/include/util.h @@ -1,13 +1,17 @@ -#pragma once +#ifndef UTIL_H +#define UTIL_H #include -#define MAX(a, b) ((a) > (b) ? (a) : (b)) -#define LEN(arr) (sizeof(arr) / sizeof(arr[0])) -#define BIT(n) (1 << (n)) +#define MAX(a, b) ((a) > (b) ? (a) : (b)) +#define LEN(arr) (sizeof(arr) / sizeof((arr)[0])) +#define BIT(n) (1 << (n)) + +// NOLINTBEGIN(bugprone-macro-parentheses) #define MEMBER_SIZE(type, member) sizeof(((type*)NULL)->member) #define MEMBER_LENGTH(type, member) \ (MEMBER_SIZE(type, member) / MEMBER_SIZE(type, member[0])) +// NOLINTEND(bugprone-macro-parentheses) #define UTF8_MAX_BYTE_COUNT 4 @@ -20,3 +24,5 @@ enum pipe_fd_index { unsigned int gcd(unsigned int a, unsigned int b); size_t truncate_utf8_string(char* const buffer, const size_t size, const size_t char_limit); + +#endif // UTIL_H diff --git a/include/watcher.h b/include/watcher.h index af1dbaf..63d3283 100644 --- a/include/watcher.h +++ b/include/watcher.h @@ -1,4 +1,5 @@ -#pragma once +#ifndef WATCHER_H +#define WATCHER_H #include #include @@ -6,22 +7,22 @@ #include "block.h" #include "main.h" -typedef enum { +enum watcher_fd_index { SIGNAL_FD = BLOCK_COUNT, WATCHER_FD_COUNT, -} watcher_fd_index; +}; typedef struct pollfd watcher_fd; typedef struct { watcher_fd fds[WATCHER_FD_COUNT]; - - const block *const blocks; - const unsigned short block_count; + unsigned short active_blocks[BLOCK_COUNT]; + unsigned short active_block_count; + bool got_signal; } watcher; -watcher watcher_new(const block *const blocks, - const unsigned short block_count); -int watcher_init(watcher *const watcher, const int signal_fd); +int watcher_init(watcher *const watcher, const block *const blocks, + const unsigned short block_count, const int signal_fd); int watcher_poll(watcher *const watcher, const int timeout_ms); -bool watcher_fd_is_readable(const watcher_fd *const watcher_fd); + +#endif // WATCHER_H diff --git a/include/x11.h b/include/x11.h index 6110876..6faaced 100644 --- a/include/x11.h +++ b/include/x11.h @@ -1,4 +1,5 @@ -#pragma once +#ifndef X11_H +#define X11_H #include @@ -8,3 +9,5 @@ x11_connection* x11_connection_open(void); void x11_connection_close(x11_connection* const connection); int x11_set_root_name(x11_connection* const connection, const char* const name); + +#endif // X11_H diff --git a/src/block.c b/src/block.c index 1a1bb03..c232d43 100644 --- a/src/block.c +++ b/src/block.c @@ -141,7 +141,7 @@ int block_update(block *const block) { return 1; } - (void)strcpy(block->output, buffer); + (void)strncpy(block->output, buffer, LEN(buffer)); return 0; } diff --git a/src/main.c b/src/main.c index 7526416..a858d88 100644 --- a/src/main.c +++ b/src/main.c @@ -86,46 +86,30 @@ static int event_loop(block *const blocks, const unsigned short block_count, return 1; } - watcher watcher = watcher_new(blocks, block_count); - if (watcher_init(&watcher, signal_handler->fd) != 0) { + watcher watcher; + if (watcher_init(&watcher, blocks, block_count, signal_handler->fd) != 0) { return 1; } status status = status_new(blocks, block_count); bool is_alive = true; while (is_alive) { - const int event_count = watcher_poll(&watcher, -1); - if (event_count == -1) { + if (watcher_poll(&watcher, -1) != 0) { return 1; } - int i = 0; - for (unsigned short j = 0; j < WATCHER_FD_COUNT; ++j) { - if (i == event_count) { - break; - } + if (watcher.got_signal) { + is_alive = signal_handler_process(signal_handler, &timer) == 0; + } - const watcher_fd *const watcher_fd = &watcher.fds[j]; - if (!watcher_fd_is_readable(watcher_fd)) { - continue; - } - - ++i; - - if (j == SIGNAL_FD) { - is_alive = signal_handler_process(signal_handler, &timer) == 0; - continue; - } - - block *const block = &blocks[j]; - (void)block_update(block); + for (unsigned short i = 0; i < watcher.active_block_count; ++i) { + (void)block_update(&blocks[watcher.active_blocks[i]]); } const bool has_status_changed = status_update(&status); - if (has_status_changed) { - if (status_write(&status, is_debug_mode, connection) != 0) { - return 1; - } + if (has_status_changed && + status_write(&status, is_debug_mode, connection) != 0) { + return 1; } } diff --git a/src/status.c b/src/status.c index c7561eb..8946aae 100644 --- a/src/status.c +++ b/src/status.c @@ -6,6 +6,7 @@ #include "block.h" #include "config.h" +#include "util.h" #include "x11.h" static bool has_status_changed(const status *const status) { @@ -26,7 +27,7 @@ status status_new(const block *const blocks, } bool status_update(status *const status) { - (void)strcpy(status->previous, status->current); + (void)strncpy(status->previous, status->current, LEN(status->current)); status->current[0] = '\0'; for (unsigned short i = 0; i < status->block_count; ++i) { @@ -34,27 +35,27 @@ bool status_update(status *const status) { if (strlen(block->output) > 0) { #if LEADING_DELIMITER - (void)strcat(status->current, DELIMITER); + (void)strncat(status->current, DELIMITER, LEN(DELIMITER)); #else if (status->current[0] != '\0') { - (void)strcat(status->current, DELIMITER); + (void)strncat(status->current, DELIMITER, LEN(DELIMITER)); } #endif #if CLICKABLE_BLOCKS if (block->signal > 0) { const char signal[] = {(char)block->signal, '\0'}; - (void)strcat(status->current, signal); + (void)strncat(status->current, signal, LEN(signal)); } #endif - (void)strcat(status->current, block->output); + (void)strncat(status->current, block->output, LEN(block->output)); } } #if TRAILING_DELIMITER if (status->current[0] != '\0') { - (void)strcat(status->current, DELIMITER); + (void)strcat(status->current, DELIMITER, LEN(DELIMITER)); } #endif diff --git a/src/util.c b/src/util.c index 822f84e..10485db 100644 --- a/src/util.c +++ b/src/util.c @@ -24,7 +24,7 @@ size_t truncate_utf8_string(char* const buffer, const size_t size, unsigned short skip = 1; - // Multibyte unicode character + // Multibyte unicode character. if ((ch & UTF8_MULTIBYTE_BIT) != 0) { // Skip continuation bytes. ch <<= 1; diff --git a/src/watcher.c b/src/watcher.c index ade4a42..972559a 100644 --- a/src/watcher.c +++ b/src/watcher.c @@ -8,20 +8,16 @@ #include "block.h" #include "util.h" -watcher watcher_new(const block* const blocks, - const unsigned short block_count) { - watcher watcher = { - .blocks = blocks, - .block_count = block_count, - }; - - return watcher; +static bool watcher_fd_is_readable(const watcher_fd* const watcher_fd) { + return (watcher_fd->revents & POLLIN) != 0; } -int watcher_init(watcher* const watcher, const int signal_fd) { +int watcher_init(watcher* const watcher, const block* const blocks, + const unsigned short block_count, const int signal_fd) { if (signal_fd == -1) { - fprintf(stderr, - "error: invalid signal file descriptor passed to watcher\n"); + (void)fprintf( + stderr, + "error: invalid signal file descriptor passed to watcher\n"); return 1; } @@ -29,10 +25,10 @@ int watcher_init(watcher* const watcher, const int signal_fd) { fd->fd = signal_fd; fd->events = POLLIN; - for (unsigned short i = 0; i < watcher->block_count; ++i) { - const int block_fd = watcher->blocks[i].pipe[READ_END]; + for (unsigned short i = 0; i < block_count; ++i) { + const int block_fd = blocks[i].pipe[READ_END]; if (block_fd == -1) { - fprintf( + (void)fprintf( stderr, "error: invalid block file descriptors passed to watcher\n"); return 1; @@ -47,17 +43,27 @@ int watcher_init(watcher* const watcher, const int signal_fd) { } int watcher_poll(watcher* watcher, const int timeout_ms) { - const int event_count = poll(watcher->fds, LEN(watcher->fds), timeout_ms); + int event_count = poll(watcher->fds, LEN(watcher->fds), timeout_ms); // Don't return non-zero status for signal interruptions. if (event_count == -1 && errno != EINTR) { (void)fprintf(stderr, "error: watcher could not poll blocks\n"); - return -1; + return 1; } - return event_count; -} + watcher->got_signal = watcher_fd_is_readable(&watcher->fds[SIGNAL_FD]); -bool watcher_fd_is_readable(const watcher_fd* const watcher_fd) { - return (watcher_fd->revents & POLLIN) != 0; + watcher->active_block_count = event_count - (int)watcher->got_signal; + unsigned short i = 0; + unsigned short j = 0; + while (i < event_count && j < LEN(watcher->active_blocks)) { + if (watcher_fd_is_readable(&watcher->fds[j])) { + watcher->active_blocks[i] = j; + ++i; + } + + ++j; + } + + return 0; } diff --git a/src/x11.c b/src/x11.c index 7335d5e..7a310e9 100644 --- a/src/x11.c +++ b/src/x11.c @@ -8,7 +8,7 @@ x11_connection *x11_connection_open(void) { xcb_connection_t *const connection = xcb_connect(NULL, NULL); if (xcb_connection_has_error(connection)) { - (void)fprintf(stderr, "error: could not connect to the X server\n"); + (void)fprintf(stderr, "error: could not connect to X server\n"); return NULL; }