From 5b4396c4a1e48e01af7f0c136533e73083a3e041 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89rico=20Nogueira?= Date: Fri, 21 Jan 2022 15:41:13 -0300 Subject: [PATCH] Improve error handling and error messages. - Rrror out clearly on most allocation failures (the others will simply segfault). Avoid getting into weird program conditions in case some operations fail. - Improve organization of main(), no function pointer usage should be necessary. - Make main thread also a worker thread: avoid leaving a useless thread around simply waiting for others to complete. Also means one less thread to launch. --- erm.c | 22 ++++++-------- erm.h | 7 ++--- remove.c | 92 ++++++++++++++++++++++++++++++-------------------------- 3 files changed, 62 insertions(+), 59 deletions(-) diff --git a/erm.c b/erm.c index 2e5d608..d49cd2d 100644 --- a/erm.c +++ b/erm.c @@ -40,24 +40,22 @@ int main(int argc, char **argv) usage(1); } - file_action action = recursive ? recurse_into : single_file; - callback_action callback = recursive ? run_queue : NULL; - const char *err_fmt = recursive ? - "failed to queue '%s': %s\n" : "failed to remove '%s': %s\n"; - int rv = 0; for (int i = 0; i < argc; i++) { const char *path = argv[i]; - if (action(path)) { - fprintf(stderr, err_fmt, path, strerror(errno)); - if (stop_at_error) { - return 1; - } else { - rv = 1; + if (recursive) { + recurse_into(path, stop_at_error); + } else { + if (single_file(path)) { + if (stop_at_error) { + return 1; + } else { + rv = 1; + } } } } - if (callback) callback(); + if (recursive) run_queue(); return rv; } diff --git a/erm.h b/erm.h index 26fedb7..0483228 100644 --- a/erm.h +++ b/erm.h @@ -1,7 +1,4 @@ -typedef int (*file_action)(const char *path); -typedef int (*callback_action)(void); - /* remove.c */ -int recurse_into(const char *); +void recurse_into(const char *, int); +void run_queue(void); int single_file(const char *); -int run_queue(void); diff --git a/remove.c b/remove.c index 14d9885..f8fab41 100644 --- a/remove.c +++ b/remove.c @@ -54,30 +54,23 @@ static inline void queue_print(struct queue *q) #endif } -static inline int queue_add(struct queue *q, char *path, struct task *parent) +static inline void queue_add(struct queue *q, char *path, struct task *parent) { - int rv = 0; - pthread_mutex_lock(&q->mtx); if (q->len + 1 > q->size) { q->size *= 2; if (q->size == 0) q->size = 32; - void *t = realloc(q->tasks, q->size * sizeof(struct task)); - if (!t) { - rv = -1; - goto error; + q->tasks = realloc(q->tasks, q->size * sizeof(struct task)); + if (!q->tasks) { + fprintf(stderr, "queue memory exhaustion: %m\n"); + exit(1); } - q->tasks = t; } - struct task t = {.path = path, .parent = parent}; - q->tasks[q->len++] = t; + q->tasks[q->len++] = (struct task){.path = path, .parent = parent}; pthread_cond_signal(&q->cond); - -error: pthread_mutex_unlock(&q->mtx); - return rv; } static inline void queue_remove(struct queue *q, struct task *t) @@ -146,7 +139,8 @@ static void *process_queue_item(void *arg) pthread_mutex_unlock(&fd_mtx); continue; } else { - break; + fprintf(stderr, "couldn't open '%s': %m\n", t.path); + exit(1); } } DIR *d = fdopendir(dfd); @@ -222,51 +216,65 @@ fast_path_dir: return NULL; } -int run_queue(void) +static void exit_init(void) +{ + fprintf(stderr, "thread initialization failed: %m\n"); + exit(1); +} + +void run_queue(void) { long nproc_l = sysconf(_SC_NPROCESSORS_ONLN); if (nproc_l < 1) nproc_l = 1; if (nproc_l > 64) nproc_l = 64; nproc = nproc_l; - pthread_attr_t pattr; - if (pthread_attr_init(&pattr)) return -1; + + /* main thread will also be a task */ + unsigned nproc1 = nproc - 1; + + if (nproc1) { + pthread_attr_t pattr; + if (pthread_attr_init(&pattr)) exit_init(); #if defined(PTHREAD_STACK_MIN) - if (pthread_attr_setstacksize(&pattr, PTHREAD_STACK_MIN)) return -1; - if (pthread_attr_setguardsize(&pattr, 1)) return -1; + if (pthread_attr_setstacksize(&pattr, PTHREAD_STACK_MIN) || + pthread_attr_setguardsize(&pattr, 1)) exit_init(); #endif - pthread_t *threads = calloc(sizeof(pthread_t), nproc); - if (!threads) return -1; + pthread_t *threads = calloc(sizeof(pthread_t), nproc1); + if (!threads) exit_init(); - unsigned i, j = 0; - for (i = 0; i < nproc; i++) { - if (pthread_create(threads+i, &pattr, process_queue_item, &queue)) { - j = 1; - break; - } - } - pthread_attr_destroy(&pattr); - /* if creating threads fails, cancell all the already created ones */ - if (j) for (j = 0; j < i; j++) { - pthread_cancel(threads[j]); - } - for (j = 0; j < i; j++) { - pthread_join(threads[j], NULL); + for (unsigned i = 0; i < nproc1; i++) + if (pthread_create(threads+i, &pattr, process_queue_item, &queue)) exit_init(); + + pthread_attr_destroy(&pattr); } - return 0; + /* become one of the worker threads */ + process_queue_item(&queue); +} + +static void fail_single_file(const char *path) +{ + fprintf(stderr, "failed to remove '%s': %m\n", path); } int single_file(const char *path) { - return remove(path); + int rv = remove(path); + if (rv) fail_single_file(path); + return rv; } -int recurse_into(const char *path) +void recurse_into(const char *path, int stop_at_error) { - if (!remove(path)) return 0; - if (errno==ENOTEMPTY) return queue_add(&queue, strdup(path), NULL); - - return 1; + if (!remove(path)) { + return; + } else if (errno == ENOTEMPTY) { + queue_add(&queue, strdup(path), NULL); + return; + } else { + fail_single_file(path); + if (stop_at_error) exit(1); + } }