From 5010a9766aff8a0c8e8644acef40d025ec2e0a48 Mon Sep 17 00:00:00 2001 From: Rosen Penev Date: Sun, 30 Jun 2024 16:50:50 -0700 Subject: [PATCH 1/9] direct initialize msghdr members Standard C99. { 0 } is somewhat interesting as some compilers warn about uninitialized members, which is bogus. Signed-off-by: Rosen Penev --- irqbalance.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/irqbalance.c b/irqbalance.c index 1490336..64b41f7 100644 --- a/irqbalance.c +++ b/irqbalance.c @@ -406,11 +406,12 @@ gboolean sock_handle(gint fd, GIOCondition condition, gpointer user_data __attri int valid_user = 0; struct iovec iov = { buff, 500 }; - struct msghdr msg = { 0 }; - msg.msg_iov = &iov; - msg.msg_iovlen = 1; - msg.msg_control = malloc(CMSG_SPACE(sizeof(struct ucred))); - msg.msg_controllen = CMSG_SPACE(sizeof(struct ucred)); + struct msghdr msg = { + .msg_iov = &iov, + .msg_iovlen = 1, + .msg_control = malloc(CMSG_SPACE(sizeof(struct ucred))), + .msg_controllen = CMSG_SPACE(sizeof(struct ucred)), + }; struct cmsghdr *cmsg; From d36ae562c668c25d737b90c4ca1a84d8e9079712 Mon Sep 17 00:00:00 2001 From: Rosen Penev Date: Thu, 4 Jul 2024 14:04:14 -0700 Subject: [PATCH 2/9] direct initialize iovec Signed-off-by: Rosen Penev --- ui/irqbalance-ui.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/ui/irqbalance-ui.c b/ui/irqbalance-ui.c index 06f4602..2e6b7d3 100644 --- a/ui/irqbalance-ui.c +++ b/ui/irqbalance-ui.c @@ -118,9 +118,10 @@ char * get_data(char *string) } struct msghdr *msg = create_credentials_msg(); - struct iovec iov; - iov.iov_base = (void *) string; - iov.iov_len = strlen(string); + struct iovec iov = { + .iov_base = (void *) string, + .iov_len = strlen(string), + }; msg->msg_iov = &iov; sendmsg(socket_fd, msg, 0); From 16564e3ced6517ddd15bbcfb84df704f7f614c3b Mon Sep 17 00:00:00 2001 From: Rosen Penev Date: Sun, 30 Jun 2024 17:26:11 -0700 Subject: [PATCH 3/9] clang-tidy: add missing free Found with clang-analyzer-unix.Malloc Signed-off-by: Rosen Penev --- ui/ui.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ui/ui.c b/ui/ui.c index c580f85..5401d02 100644 --- a/ui/ui.c +++ b/ui/ui.c @@ -112,6 +112,7 @@ int get_valid_sleep_input(int column_offest) char *error; new_sleep = strtol(input, &error, 10); if((*error == '\0') && (new_sleep >= 1)) { + free(input); break; } new_sleep = setup.sleep; From 2ed4dd0233a6e070c7ad128594892510d28f2ea9 Mon Sep 17 00:00:00 2001 From: Rosen Penev Date: Sun, 30 Jun 2024 17:14:55 -0700 Subject: [PATCH 4/9] clang-tidy: don't assign in if Found with bugprone-assignment-in-if-condition Signed-off-by: Rosen Penev --- irqbalance.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/irqbalance.c b/irqbalance.c index 64b41f7..dfd5d2d 100644 --- a/irqbalance.c +++ b/irqbalance.c @@ -654,13 +654,16 @@ int main(int argc, char** argv) if (daemon(0,0)) exit(EXIT_FAILURE); /* Write pidfile which can be used to avoid starting multiple instances */ - if (pidfile && (pidfd = open(pidfile, - O_WRONLY | O_CREAT | O_EXCL | O_TRUNC, - S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH)) >= 0) { - char str[16]; - snprintf(str, sizeof(str), "%u\n", getpid()); - write(pidfd, str, strlen(str)); - close(pidfd); + if (pidfile) { + pidfd = open(pidfile, + O_WRONLY | O_CREAT | O_EXCL | O_TRUNC, + S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); + if (pidfd >= 0) { + char str[16]; + snprintf(str, sizeof(str), "%u\n", getpid()); + write(pidfd, str, strlen(str)); + close(pidfd); + } } } From 13916f7e14a2e6270e40519aa2f59319136dad08 Mon Sep 17 00:00:00 2001 From: Rosen Penev Date: Sun, 30 Jun 2024 17:16:30 -0700 Subject: [PATCH 5/9] clang-tidy: remove pointless casts Found with readability-redundant-casting Signed-off-by: Rosen Penev --- cputree.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cputree.c b/cputree.c index 6c7b3b4..4820bdf 100644 --- a/cputree.c +++ b/cputree.c @@ -271,7 +271,7 @@ static struct topo_obj* add_cpu_to_cache_domain(struct topo_obj *cpu, entry = g_list_find(cache->children, cpu); if (!entry) { cache->children = g_list_append(cache->children, cpu); - cpu->parent = (struct topo_obj *)cache; + cpu->parent = cache; } if (!numa_avail || (nodeid > NUMA_NO_NODE)) @@ -441,7 +441,7 @@ static void dump_numa_node_num(struct topo_obj *p, void *data __attribute__((unu static void dump_balance_obj(struct topo_obj *d, void *data __attribute__((unused))) { - struct topo_obj *c = (struct topo_obj *)d; + struct topo_obj *c = d; log(TO_CONSOLE, LOG_INFO, "%s%s%s%sCPU number %i numa_node is ", log_indent, log_indent, log_indent, log_indent, c->number); for_each_object(cpu_numa_node(c), dump_numa_node_num, NULL); From a7cfbeb0a6609dd56507c129ea2163816c4a4a5f Mon Sep 17 00:00:00 2001 From: Rosen Penev Date: Sun, 30 Jun 2024 16:49:20 -0700 Subject: [PATCH 6/9] use g_malloc and friends These should not fail. There are no null checks. Signed-off-by: Rosen Penev --- irqbalance.c | 4 ++-- ui/helpers.c | 6 ++--- ui/irqbalance-ui.c | 57 ++++++++++++++++++++-------------------------- ui/ui.c | 17 +++++++------- 4 files changed, 38 insertions(+), 46 deletions(-) diff --git a/irqbalance.c b/irqbalance.c index dfd5d2d..3875d5d 100644 --- a/irqbalance.c +++ b/irqbalance.c @@ -409,7 +409,7 @@ gboolean sock_handle(gint fd, GIOCondition condition, gpointer user_data __attri struct msghdr msg = { .msg_iov = &iov, .msg_iovlen = 1, - .msg_control = malloc(CMSG_SPACE(sizeof(struct ucred))), + .msg_control = g_malloc(CMSG_SPACE(sizeof(struct ucred))), .msg_controllen = CMSG_SPACE(sizeof(struct ucred)), }; @@ -540,7 +540,7 @@ gboolean sock_handle(gint fd, GIOCondition condition, gpointer user_data __attri } out: - free(msg.msg_control); + g_free(msg.msg_control); return TRUE; } diff --git a/ui/helpers.c b/ui/helpers.c index 3b4c51f..529b1b3 100644 --- a/ui/helpers.c +++ b/ui/helpers.c @@ -73,7 +73,7 @@ char * hex_to_bitmap(char hex_digit) { return "0000\0"; } - char *bitmap = malloc(5 * sizeof(char)); + char *bitmap = g_malloc_n(5, sizeof(char)); bitmap[4] = '\0'; int i; for(i = 3; i >= 0; i--) { @@ -86,7 +86,7 @@ char * hex_to_bitmap(char hex_digit) { gpointer copy_cpu_ban(gconstpointer src, gpointer data __attribute__((unused))) { cpu_ban_t *old = (cpu_ban_t *)src; - cpu_ban_t *new = malloc(sizeof(cpu_ban_t)); + cpu_ban_t *new = g_malloc(sizeof(cpu_ban_t)); new->number = old->number; new->is_banned = old->is_banned; new->is_changed = 0; @@ -96,7 +96,7 @@ gpointer copy_cpu_ban(gconstpointer src, gpointer data __attribute__((unused))) gpointer copy_irq(gconstpointer src, gpointer data __attribute__((unused))) { irq_t *old = (irq_t *)src; - irq_t *new = malloc(sizeof(irq_t)); + irq_t *new = g_malloc(sizeof(irq_t)); new->vector = old->vector; new->load = old->load; new->diff = old->diff; diff --git a/ui/irqbalance-ui.c b/ui/irqbalance-ui.c index 2e6b7d3..9bb76fd 100644 --- a/ui/irqbalance-ui.c +++ b/ui/irqbalance-ui.c @@ -30,15 +30,14 @@ static int default_bufsz = 8192; struct msghdr * create_credentials_msg(void) { - struct ucred *credentials = malloc(sizeof(struct ucred)); + struct ucred *credentials = g_malloc(sizeof(struct ucred)); credentials->pid = getpid(); credentials->uid = geteuid(); credentials->gid = getegid(); - struct msghdr *msg = malloc(sizeof(struct msghdr)); - memset(msg, 0, sizeof(struct msghdr)); + struct msghdr *msg = g_malloc0(sizeof(struct msghdr)); msg->msg_iovlen = 1; - msg->msg_control = malloc(CMSG_SPACE(sizeof(struct ucred))); + msg->msg_control = g_malloc(CMSG_SPACE(sizeof(struct ucred))); msg->msg_controllen = CMSG_SPACE(sizeof(struct ucred)); struct cmsghdr *cmsg = CMSG_FIRSTHDR(msg); @@ -47,7 +46,7 @@ struct msghdr * create_credentials_msg(void) cmsg->cmsg_len = CMSG_LEN(sizeof(struct ucred)); memcpy(CMSG_DATA(cmsg), credentials, sizeof(struct ucred)); - free(credentials); + g_free(credentials); return msg; } @@ -100,8 +99,8 @@ void send_settings(char *data) sendmsg(socket_fd, msg, 0); close(socket_fd); - free(msg->msg_control); - free(msg); + g_free(msg->msg_control); + g_free(msg); } char * get_data(char *string) @@ -125,20 +124,20 @@ char * get_data(char *string) msg->msg_iov = &iov; sendmsg(socket_fd, msg, 0); - char *data = malloc(default_bufsz); + char *data = g_malloc(default_bufsz); int len = recv(socket_fd, data, default_bufsz, MSG_TRUNC); close(socket_fd); - free(msg->msg_control); - free(msg); + g_free(msg->msg_control); + g_free(msg); if (len < 0) { - free(data); + g_free(data); return NULL; } data[len] = '\0'; if (len >= default_bufsz) { /* msg was truncated, increase bufsz and try again */ default_bufsz += 8192; - free(data); + g_free(data); goto try_again; } return data; @@ -163,7 +162,7 @@ void parse_setup(char *setup_data) token = strtok_r(NULL, " ", &ptr); /* Parse banned IRQ data */ while(!strncmp(token, "IRQ", strlen("IRQ"))) { - new_irq = malloc(sizeof(irq_t)); + new_irq = g_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")) != 0) goto out; @@ -189,26 +188,24 @@ void parse_setup(char *setup_data) char *map = hex_to_bitmap(token[i]); for(j = 3; j >= 0; j--) { if(map[j] == '1') { - uint64_t *banned_cpu = malloc(sizeof(uint64_t)); + uint64_t *banned_cpu = g_malloc(sizeof(uint64_t)); *banned_cpu = cpu; setup.banned_cpus = g_list_append(setup.banned_cpus, banned_cpu); } cpu++; } - free(map); + g_free(map); } - free(copy); + g_free(copy); return; out: { /* Invalid data presented */ printf("Invalid data sent. Unexpected token: %s", token); - if (new_irq) { - free(new_irq); - } - free(copy); + g_free(new_irq); + g_free(copy); g_list_free(tree); exit(1); } @@ -252,7 +249,7 @@ void assign_cpu_lists(cpu_node_t *node, void *data __attribute__((unused))) void assign_cpu_mask(cpu_node_t *node, void *data __attribute__((unused))) { - char *mask = malloc(16 * sizeof(char)); + char *mask = g_malloc_n(16, sizeof(char)); mask[0] = '\0'; unsigned int sum = 0; GList *list_entry = g_list_first(node->cpu_list); @@ -281,7 +278,7 @@ void parse_into_tree(char *data) if (!data || strlen(data) == 0) return; - copy = strdup(data); + copy = g_strdup(data); if (!copy) return; @@ -289,8 +286,8 @@ void parse_into_tree(char *data) while(token != NULL) { /* Parse node data */ if(strncmp(token, "TYPE", strlen("TYPE")) != 0) { - free(copy); - goto out; + g_free(copy); + goto out; } new = g_malloc0(sizeof(cpu_node_t)); new->type = strtol(strtok_r(NULL, " ", &ptr), NULL, 10); @@ -312,7 +309,7 @@ void parse_into_tree(char *data) /* Parse assigned IRQ data */ while((token != NULL) && (!strncmp(token, "IRQ", strlen("IRQ")))) { - new_irq = malloc(sizeof(irq_t)); + new_irq = g_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")) != 0) goto out; @@ -343,7 +340,7 @@ void parse_into_tree(char *data) new = NULL; } - free(copy); + g_free(copy); for_each_node(tree, assign_cpu_lists, NULL); for_each_node(tree, assign_cpu_mask, NULL); return; @@ -351,12 +348,8 @@ void parse_into_tree(char *data) out: { /* Invalid data presented */ printf("Invalid data sent. Unexpected token: %s\n", token); - if (new_irq) { - free(new_irq); - } - if (new) { - free(new); - } + g_free(new_irq); + g_free(new); g_list_free(tree); exit(1); } diff --git a/ui/ui.c b/ui/ui.c index 5401d02..be5df5e 100644 --- a/ui/ui.c +++ b/ui/ui.c @@ -51,7 +51,7 @@ void show_footer(void) char * check_control_in_sleep_input(int max_len, int column_offest, int line_offset) { - char *input_to = malloc(max_len * sizeof(char)); + char *input_to = g_malloc_n(max_len, sizeof(char)); int iteration = 0; while(iteration < max_len) { int new = getch(); @@ -76,7 +76,7 @@ char * check_control_in_sleep_input(int max_len, int column_offest, int line_off attrset(COLOR_PAIR(5) | A_REVERSE | A_BOLD); break; case 27: - free(input_to); + g_free(input_to); return NULL; default: input_to[iteration] = new; @@ -112,7 +112,7 @@ int get_valid_sleep_input(int column_offest) char *error; new_sleep = strtol(input, &error, 10); if((*error == '\0') && (new_sleep >= 1)) { - free(input); + g_free(input); break; } new_sleep = setup.sleep; @@ -121,7 +121,7 @@ int get_valid_sleep_input(int column_offest) "Invalid input: %s ", input); refresh(); - free(input); + g_free(input); } attrset(COLOR_PAIR(1)); @@ -132,7 +132,7 @@ int get_valid_sleep_input(int column_offest) void get_banned_cpu(int *cpu, char *data __attribute__((unused))) { - cpu_ban_t *new = malloc(sizeof(cpu_ban_t)); + cpu_ban_t *new = g_malloc(sizeof(cpu_ban_t)); new->number = *cpu; new->is_banned = 1; all_cpus = g_list_append(all_cpus, new); @@ -237,7 +237,7 @@ void get_new_cpu_ban_values(cpu_ban_t *cpu, void *data) void get_cpu(cpu_node_t *node, void *data __attribute__((unused))) { if(node->type == OBJ_TYPE_CPU) { - cpu_ban_t *new = malloc(sizeof(cpu_ban_t)); + cpu_ban_t *new = g_malloc(sizeof(cpu_ban_t)); new->number = node->number; new->is_banned = 0; all_cpus = g_list_append(all_cpus, new); @@ -402,10 +402,9 @@ void get_irq_name(int end) char buffer[128]; if (irq_name == NULL) { - irq_name = malloc(sizeof(char *) * LINES); + irq_name = g_malloc_n(LINES, sizeof(char *)); for (i = 4; i < LINES; i++) { - irq_name[i] = malloc(sizeof(char) * 50); - memset(irq_name[i], 0, sizeof(char) * 50); + irq_name[i] = g_malloc0_n(50, sizeof(char)); } } From 7622883bad5b75d28d8e1cb3a2c48b9589aa82b4 Mon Sep 17 00:00:00 2001 From: Rosen Penev Date: Thu, 4 Jul 2024 13:54:12 -0700 Subject: [PATCH 7/9] remove malloc from ucred This just gets freed at the end after getting copied. Signed-off-by: Rosen Penev --- ui/irqbalance-ui.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/ui/irqbalance-ui.c b/ui/irqbalance-ui.c index 9bb76fd..d281f47 100644 --- a/ui/irqbalance-ui.c +++ b/ui/irqbalance-ui.c @@ -30,10 +30,11 @@ static int default_bufsz = 8192; struct msghdr * create_credentials_msg(void) { - struct ucred *credentials = g_malloc(sizeof(struct ucred)); - credentials->pid = getpid(); - credentials->uid = geteuid(); - credentials->gid = getegid(); + struct ucred credentials = { + .pid = getpid(), + .uid = geteuid(), + .gid = getegid(), + }; struct msghdr *msg = g_malloc0(sizeof(struct msghdr)); msg->msg_iovlen = 1; @@ -44,9 +45,7 @@ struct msghdr * create_credentials_msg(void) cmsg->cmsg_level = SOL_SOCKET; cmsg->cmsg_type = SCM_CREDENTIALS; cmsg->cmsg_len = CMSG_LEN(sizeof(struct ucred)); - memcpy(CMSG_DATA(cmsg), credentials, sizeof(struct ucred)); - - g_free(credentials); + memcpy(CMSG_DATA(cmsg), &credentials, sizeof(struct ucred)); return msg; } From e39848bfe45ea580bc0fd3b966a4199fb2fcd5b8 Mon Sep 17 00:00:00 2001 From: Rosen Penev Date: Thu, 4 Jul 2024 14:13:54 -0700 Subject: [PATCH 8/9] gcc analyzer: add NULL checks Found with -Wanalyzer-null-argument Signed-off-by: Rosen Penev --- irqbalance.c | 2 +- ui/ui.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/irqbalance.c b/irqbalance.c index 3875d5d..0b77376 100644 --- a/irqbalance.c +++ b/irqbalance.c @@ -503,7 +503,7 @@ gboolean sock_handle(gint fd, GIOCondition condition, gpointer user_data __attri recv_size - strlen("settings cpus ")); cpu_ban_string[recv_size - strlen("settings cpus ")] = '\0'; banned_cpumask_from_ui = strtok(cpu_ban_string, " "); - if (!strncmp(banned_cpumask_from_ui, "NULL", strlen("NULL"))) { + if (banned_cpumask_from_ui && !strncmp(banned_cpumask_from_ui, "NULL", strlen("NULL"))) { banned_cpumask_from_ui = NULL; free(cpu_ban_string); cpu_ban_string = NULL; diff --git a/ui/ui.c b/ui/ui.c index be5df5e..8d7c493 100644 --- a/ui/ui.c +++ b/ui/ui.c @@ -418,6 +418,8 @@ void get_irq_name(int end) cmd = alloca(sizeof(char) * (len + 1)); snprintf(cmd, len + 1, "cat /proc/interrupts | awk '{for (i=%d;i<=NF;i++)printf(\"%%s \", $i);print \"\"}' | cut -c-49", cpunr + 2); output = popen(cmd, "r"); + if (!output) + return; for (i = 0; i <= offset; i++) fgets(buffer, 50, output); for (i = 4; i < end; i++) From 4d537284c8d272cf05a64ee8ba226dd54c496a55 Mon Sep 17 00:00:00 2001 From: Rosen Penev Date: Thu, 4 Jul 2024 14:24:30 -0700 Subject: [PATCH 9/9] gcc analyzer: increase socket_name size Now matches sun_path size. Found with -Wanalyzer-out-of-bounds Signed-off-by: Rosen Penev --- irqbalance.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/irqbalance.c b/irqbalance.c index 0b77376..4f3b97d 100644 --- a/irqbalance.c +++ b/irqbalance.c @@ -70,7 +70,7 @@ unsigned long migrate_ratio = 0; #ifdef HAVE_IRQBALANCEUI int socket_fd; -char socket_name[64]; +char socket_name[108]; char *banned_cpumask_from_ui = NULL; #endif