From 80a8aeb1dbb055bc1daa30e3bf8d80d5b07b33c7 Mon Sep 17 00:00:00 2001 From: Rosen Penev Date: Sun, 31 Mar 2024 15:27:43 -0700 Subject: [PATCH 1/8] ui: change void to char It ends up being treated as a char anyway. Signed-off-by: Rosen Penev --- ui/helpers.c | 2 +- ui/helpers.h | 2 +- ui/ui.c | 8 ++++---- ui/ui.h | 6 +++--- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/ui/helpers.c b/ui/helpers.c index 247f826..3b4c51f 100644 --- a/ui/helpers.c +++ b/ui/helpers.c @@ -117,7 +117,7 @@ void for_each_cpu(GList *list, void (*fp)(cpu_ban_t *cpu, void *data), void *dat } } -void for_each_int(GList *list, void (*fp)(int *number, void *data), void *data) +void for_each_int(GList *list, void (*fp)(int *number, char *data), void *data) { GList *entry; entry = g_list_first(list); diff --git a/ui/helpers.h b/ui/helpers.h index 922914b..176a8d3 100644 --- a/ui/helpers.h +++ b/ui/helpers.h @@ -16,7 +16,7 @@ char * hex_to_bitmap(char hex_digit); gpointer copy_cpu_ban(gconstpointer src, gpointer data); gpointer copy_irq(gconstpointer src, gpointer data); void for_each_cpu(GList *list, void (*fp)(cpu_ban_t *cpu, void *data), void *data); -void for_each_int(GList *list, void (*fp)(int *number, void *data), void *data); +void for_each_int(GList *list, void (*fp)(int *number, char *data), void *data); void for_each_irq(GList *list, void (*fp)(irq_t *irq, void *data), void *data); void for_each_node(GList *list, void (*fp)(cpu_node_t *node, void *data), void *data); diff --git a/ui/ui.c b/ui/ui.c index a107fb9..2695ef0 100644 --- a/ui/ui.c +++ b/ui/ui.c @@ -130,7 +130,7 @@ int get_valid_sleep_input(int column_offest) return new_sleep; } -void get_banned_cpu(int *cpu, void *data __attribute__((unused))) +void get_banned_cpu(int *cpu, char *data __attribute__((unused))) { cpu_ban_t *new = malloc(sizeof(cpu_ban_t)); new->number = *cpu; @@ -188,9 +188,9 @@ void print_all_cpus(void) max_offset = 0; } -void add_banned_cpu(int *banned_cpu, void *data) +void add_banned_cpu(int *banned_cpu, char *data) { - snprintf((char *)data + strlen(data), 1024 - strlen(data), "%d, ", *banned_cpu); + snprintf(data + strlen(data), 1024 - strlen(data), "%d, ", *banned_cpu); } void display_banned_cpus(void) @@ -369,7 +369,7 @@ static inline void bsnl_emit(char *buf, int buflen) snprintf(buf + len, buflen - len, "%d-%d", rbot, rtop); } -void copy_assigned_obj(int *number, void *data) +void copy_assigned_obj(int *number, char *data) { if (rtop == -1) { rbot = rtop = *number; diff --git a/ui/ui.h b/ui/ui.h index f3485d4..5fbdd7c 100644 --- a/ui/ui.h +++ b/ui/ui.h @@ -23,10 +23,10 @@ void show_footer(void); char * check_control_in_sleep_input(int max_len, int column_offest, int line_offset); int get_valid_sleep_input(int column_offest); -void get_banned_cpu(int *cpu, void *data); +void get_banned_cpu(int *cpu, char *data); void print_cpu_line(cpu_ban_t *cpu, void *data); void print_all_cpus(void); -void add_banned_cpu(int *banned_cpu, void *data); +void add_banned_cpu(int *banned_cpu, char *data); void display_banned_cpus(void); int toggle_cpu(GList *cpu_list, int cpu_number); void get_new_cpu_ban_values(cpu_ban_t *cpu, void *data); @@ -34,7 +34,7 @@ void get_cpu(cpu_node_t *node, void *data); void handle_sleep_setting(void); void handle_cpu_banning(void); -void copy_assigned_obj(int *number, void *data); +void copy_assigned_obj(int *number, char *data); void print_assigned_objects_string(irq_t *irq, int *line_offset); void print_irq_line(irq_t *irq, void *data); void print_all_irqs(void); From 122ae9e27078585e5c618bee6741f723c03f1aef Mon Sep 17 00:00:00 2001 From: Rosen Penev Date: Mon, 1 Apr 2024 13:56:46 -0700 Subject: [PATCH 2/8] clang-tidy: don't assign in if Found with bugprone-assignment-in-if-condition Signed-off-by: Rosen Penev --- classify.c | 17 +++++++++++------ irqbalance.c | 3 ++- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/classify.c b/classify.c index 1601a1e..df9c13c 100644 --- a/classify.c +++ b/classify.c @@ -200,25 +200,30 @@ static unsigned int read_pci_data(const char *devpath, const char* file) /* Get pci information for IRQ classification */ static int get_pci_info(const char *devpath, struct pci_info *pci) { - unsigned int data = PCI_INVAL_DATA; + unsigned int data; - if ((data = read_pci_data(devpath, "vendor")) == PCI_INVAL_DATA) + data = read_pci_data(devpath, "vendor"); + if (data == PCI_INVAL_DATA) return -ENODEV; pci->vendor = (unsigned short)data; - if ((data = read_pci_data(devpath, "device")) == PCI_INVAL_DATA) + data = read_pci_data(devpath, "device"); + if (data == PCI_INVAL_DATA) return -ENODEV; pci->device = (unsigned short)data; - if ((data = read_pci_data(devpath, "subsystem_vendor")) == PCI_INVAL_DATA) + data = read_pci_data(devpath, "subsystem_vendor"); + if (data == PCI_INVAL_DATA) return -ENODEV; pci->sub_vendor = (unsigned short)data; - if ((data = read_pci_data(devpath, "subsystem_device")) == PCI_INVAL_DATA) + data = read_pci_data(devpath, "subsystem_device"); + if (data == PCI_INVAL_DATA) return -ENODEV; pci->sub_device = (unsigned short)data; - if ((data = read_pci_data(devpath, "class")) == PCI_INVAL_DATA) + data = read_pci_data(devpath, "class"); + if (data == PCI_INVAL_DATA) return -ENODEV; pci->class = data; diff --git a/irqbalance.c b/irqbalance.c index 373161f..a8c56c2 100644 --- a/irqbalance.c +++ b/irqbalance.c @@ -422,7 +422,8 @@ gboolean sock_handle(gint fd, GIOCondition condition, gpointer user_data __attri log(TO_ALL, LOG_WARNING, "Connection couldn't be accepted.\n"); goto out; } - if ((recv_size = recvmsg(sock, &msg, 0)) < 0) { + recv_size = recvmsg(sock, &msg, 0); + if (recv_size < 0) { log(TO_ALL, LOG_WARNING, "Error while receiving data.\n"); goto out_close; } From 4c1b0a09bf78365c88e2fdf9713540e59f0375fc Mon Sep 17 00:00:00 2001 From: Rosen Penev Date: Mon, 1 Apr 2024 13:58:40 -0700 Subject: [PATCH 3/8] clang-tidy: properly use strncmp Found with bugprone-suspicious-string-compare Signed-off-by: Rosen Penev --- ui/irqbalance-ui.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/ui/irqbalance-ui.c b/ui/irqbalance-ui.c index f5122ee..581c110 100644 --- a/ui/irqbalance-ui.c +++ b/ui/irqbalance-ui.c @@ -157,7 +157,7 @@ void parse_setup(char *setup_data) setup.banned_irqs = NULL; setup.banned_cpus = NULL; token = strtok_r(copy, " ", &ptr); - if(strncmp(token, "SLEEP", strlen("SLEEP"))) goto out; + if(strncmp(token, "SLEEP", strlen("SLEEP")) != 0) goto out; setup.sleep = strtol(strtok_r(NULL, " ", &ptr), NULL, 10); token = strtok_r(NULL, " ", &ptr); /* Parse banned IRQ data */ @@ -165,13 +165,13 @@ void parse_setup(char *setup_data) new_irq = malloc(sizeof(irq_t)); new_irq->vector = strtol(strtok_r(NULL, " ", &ptr), NULL, 10); token = strtok_r(NULL, " ", &ptr); - if(strncmp(token, "LOAD", strlen("LOAD"))) goto out; + if(strncmp(token, "LOAD", strlen("LOAD")) != 0) goto out; new_irq->load = strtol(strtok_r(NULL, " ", &ptr), NULL, 10); token = strtok_r(NULL, " ", &ptr); - if(strncmp(token, "DIFF", strlen("DIFF"))) goto out; + if(strncmp(token, "DIFF", strlen("DIFF")) != 0) goto out; new_irq->diff = strtol(strtok_r(NULL, " ", &ptr), NULL, 10); token = strtok_r(ptr, " ", &ptr); - if(strncmp(token, "CLASS", strlen("CLASS"))) goto out; + if(strncmp(token, "CLASS", strlen("CLASS")) != 0) goto out; new_irq->class = strtol(strtok_r(NULL, " ", &ptr), NULL, 10); new_irq->is_banned = 1; new_irq->assigned_to = NULL; @@ -180,7 +180,7 @@ void parse_setup(char *setup_data) new_irq = NULL; } - if(strncmp(token, "BANNED", strlen("BANNED"))) goto out; + if(strncmp(token, "BANNED", strlen("BANNED")) != 0) goto out; token = strtok_r(NULL, " ", &ptr); for(i = strlen(token) - 1; i >= 0; i--) { if (token[i] == ',') @@ -287,7 +287,7 @@ void parse_into_tree(char *data) token = strtok_r(copy, " ", &ptr); while(token != NULL) { /* Parse node data */ - if(strncmp(token, "TYPE", strlen("TYPE"))) { + if(strncmp(token, "TYPE", strlen("TYPE")) != 0) { free(copy); goto out; } @@ -303,13 +303,13 @@ void parse_into_tree(char *data) parent = parent->parent; } token = strtok_r(NULL, " ", &ptr); - if(strncmp(token, "NUMBER", strlen("NUMBER"))) goto out; + if(strncmp(token, "NUMBER", strlen("NUMBER")) != 0) goto out; new->number = strtol(strtok_r(NULL, " ", &ptr), NULL, 10); token = strtok_r(NULL, " ", &ptr); - if(strncmp(token, "LOAD", strlen("LOAD"))) goto out; + if(strncmp(token, "LOAD", strlen("LOAD")) != 0) goto out; new->load = strtol(strtok_r(NULL, " ", &ptr), NULL, 10); token = strtok_r(NULL, " ", &ptr); - if(strncmp(token, "SAVE_MODE", strlen("SAVE_MODE"))) goto out; + if(strncmp(token, "SAVE_MODE", strlen("SAVE_MODE")) != 0) goto out; new->is_powersave = strtol(strtok_r(NULL, " ", &ptr), NULL, 10); token = strtok_r(NULL, " ", &ptr); @@ -318,13 +318,13 @@ void parse_into_tree(char *data) new_irq = malloc(sizeof(irq_t)); new_irq->vector = strtol(strtok_r(NULL, " ", &ptr), NULL, 10); token = strtok_r(NULL, " ", &ptr); - if(strncmp(token, "LOAD", strlen("LOAD"))) goto out; + if(strncmp(token, "LOAD", strlen("LOAD")) != 0) goto out; new_irq->load = strtol(strtok_r(NULL, " ", &ptr), NULL, 10); token = strtok_r(NULL, " ", &ptr); - if(strncmp(token, "DIFF", strlen("DIFF"))) goto out; + if(strncmp(token, "DIFF", strlen("DIFF")) != 0) goto out; new_irq->diff = strtol(strtok_r(NULL, " ", &ptr), NULL, 10); token = strtok_r(NULL, " ", &ptr); - if(strncmp(token, "CLASS", strlen("CLASS"))) goto out; + if(strncmp(token, "CLASS", strlen("CLASS")) != 0) goto out; new_irq->class = strtol(strtok_r(NULL, " ", &ptr), NULL, 10); new_irq->is_banned = 0; new->irqs = g_list_append(new->irqs, new_irq); @@ -332,7 +332,7 @@ void parse_into_tree(char *data) new_irq = NULL; } - if((token == NULL) || (strncmp(token, "IRQ", strlen("IRQ")))) { + if((token == NULL) || (strncmp(token, "IRQ", strlen("IRQ")) != 0)) { new->parent = parent; if(parent == NULL) { tree = g_list_append(tree, new); From 8f575ec8bd70bd6895fc876d203ae28b7ab8c495 Mon Sep 17 00:00:00 2001 From: Rosen Penev Date: Mon, 1 Apr 2024 14:04:46 -0700 Subject: [PATCH 4/8] replace malloc with g_malloc0 Aborts on failure. There's no null check. Signed-off-by: Rosen Penev --- ui/irqbalance-ui.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/ui/irqbalance-ui.c b/ui/irqbalance-ui.c index 581c110..06f4602 100644 --- a/ui/irqbalance-ui.c +++ b/ui/irqbalance-ui.c @@ -291,11 +291,7 @@ void parse_into_tree(char *data) free(copy); goto out; } - new = malloc(sizeof(cpu_node_t)); - new->irqs = NULL; - new->children = NULL; - new->cpu_list = NULL; - new->cpu_mask = NULL; + new = g_malloc0(sizeof(cpu_node_t)); new->type = strtol(strtok_r(NULL, " ", &ptr), NULL, 10); if(new->type == OBJ_TYPE_NODE) { parent = NULL; From 2c3cbb5713e86199a10c8624eaf188609635d443 Mon Sep 17 00:00:00 2001 From: Rosen Penev Date: Mon, 1 Apr 2024 14:11:56 -0700 Subject: [PATCH 5/8] clang-tidy: don't use else after return Found with readability-else-after-return Signed-off-by: Rosen Penev --- bitmap.h | 15 +++++---------- classify.c | 6 ++---- irqbalance.c | 6 +++--- numa.c | 2 +- ui/ui.c | 13 ++++++------- 5 files changed, 17 insertions(+), 25 deletions(-) diff --git a/bitmap.h b/bitmap.h index 7afce59..9eca1af 100644 --- a/bitmap.h +++ b/bitmap.h @@ -282,8 +282,7 @@ static inline int bitmap_equal(const unsigned long *src1, { if (nbits <= BITS_PER_LONG) return ! ((*src1 ^ *src2) & BITMAP_LAST_WORD_MASK(nbits)); - else - return __bitmap_equal(src1, src2, nbits); + return __bitmap_equal(src1, src2, nbits); } static inline int bitmap_intersects(const unsigned long *src1, @@ -291,8 +290,7 @@ static inline int bitmap_intersects(const unsigned long *src1, { if (nbits <= BITS_PER_LONG) return ((*src1 & *src2) & BITMAP_LAST_WORD_MASK(nbits)) != 0; - else - return __bitmap_intersects(src1, src2, nbits); + return __bitmap_intersects(src1, src2, nbits); } static inline int bitmap_subset(const unsigned long *src1, @@ -300,24 +298,21 @@ static inline int bitmap_subset(const unsigned long *src1, { if (nbits <= BITS_PER_LONG) return ! ((*src1 & ~(*src2)) & BITMAP_LAST_WORD_MASK(nbits)); - else - return __bitmap_subset(src1, src2, nbits); + return __bitmap_subset(src1, src2, nbits); } static inline int bitmap_empty(const unsigned long *src, int nbits) { if (nbits <= BITS_PER_LONG) return ! (*src & BITMAP_LAST_WORD_MASK(nbits)); - else - return __bitmap_empty(src, nbits); + return __bitmap_empty(src, nbits); } static inline int bitmap_full(const unsigned long *src, int nbits) { if (nbits <= BITS_PER_LONG) return ! (~(*src) & BITMAP_LAST_WORD_MASK(nbits)); - else - return __bitmap_full(src, nbits); + return __bitmap_full(src, nbits); } static inline int bitmap_weight(const unsigned long *src, int nbits) diff --git a/classify.c b/classify.c index df9c13c..cc0200d 100644 --- a/classify.c +++ b/classify.c @@ -295,8 +295,7 @@ gint substr_find(gconstpointer a, gconstpointer b) { if (strstr(b, a)) return 0; - else - return 1; + return 1; } static void add_banned_module(char *modname, GList **modlist) @@ -558,8 +557,7 @@ static int check_for_module_ban(char *name) if (entry) return 1; - else - return 0; + return 0; } static int check_for_irq_ban(struct irq_info *irq, char *mod) diff --git a/irqbalance.c b/irqbalance.c index a8c56c2..ab4e448 100644 --- a/irqbalance.c +++ b/irqbalance.c @@ -330,10 +330,10 @@ gboolean scan(gpointer data __attribute__((unused))) if (keep_going) { return TRUE; - } else { - g_main_loop_quit(main_loop); - return FALSE; } + + g_main_loop_quit(main_loop); + return FALSE; } void get_irq_data(struct irq_info *irq, void *data) diff --git a/numa.c b/numa.c index 13d7ebd..5143ea0 100644 --- a/numa.c +++ b/numa.c @@ -116,7 +116,7 @@ void connect_cpu_mem_topo(struct topo_obj *p, void *data __attribute__((unused)) if (len == 0) { return; - } else if (len > 1) { + } if (len > 1) { for_each_object(p->children, connect_cpu_mem_topo, NULL); return; } diff --git a/ui/ui.c b/ui/ui.c index 2695ef0..c580f85 100644 --- a/ui/ui.c +++ b/ui/ui.c @@ -113,14 +113,13 @@ int get_valid_sleep_input(int column_offest) new_sleep = strtol(input, &error, 10); if((*error == '\0') && (new_sleep >= 1)) { break; - } else { - new_sleep = setup.sleep; - attrset(COLOR_PAIR(4) | A_BOLD); - mvprintw(LINES - 2, 1, - "Invalid input: %s ", - input); - refresh(); } + new_sleep = setup.sleep; + attrset(COLOR_PAIR(4) | A_BOLD); + mvprintw(LINES - 2, 1, + "Invalid input: %s ", + input); + refresh(); free(input); } From 9b1ced291f14debdae735723978ed5b44da9deee Mon Sep 17 00:00:00 2001 From: Rosen Penev Date: Mon, 1 Apr 2024 14:14:55 -0700 Subject: [PATCH 6/8] clang-tidy: remove return in void functions Found with readability-redundant-control-flow Signed-off-by: Rosen Penev --- classify.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/classify.c b/classify.c index cc0200d..08340db 100644 --- a/classify.c +++ b/classify.c @@ -102,8 +102,6 @@ static void apply_pci_quirks(const struct pci_info *pci, int *irq_class) break; } } - - return; } /* Determin IRQ class based on PCI class code */ @@ -283,7 +281,6 @@ static void add_banned_irq(int irq, GList **list) *list = g_list_append(*list, new); log(TO_CONSOLE, LOG_INFO, "IRQ %d was BANNED.\n", irq); - return; } void add_cl_banned_irq(int irq) From 02f7c174f1dfb0699cec4078d90a93901556a0c1 Mon Sep 17 00:00:00 2001 From: Rosen Penev Date: Mon, 1 Apr 2024 14:16:26 -0700 Subject: [PATCH 7/8] clang-tidy: remove redundant declarations Found with readability-redundant-declaration Signed-off-by: Rosen Penev --- ui/ui.h | 1 - 1 file changed, 1 deletion(-) diff --git a/ui/ui.h b/ui/ui.h index 5fbdd7c..e41ca24 100644 --- a/ui/ui.h +++ b/ui/ui.h @@ -11,7 +11,6 @@ #include "irqbalance-ui.h" #include "helpers.h" -extern GList *tree; extern setup_t setup; extern int offset; From e78ea2676fdb2754b58ddec3c88b185f1b0b9949 Mon Sep 17 00:00:00 2001 From: Rosen Penev Date: Mon, 1 Apr 2024 14:18:35 -0700 Subject: [PATCH 8/8] clang-tidy: remove duplicate include Found with readability-duplicate-include Signed-off-by: Rosen Penev --- irqbalance.c | 1 - 1 file changed, 1 deletion(-) diff --git a/irqbalance.c b/irqbalance.c index ab4e448..870d7c0 100644 --- a/irqbalance.c +++ b/irqbalance.c @@ -26,7 +26,6 @@ #include #include #include -#include #include #include #include