From 03223398b38c550533fb8579b599fa84716d41c4 Mon Sep 17 00:00:00 2001 From: Egor Tensin Date: Fri, 26 Aug 2022 06:51:36 +0200 Subject: fix pthread error handling pthread functions return positive error codes. --- src/log.h | 18 +++++++++++++++++ src/server.c | 60 ++++++++++++++++++++++++-------------------------------- src/signal.c | 11 +++++++---- src/tcp_server.c | 16 +++++++-------- src/worker.c | 31 +++++++++++++---------------- 5 files changed, 73 insertions(+), 63 deletions(-) diff --git a/src/log.h b/src/log.h index 03e8856..ea9aacb 100644 --- a/src/log.h +++ b/src/log.h @@ -1,16 +1,34 @@ #ifndef __LOG_H__ #define __LOG_H__ +#include #include #include #include +#define CONCAT_INNER(a, b) a##b +#define CONCAT(a, b) CONCAT_INNER(a, b) + #define print_errno(s) \ { \ fprintf(stderr, "%s(%d): ", basename(__FILE__), __LINE__); \ perror(s); \ } +#define pthread_print_errno(var, s) \ + { \ + errno = var; \ + print_errno(s); \ + var = -var; \ + } + +#define pthread_check(expr, s) \ + { \ + int CONCAT(ret, __LINE__) = expr; \ + if (CONCAT(ret, __LINE__)) \ + pthread_print_errno(CONCAT(ret, __LINE__), s); \ + } + #define print_error(...) \ { \ fprintf(stderr, "%s(%d): ", basename(__FILE__), __LINE__); \ diff --git a/src/server.c b/src/server.c index e285a9e..509d144 100644 --- a/src/server.c +++ b/src/server.c @@ -65,16 +65,16 @@ static void *server_scheduler(void *_server) int ret = 0; ret = pthread_mutex_lock(&server->scheduler_mtx); - if (ret < 0) { - print_errno("pthread_mutex_lock"); + if (ret) { + pthread_print_errno(ret, "pthread_mutex_lock"); goto exit; } while (1) { while (!server_has_runs_and_workers(server)) { ret = pthread_cond_wait(&server->scheduler_cv, &server->scheduler_mtx); - if (ret < 0) { - print_errno("pthread_cond_wait"); + if (ret) { + pthread_print_errno(ret, "pthread_cond_wait"); goto unlock; } } @@ -85,8 +85,7 @@ static void *server_scheduler(void *_server) } unlock: - if (pthread_mutex_unlock(&server->scheduler_mtx)) - print_errno("pthread_mutex_unlock"); + pthread_check(pthread_mutex_unlock(&server->scheduler_mtx), "pthread_mutex_unlock"); exit: return NULL; @@ -105,32 +104,30 @@ int server_create(struct server *server, const struct settings *settings) ci_queue_create(&server->ci_queue); ret = pthread_mutex_init(&server->scheduler_mtx, NULL); - if (ret < 0) { - print_errno("pthread_mutex_init"); + if (ret) { + pthread_print_errno(ret, "pthread_mutex_init"); goto destroy_tcp_server; } ret = pthread_cond_init(&server->scheduler_cv, NULL); - if (ret < 0) { - print_errno("pthread_cond_init"); + if (ret) { + pthread_print_errno(ret, "pthread_cond_init"); goto destroy_scheduler_mtx; } ret = pthread_create(&server->scheduler, NULL, server_scheduler, server); - if (ret < 0) { - print_errno("pthread_create"); + if (ret) { + pthread_print_errno(ret, "pthread_create"); goto destroy_scheduler_cv; } return ret; destroy_scheduler_cv: - if (pthread_cond_destroy(&server->scheduler_cv)) - print_errno("pthread_cond_destroy"); + pthread_check(pthread_cond_destroy(&server->scheduler_cv), "pthread_cond_destroy"); destroy_scheduler_mtx: - if (pthread_mutex_destroy(&server->scheduler_mtx)) - print_errno("pthread_mutex_destroy"); + pthread_check(pthread_mutex_destroy(&server->scheduler_mtx), "pthread_mutex_destroy"); ci_queue_destroy(&server->ci_queue); @@ -144,12 +141,9 @@ destroy_tcp_server: void server_destroy(struct server *server) { - if (pthread_join(server->scheduler, NULL)) - print_errno("pthread_join"); - if (pthread_cond_destroy(&server->scheduler_cv)) - print_errno("pthread_cond_destroy"); - if (pthread_mutex_destroy(&server->scheduler_mtx)) - print_errno("pthread_mutex_destroy"); + pthread_check(pthread_join(server->scheduler, NULL), "pthread_join"); + pthread_check(pthread_cond_destroy(&server->scheduler_cv), "pthread_cond_destroy"); + pthread_check(pthread_mutex_destroy(&server->scheduler_mtx), "pthread_mutex_destroy"); ci_queue_destroy(&server->ci_queue); worker_queue_destroy(&server->worker_queue); tcp_server_destroy(&server->tcp_server); @@ -218,8 +212,8 @@ int server_new_worker(struct server *server, int fd) print_log("Registering a new worker\n"); ret = pthread_mutex_lock(&server->scheduler_mtx); - if (ret < 0) { - print_errno("pthread_mutex_lock"); + if (ret) { + pthread_print_errno(ret, "pthread_mutex_lock"); return ret; } @@ -230,14 +224,13 @@ int server_new_worker(struct server *server, int fd) worker_queue_push(&server->worker_queue, entry); ret = pthread_cond_signal(&server->scheduler_cv); - if (ret < 0) { - print_errno("pthread_cond_signal"); + if (ret) { + pthread_print_errno(ret, "pthread_cond_signal"); goto unlock; } unlock: - if (pthread_mutex_unlock(&server->scheduler_mtx)) - print_errno("pthread_mutex_unlock"); + pthread_check(pthread_mutex_unlock(&server->scheduler_mtx), "pthread_mutex_unlock"); return ret; } @@ -250,8 +243,8 @@ int server_ci_run(struct server *server, const char *url, const char *rev) print_log("Scheduling a new CI run for repository %s\n", url); ret = pthread_mutex_lock(&server->scheduler_mtx); - if (ret < 0) { - print_errno("pthread_mutex_lock"); + if (ret) { + pthread_print_errno(ret, "pthread_mutex_lock"); return ret; } @@ -262,14 +255,13 @@ int server_ci_run(struct server *server, const char *url, const char *rev) ci_queue_push(&server->ci_queue, entry); ret = pthread_cond_signal(&server->scheduler_cv); - if (ret < 0) { - print_errno("pthread_cond_signal"); + if (ret) { + pthread_print_errno(ret, "pthread_cond_signal"); goto unlock; } unlock: - if (pthread_mutex_unlock(&server->scheduler_mtx)) - print_errno("pthread_mutex_unlock"); + pthread_check(pthread_mutex_unlock(&server->scheduler_mtx), "pthread_mutex_unlock"); return ret; } diff --git a/src/signal.c b/src/signal.c index fc4ac8b..a0b9c3d 100644 --- a/src/signal.c +++ b/src/signal.c @@ -8,16 +8,19 @@ volatile sig_atomic_t global_stop_flag = 0; int signal_set_thread_attr(pthread_attr_t *attr) { + int ret = 0; + sigset_t set; sigemptyset(&set); sigaddset(&set, SIGINT); sigaddset(&set, SIGQUIT); sigaddset(&set, SIGTERM); - if (pthread_attr_setsigmask_np(attr, &set)) { - print_errno("pthread_attr_setsigmask_np"); - return -1; + ret = pthread_attr_setsigmask_np(attr, &set); + if (ret) { + pthread_print_errno(ret, "pthread_attr_setsigmask_np"); + return ret; } - return 0; + return ret; } diff --git a/src/tcp_server.c b/src/tcp_server.c index dc7d47b..e479cd0 100644 --- a/src/tcp_server.c +++ b/src/tcp_server.c @@ -58,29 +58,29 @@ int tcp_server_accept(const struct tcp_server *server, tcp_server_conn_handler h *ctx = (struct child_context){conn_fd, handler, arg}; ret = pthread_attr_init(&child_attr); - if (ret < 0) { - print_errno("pthread_attr_init"); + if (ret) { + pthread_print_errno(ret, "pthread_attr_init"); goto free_ctx; } ret = pthread_attr_setdetachstate(&child_attr, PTHREAD_CREATE_DETACHED); - if (ret < 0) { - print_errno("pthread_attr_setdetachstate"); + if (ret) { + pthread_print_errno(ret, "pthread_attr_setdetachstate"); goto destroy_attr; } ret = pthread_create(&child, &child_attr, connection_thread, ctx); - if (ret < 0) { - print_errno("pthread_create"); + if (ret) { + pthread_print_errno(ret, "pthread_create"); goto destroy_attr; } - pthread_attr_destroy(&child_attr); + pthread_check(pthread_attr_destroy(&child_attr), "pthread_attr_destroy"); return ret; destroy_attr: - pthread_attr_destroy(&child_attr); + pthread_check(pthread_attr_destroy(&child_attr), "pthread_attr_destroy"); free_ctx: free(ctx); diff --git a/src/worker.c b/src/worker.c index d26408f..1c27173 100644 --- a/src/worker.c +++ b/src/worker.c @@ -27,8 +27,8 @@ int worker_create(struct worker *worker, const struct settings *settings) worker->fd = ret; ret = pthread_mutex_init(&worker->task_mtx, NULL); - if (ret < 0) { - print_errno("pthread_mutex_init"); + if (ret) { + pthread_print_errno(ret, "pthread_mutex_init"); goto close; } @@ -49,12 +49,10 @@ void worker_destroy(struct worker *worker) print_log("Shutting down\n"); if (worker->task_active) { - if (pthread_join(worker->task, NULL)) - print_errno("pthread_join"); + pthread_check(pthread_join(worker->task, NULL), "pthread_join"); worker->task_active = 0; } - if (pthread_mutex_destroy(&worker->task_mtx)) - print_errno("pthread_mutex_destroy"); + pthread_check(pthread_mutex_destroy(&worker->task_mtx), "pthread_mutex_destroy"); close(worker->fd); libgit_shutdown(); } @@ -166,8 +164,8 @@ static int worker_schedule_task(struct worker *worker, const struct msg *msg, wo } ret = pthread_attr_init(&attr); - if (ret < 0) { - print_errno("pthread_attr_init"); + if (ret) { + pthread_print_errno(ret, "pthread_attr_init"); goto free_msg; } @@ -176,17 +174,17 @@ static int worker_schedule_task(struct worker *worker, const struct msg *msg, wo goto free_attr; ret = pthread_create(&worker->task, NULL, worker_task_wrapper, ctx); - if (ret < 0) { - print_errno("pthread_create"); + if (ret) { + pthread_print_errno(ret, "pthread_create"); goto free_attr; } worker->task_active = 1; - pthread_attr_destroy(&attr); + pthread_check(pthread_attr_destroy(&attr), "pthread_attr_destroy"); return ret; free_attr: - pthread_attr_destroy(&attr); + pthread_check(pthread_attr_destroy(&attr), "pthread_attr_destroy"); free_msg: msg_free(ctx->msg); @@ -209,7 +207,7 @@ static int worker_msg_handler(struct worker *worker, const struct msg *msg) case EBUSY: break; default: - print_errno("pthread_tryjoin_np"); + pthread_print_errno(ret, "pthread_tryjoin_np"); return ret; } } @@ -247,15 +245,14 @@ static int worker_msg_handler_lock(const struct msg *msg, void *_worker) int ret = 0; ret = pthread_mutex_lock(&worker->task_mtx); - if (ret < 0) { - print_errno("pthread_mutex_lock"); + if (ret) { + pthread_print_errno(ret, "pthread_mutex_lock"); return -1; } ret = worker_msg_handler(worker, msg); - if (pthread_mutex_unlock(&worker->task_mtx)) - print_errno("pthread_mutex_unlock"); + pthread_check(pthread_mutex_unlock(&worker->task_mtx), "pthread_mutex_lock"); return ret; } -- cgit v1.2.3