From f2917eb1637a0d9a0a003288c00dff5e5ba50297 Mon Sep 17 00:00:00 2001 From: TapTap Date: Sun, 30 Aug 2026 13:50:42 +0200 Subject: [PATCH 1/7] refactor: remove dead API, rename to_disk/config_is_remote_dest, fix perror newlines - Delete unused public array_list_extend (made static) - Delete legacy 16-parameter parallel_scanner_create wrapper; migrate test to parallel_scanner_create_with_options - Rename to_disk -> file_write_to_disk and is_remote_dest -> config_is_remote_dest for module_action naming convention - Remove stray newlines in perror calls (perror already appends one) --- src/client/client_send.c | 4 ++-- src/client/scanner.c | 14 ------------- src/client/scanner.h | 7 ------- src/shared/array_list.c | 2 +- src/shared/array_list.h | 1 - src/shared/config.c | 4 ++-- src/shared/config.h | 2 +- src/shared/file.c | 2 +- src/shared/file.h | 2 +- tests/test_chunk.c | 6 +++--- tests/test_compression.c | 4 ++-- tests/test_config.c | 30 ++++++++++++++-------------- tests/test_file.c | 40 +++++++++++++++++++------------------- tests/test_file_sendfile.c | 8 ++++---- tests/test_fuzz_smoke.c | 2 +- tests/test_metadata.c | 2 +- tests/test_property.c | 2 +- tests/test_robustness.c | 2 +- tests/test_scanner.c | 6 +++--- 19 files changed, 59 insertions(+), 81 deletions(-) diff --git a/src/client/client_send.c b/src/client/client_send.c index b010866..e883cf7 100644 --- a/src/client/client_send.c +++ b/src/client/client_send.c @@ -739,7 +739,7 @@ int send_files_multithreaded(Config* config) { sender_created = (thrd_create(&sender, send_chunks_multithreaded, context) == thrd_success); if (!scanner_created || !loader_created || !sender_created) { - perror("Error creating threads.\n"); + perror("Error creating threads"); pipeline_cancel(context); mtx_lock(&context->mutex_progress); context->sender_done = true; @@ -759,7 +759,7 @@ int send_files_multithreaded(Config* config) { if (config->show_progress) { progress_created = (thrd_create(&progress, progress_thread_fn, context) == thrd_success); if (!progress_created) { - perror("Error creating progress thread.\n"); + perror("Error creating progress thread"); /* Non-fatal; continue without progress reporting */ } } diff --git a/src/client/scanner.c b/src/client/scanner.c index 3f50a79..9f99530 100644 --- a/src/client/scanner.c +++ b/src/client/scanner.c @@ -645,20 +645,6 @@ ParallelScanner* parallel_scanner_create_with_options(const char* root_directory return ps; } -ParallelScanner* parallel_scanner_create(const char* root_directory, bool use_metadata, - unsigned long long chunk_size, char** exclude_patterns, - int exclude_count, char** include_patterns, - int include_count, unsigned long long max_size, - unsigned long long min_size, int max_depth, - int num_threads, bool follow_symlinks, bool copy_links, - bool safe_links, bool copy_unsafe_links, bool checksum) { - ScannerOptions options = {use_metadata, chunk_size, exclude_patterns, exclude_count, - include_patterns, include_count, max_size, min_size, - max_depth, num_threads, follow_symlinks, copy_links, - safe_links, copy_unsafe_links, checksum}; - return parallel_scanner_create_with_options(root_directory, &options); -} - Chunk* parallel_scanner_next(ParallelScanner* ps) { if (ps->initial_chunk) { Chunk* c = ps->initial_chunk; diff --git a/src/client/scanner.h b/src/client/scanner.h index 7d3ba56..9bb24f8 100644 --- a/src/client/scanner.h +++ b/src/client/scanner.h @@ -77,13 +77,6 @@ Chunk* directory_scanner_next(DirectoryScanner* scanner); bool directory_scanner_failed(const DirectoryScanner* scanner); void directory_scanner_destroy(DirectoryScanner* scanner); -ParallelScanner* parallel_scanner_create(const char* root_directory, bool use_metadata, - unsigned long long chunk_size, char** exclude_patterns, - int exclude_count, char** include_patterns, - int include_count, unsigned long long max_size, - unsigned long long min_size, int max_depth, - int num_threads, bool follow_symlinks, bool copy_links, - bool safe_links, bool copy_unsafe_links, bool checksum); ParallelScanner* parallel_scanner_create_with_options(const char* root_directory, const ScannerOptions* options); Chunk* parallel_scanner_next(ParallelScanner* scanner); diff --git a/src/shared/array_list.c b/src/shared/array_list.c index 4df49d0..6dc785e 100644 --- a/src/shared/array_list.c +++ b/src/shared/array_list.c @@ -34,7 +34,7 @@ void array_list_delete(ArrayList* array_list) { free(array_list); } -bool array_list_extend(ArrayList* array_list) { +static bool array_list_extend(ArrayList* array_list) { if (array_list == NULL) return false; int new_capacity = array_list->capacity * 2; diff --git a/src/shared/array_list.h b/src/shared/array_list.h index 9ecbe0b..69485bc 100644 --- a/src/shared/array_list.h +++ b/src/shared/array_list.h @@ -14,7 +14,6 @@ typedef struct ArrayList { ArrayList* array_list_create(void (*item_destroyer)(void* item)); void array_list_delete(ArrayList* array_list); -bool array_list_extend(ArrayList* array_list); bool array_list_add(ArrayList* array_list, void* item); void** array_list_to_array(const ArrayList* array_list); diff --git a/src/shared/config.c b/src/shared/config.c index 8938957..57c8eef 100644 --- a/src/shared/config.c +++ b/src/shared/config.c @@ -108,7 +108,7 @@ Config* config_create(void) { return config; } -bool is_remote_dest(const char* s) { +bool config_is_remote_dest(const char* s) { if (s == NULL) return false; const char* colon = strchr(s, ':'); @@ -124,7 +124,7 @@ bool is_remote_dest(const char* s) { } void config_parse_ssh_dest(Config* config) { - if (!is_remote_dest(config->receive_root_directory)) + if (!config_is_remote_dest(config->receive_root_directory)) return; config->transport = TRANSPORT_SSH; config->ssh_destination = str_dup(config->receive_root_directory); diff --git a/src/shared/config.h b/src/shared/config.h index 986a5c4..4a218b0 100644 --- a/src/shared/config.h +++ b/src/shared/config.h @@ -135,7 +135,7 @@ Config* config_create(void); void config_delete(Config* config); bool config_send(int file_descriptor, const Config* config); Config* config_receive(int file_descriptor); -bool is_remote_dest(const char* s); +bool config_is_remote_dest(const char* s); void config_parse_ssh_dest(Config* config); #endif diff --git a/src/shared/file.c b/src/shared/file.c index 6122d39..c8afb0e 100644 --- a/src/shared/file.c +++ b/src/shared/file.c @@ -647,7 +647,7 @@ File* receive_incremental_check(int fd, const Config* config, bool* skipped) { return file; } -bool to_disk(const char* path, const void* data, unsigned long long data_size, bool inplace, +bool file_write_to_disk(const char* path, const void* data, unsigned long long data_size, bool inplace, bool sparse) { if (!path || (!data && data_size != 0) || has_path_traversal(path)) return false; diff --git a/src/shared/file.h b/src/shared/file.h index 1bc153d..3c388c2 100644 --- a/src/shared/file.h +++ b/src/shared/file.h @@ -35,7 +35,7 @@ bool file_send_sendfile(File* file, int file_descriptor, bool use_metadata, int size_t file_content_to_buffer(File* file); FileMetadata* file_metadata_create(const struct stat* stats); void file_metadata_destroy(void* metadata); -bool to_disk(const char* path, const void* data, unsigned long long data_size, bool inplace, +bool file_write_to_disk(const char* path, const void* data, unsigned long long data_size, bool inplace, bool sparse); bool file_save_to_disk(const char* root_directory, const File* file, const Config* config); bool file_set_authorized_root(int fd, const char* canonical_path); diff --git a/tests/test_chunk.c b/tests/test_chunk.c index c36c80a..7913b8e 100644 --- a/tests/test_chunk.c +++ b/tests/test_chunk.c @@ -11,7 +11,7 @@ static void test_file_operations() { char* test_content = "Hello, Chunk System!"; unsigned long long test_len = strlen(test_content); - to_disk(test_path, test_content, test_len, false, false); + file_write_to_disk(test_path, test_content, test_len, false, false); File* f = file_create(test_path); EXPECT_NOT_NULL(f); @@ -43,8 +43,8 @@ static void test_chunk_operations() { char* content2 = "chunk item number 2"; unsigned long long len2 = strlen(content2); - to_disk(path1, content1, len1, false, false); - to_disk(path2, content2, len2, false, false); + file_write_to_disk(path1, content1, len1, false, false); + file_write_to_disk(path2, content2, len2, false, false); struct stat st1, st2; stat(path1, &st1); diff --git a/tests/test_compression.c b/tests/test_compression.c index ade4812..a859e6a 100644 --- a/tests/test_compression.c +++ b/tests/test_compression.c @@ -62,8 +62,8 @@ static void test_chunk_compress_decompress_roundtrip() { char* content2 = "chunk compression test file 2 with more data"; unsigned long long len2 = strlen(content2); - to_disk(path1, content1, len1, false, false); - to_disk(path2, content2, len2, false, false); + file_write_to_disk(path1, content1, len1, false, false); + file_write_to_disk(path2, content2, len2, false, false); struct stat st1, st2; EXPECT_EQ_INT(stat(path1, &st1), 0); diff --git a/tests/test_config.c b/tests/test_config.c index 817d6df..4f124db 100644 --- a/tests/test_config.c +++ b/tests/test_config.c @@ -244,26 +244,26 @@ static void test_config_receive_truncated() { close(p[1]); } -static void test_is_remote_dest() { +static void test_config_is_remote_dest() { /* Valid SSH-style destinations */ - EXPECT_TRUE(is_remote_dest("user@host:/path")); - EXPECT_TRUE(is_remote_dest("host:/path")); - EXPECT_TRUE(is_remote_dest("user@192.168.1.1:/remote/path")); + EXPECT_TRUE(config_is_remote_dest("user@host:/path")); + EXPECT_TRUE(config_is_remote_dest("host:/path")); + EXPECT_TRUE(config_is_remote_dest("user@192.168.1.1:/remote/path")); /* Invalid destinations */ - EXPECT_FALSE(is_remote_dest(NULL)); - EXPECT_FALSE(is_remote_dest("")); - EXPECT_FALSE(is_remote_dest(":")); - EXPECT_FALSE(is_remote_dest("/local/path")); - EXPECT_FALSE(is_remote_dest("relative/path")); + EXPECT_FALSE(config_is_remote_dest(NULL)); + EXPECT_FALSE(config_is_remote_dest("")); + EXPECT_FALSE(config_is_remote_dest(":")); + EXPECT_FALSE(config_is_remote_dest("/local/path")); + EXPECT_FALSE(config_is_remote_dest("relative/path")); /* C:/windows/path is treated as remote (colon with no preceding slash) */ - EXPECT_TRUE(is_remote_dest("C:/windows/path")); + EXPECT_TRUE(config_is_remote_dest("C:/windows/path")); /* Edge cases */ - EXPECT_FALSE(is_remote_dest("noslash")); - EXPECT_FALSE(is_remote_dest("/")); - EXPECT_TRUE(is_remote_dest("host:")); - EXPECT_TRUE(is_remote_dest("user@host:")); + EXPECT_FALSE(config_is_remote_dest("noslash")); + EXPECT_FALSE(config_is_remote_dest("/")); + EXPECT_TRUE(config_is_remote_dest("host:")); + EXPECT_TRUE(config_is_remote_dest("user@host:")); } void test_config() { @@ -278,5 +278,5 @@ void test_config() { test_config_send_receive_version_mismatch(); test_config_receive_truncated(); } - test_is_remote_dest(); + test_config_is_remote_dest(); } diff --git a/tests/test_file.c b/tests/test_file.c index d867b4f..ad82584 100644 --- a/tests/test_file.c +++ b/tests/test_file.c @@ -34,7 +34,7 @@ static void test_file_destroy_normal() { static void test_file_load_data() { const char* content = "Hello Load Test"; - EXPECT_TRUE(to_disk("test_file_load_data.txt", content, strlen(content), false, false)); + EXPECT_TRUE(file_write_to_disk("test_file_load_data.txt", content, strlen(content), false, false)); struct stat st; EXPECT_EQ_INT(stat("test_file_load_data.txt", &st), 0); @@ -87,15 +87,15 @@ static void test_file_save_to_disk() { rmdir("test_save_tmp"); } -static void test_to_disk_basic() { - const char* content = "Basic to_disk test"; - EXPECT_TRUE(to_disk("test_to_disk_basic.txt", content, strlen(content), false, false)); +static void test_file_write_to_disk_basic() { + const char* content = "Basic file_write_to_disk test"; + EXPECT_TRUE(file_write_to_disk("test_file_write_to_disk_basic.txt", content, strlen(content), false, false)); struct stat st; - EXPECT_EQ_INT(stat("test_to_disk_basic.txt", &st), 0); + EXPECT_EQ_INT(stat("test_file_write_to_disk_basic.txt", &st), 0); EXPECT_EQ_INT((int)st.st_size, (int)strlen(content)); - FILE* fp = fopen("test_to_disk_basic.txt", "rb"); + FILE* fp = fopen("test_file_write_to_disk_basic.txt", "rb"); EXPECT_NOT_NULL(fp); char buf[100]; size_t nread = fread(buf, 1, sizeof(buf), fp); @@ -103,12 +103,12 @@ static void test_to_disk_basic() { EXPECT_EQ_INT((int)nread, (int)strlen(content)); EXPECT_EQ_INT(memcmp(buf, content, strlen(content)), 0); - unlink("test_to_disk_basic.txt"); + unlink("test_file_write_to_disk_basic.txt"); } -static void test_to_disk_creates_dirs() { +static void test_file_write_to_disk_creates_dirs() { const char* content = "Nested dir test"; - EXPECT_TRUE(to_disk("test_nested_tmp/nested/file.txt", content, strlen(content), false, false)); + EXPECT_TRUE(file_write_to_disk("test_nested_tmp/nested/file.txt", content, strlen(content), false, false)); struct stat st; EXPECT_EQ_INT(stat("test_nested_tmp/nested/file.txt", &st), 0); @@ -126,15 +126,15 @@ static void test_to_disk_creates_dirs() { rmdir("test_nested_tmp"); } -static void test_to_disk_does_not_follow_symlink() { - const char* outside = "test_to_disk_outside.txt"; - const char* link = "test_to_disk_link.txt"; +static void test_file_write_to_disk_does_not_follow_symlink() { + const char* outside = "test_file_write_to_disk_outside.txt"; + const char* link = "test_file_write_to_disk_link.txt"; const char* content = "confined"; unlink(outside); unlink(link); - EXPECT_TRUE(to_disk(outside, "outside", 7, false, false)); + EXPECT_TRUE(file_write_to_disk(outside, "outside", 7, false, false)); EXPECT_EQ_INT(symlink(outside, link), 0); - EXPECT_TRUE(to_disk(link, content, strlen(content), false, false)); + EXPECT_TRUE(file_write_to_disk(link, content, strlen(content), false, false)); FILE* fp = fopen(outside, "rb"); char buf[16] = {0}; EXPECT_NOT_NULL(fp); @@ -151,7 +151,7 @@ static void test_to_disk_does_not_follow_symlink() { static void test_file_content_to_buffer() { const char* content = "Buffer content test"; - EXPECT_TRUE(to_disk("test_buffer_file.txt", content, strlen(content), false, false)); + EXPECT_TRUE(file_write_to_disk("test_buffer_file.txt", content, strlen(content), false, false)); File* f = file_create("test_buffer_file.txt"); EXPECT_NOT_NULL(f); @@ -273,7 +273,7 @@ static void test_file_send_no_path() { } static void test_file_metadata_create() { - EXPECT_TRUE(to_disk("test_meta_file.txt", "metadata test", 13, false, false)); + EXPECT_TRUE(file_write_to_disk("test_meta_file.txt", "metadata test", 13, false, false)); struct stat st; EXPECT_EQ_INT(stat("test_meta_file.txt", &st), 0); @@ -382,7 +382,7 @@ static void test_file_send_single_calls_metadata_and_path() { /* Create a real file on disk so we can have metadata */ const char* content = "File with metadata"; size_t len = strlen(content); - EXPECT_TRUE(to_disk("test_meta_send.txt", content, len, false, false)); + EXPECT_TRUE(file_write_to_disk("test_meta_send.txt", content, len, false, false)); struct stat st; EXPECT_EQ_INT(stat("test_meta_send.txt", &st), 0); @@ -455,9 +455,9 @@ void test_file() { test_file_load_data(); test_file_load_data_missing_file(); test_file_save_to_disk(); - test_to_disk_basic(); - test_to_disk_creates_dirs(); - test_to_disk_does_not_follow_symlink(); + test_file_write_to_disk_basic(); + test_file_write_to_disk_creates_dirs(); + test_file_write_to_disk_does_not_follow_symlink(); test_file_content_to_buffer(); test_file_save_to_disk_path_traversal(); test_file_save_to_disk_deep_traversal(); diff --git a/tests/test_file_sendfile.c b/tests/test_file_sendfile.c index fe6326d..61663e3 100644 --- a/tests/test_file_sendfile.c +++ b/tests/test_file_sendfile.c @@ -15,7 +15,7 @@ static void test_sendfile_basic() { const char* content = "Hello from sendfile test!"; size_t len = strlen(content); - EXPECT_TRUE(to_disk("test_sendfile_basic.txt", content, len, false, false)); + EXPECT_TRUE(file_write_to_disk("test_sendfile_basic.txt", content, len, false, false)); File* file = file_create("test_sendfile_basic.txt"); EXPECT_NOT_NULL(file); @@ -77,7 +77,7 @@ static void test_sendfile_basic() { static void test_sendfile_empty_file() { const char* content = ""; size_t len = 0; - EXPECT_TRUE(to_disk("test_sendfile_empty.txt", content, len, false, false)); + EXPECT_TRUE(file_write_to_disk("test_sendfile_empty.txt", content, len, false, false)); File* file = file_create("test_sendfile_empty.txt"); EXPECT_NOT_NULL(file); @@ -156,7 +156,7 @@ static void test_sendfile_missing_file() { static void test_sendfile_compression_fallback() { const char* content = "Compression fallback content"; size_t len = strlen(content); - EXPECT_TRUE(to_disk("test_sendfile_comp.txt", content, len, false, false)); + EXPECT_TRUE(file_write_to_disk("test_sendfile_comp.txt", content, len, false, false)); struct stat st; EXPECT_EQ_INT(stat("test_sendfile_comp.txt", &st), 0); @@ -222,7 +222,7 @@ static void test_sendfile_compression_fallback() { static void test_sendfile_no_path() { const char* content = "No path sendfile test"; size_t len = strlen(content); - EXPECT_TRUE(to_disk("test_sendfile_nopath.txt", content, len, false, false)); + EXPECT_TRUE(file_write_to_disk("test_sendfile_nopath.txt", content, len, false, false)); File* file = file_create("test_sendfile_nopath.txt"); EXPECT_NOT_NULL(file); diff --git a/tests/test_fuzz_smoke.c b/tests/test_fuzz_smoke.c index a7a46b3..193b86c 100644 --- a/tests/test_fuzz_smoke.c +++ b/tests/test_fuzz_smoke.c @@ -101,7 +101,7 @@ static void test_fuzz_delta_deserialize() { /* Smoke test for metadata_from_buf fuzz target */ static void test_fuzz_metadata_from_buf() { /* Create a real file to get metadata from */ - EXPECT_TRUE(to_disk("fuzz_meta_test.txt", "metadata test", 13, false, false)); + EXPECT_TRUE(file_write_to_disk("fuzz_meta_test.txt", "metadata test", 13, false, false)); struct stat st; EXPECT_EQ_INT(stat("fuzz_meta_test.txt", &st), 0); diff --git a/tests/test_metadata.c b/tests/test_metadata.c index e73e922..6db0d31 100644 --- a/tests/test_metadata.c +++ b/tests/test_metadata.c @@ -125,7 +125,7 @@ static void test_metadata_rejects_invalid_values() { static void test_file_restore_metadata() { const char* path = "temp_meta_restore_test.txt"; const char* content = "test content"; - EXPECT_TRUE(to_disk(path, content, strlen(content), false, false)); + EXPECT_TRUE(file_write_to_disk(path, content, strlen(content), false, false)); FileMetadata m; m.mode = 0644; diff --git a/tests/test_property.c b/tests/test_property.c index 4bcf4ef..6327bd6 100644 --- a/tests/test_property.c +++ b/tests/test_property.c @@ -84,7 +84,7 @@ static void test_property_chunk_roundtrip() { for (int i = 0; i < content_len; i++) content[i] = (char)(rand() % 256); - to_disk(path, content, content_len, false, false); + file_write_to_disk(path, content, content_len, false, false); struct stat st; stat(path, &st); diff --git a/tests/test_robustness.c b/tests/test_robustness.c index e4e20af..a03f163 100644 --- a/tests/test_robustness.c +++ b/tests/test_robustness.c @@ -13,7 +13,7 @@ static void test_chunk_deserialize_truncated() { char* path = "test_rob_trunc.txt"; char* content = "hello"; - to_disk(path, content, strlen(content), false, false); + file_write_to_disk(path, content, strlen(content), false, false); struct stat st; stat(path, &st); diff --git a/tests/test_scanner.c b/tests/test_scanner.c index 0dbcdc1..8667fd5 100644 --- a/tests/test_scanner.c +++ b/tests/test_scanner.c @@ -7,7 +7,7 @@ #include static void create_test_file(const char* path, const char* content) { - (void)to_disk(path, content, strlen(content), false, false); + (void)file_write_to_disk(path, content, strlen(content), false, false); } static void test_scanner_single_file() { @@ -394,8 +394,8 @@ static void test_parallel_scanner_root_chunks_without_workers() { create_test_file(file1, "a"); create_test_file(file2, "b"); - ParallelScanner* scanner = parallel_scanner_create(dir, false, 1, NULL, 0, NULL, 0, 0, 0, 0, 0, - false, false, false, false, false); + ScannerOptions options = {false, 1, NULL, 0, NULL, 0, 0, 0, 0, 0, false, false, false, false, false}; + ParallelScanner* scanner = parallel_scanner_create_with_options(dir, &options); EXPECT_NOT_NULL(scanner); int total_files = 0; From e3e766ba3d9f4cb0e26c0b4c952feeb8b2dbacd3 Mon Sep 17 00:00:00 2001 From: TapTap Date: Sun, 30 Aug 2026 13:53:59 +0200 Subject: [PATCH 2/7] refactor: table-driven CLI option parsing in client_cli - Add OPTION_TABLE for options that map directly to Config fields (flag/string/pos-int/nonneg-int/ull kinds) - Extract parse_ull_arg() replacing 5 duplicated strtoull blocks - Extract config_add_pattern() replacing duplicated --exclude/--include append logic, also reused by read_patterns_from_file() - parse_args() reduced from ~275 to ~160 lines --- src/client/client_cli.c | 342 ++++++++++++++++++++-------------------- 1 file changed, 174 insertions(+), 168 deletions(-) diff --git a/src/client/client_cli.c b/src/client/client_cli.c index 0e51ed4..533b49d 100644 --- a/src/client/client_cli.c +++ b/src/client/client_cli.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -87,105 +88,187 @@ static int set_nonneg_int_option(int* dest, const char* value, const char* optio static int read_patterns_from_file(const char* filepath, char*** patterns, int* count); +/* Parse a string as an unsigned long long. Returns 0 on success, -1 on error. */ +static int parse_ull_arg(const char* val, unsigned long long* out, const char* optname) { + char* end; + errno = 0; + unsigned long long v = strtoull(val, &end, 10); + if (errno != 0 || *end != '\0') { + fprintf(stderr, "Error: %s must be a non-negative integer\n", optname); + return -1; + } + *out = v; + return 0; +} + +/* Append a duplicated pattern to a growable pattern array. Returns 0 on success, -1 on error. */ +static int config_add_pattern(char*** patterns, int* count, const char* value, + const char* optname) { + char** tmp = realloc(*patterns, (*count + 1) * sizeof(char*)); + if (!tmp) { + fprintf(stderr, "Error: memory allocation failed for %s\n", optname); + return -1; + } + *patterns = tmp; + char* dup = str_dup(value); + if (!dup) { + fprintf(stderr, "Error: memory allocation failed for %s\n", optname); + return -1; + } + (*patterns)[(*count)++] = dup; + return 0; +} + +typedef enum { + OPT_FLAG, + OPT_STRING, + OPT_POS_INT, + OPT_NONNEG_INT, + OPT_ULL, +} OptKind; + +typedef struct { + const char* name; + const char* alias; + OptKind kind; + size_t offset; /* offsetof of the target field in Config */ +} OptionEntry; + +/* Options that map directly onto a Config field with no side effects. */ +static const OptionEntry OPTION_TABLE[] = { + {"--dry-run", "-n", OPT_FLAG, offsetof(Config, dry_run)}, + {"--delete", NULL, OPT_FLAG, offsetof(Config, use_delete)}, + {"--incremental", NULL, OPT_FLAG, offsetof(Config, use_incremental)}, + {"--delta", NULL, OPT_FLAG, offsetof(Config, use_delta)}, + {"--save-to-disk", NULL, OPT_FLAG, offsetof(Config, save_to_disk)}, + {"--progress", NULL, OPT_FLAG, offsetof(Config, show_progress)}, + {"--tls", NULL, OPT_FLAG, offsetof(Config, use_tls)}, + {"--backup", NULL, OPT_FLAG, offsetof(Config, backup)}, + {"--stats", NULL, OPT_FLAG, offsetof(Config, stats)}, + {"--partial", NULL, OPT_FLAG, offsetof(Config, partial)}, + {"--links", "-l", OPT_FLAG, offsetof(Config, follow_symlinks)}, + {"--copy-links", NULL, OPT_FLAG, offsetof(Config, copy_links)}, + {"--safe-links", NULL, OPT_FLAG, offsetof(Config, safe_links)}, + {"--copy-unsafe-links", NULL, OPT_FLAG, offsetof(Config, copy_unsafe_links)}, + {"--sparse", "-S", OPT_FLAG, offsetof(Config, preserve_sparse)}, + {"--inplace", NULL, OPT_FLAG, offsetof(Config, inplace)}, + {"--checksum", NULL, OPT_FLAG, offsetof(Config, checksum)}, + + {"--source-dir", NULL, OPT_STRING, offsetof(Config, send_directory)}, + {"--dest-dir", NULL, OPT_STRING, offsetof(Config, receive_root_directory)}, + {"--server-host", NULL, OPT_STRING, offsetof(Config, server_host)}, + {"--cert", NULL, OPT_STRING, offsetof(Config, tls_cert)}, + {"--key", NULL, OPT_STRING, offsetof(Config, tls_key)}, + {"--ca", NULL, OPT_STRING, offsetof(Config, tls_ca)}, + {"--backup-dir", NULL, OPT_STRING, offsetof(Config, backup_dir)}, + {"--fastsync-server-path", NULL, OPT_STRING, offsetof(Config, fastsync_server_path)}, + {"--partial-dir", NULL, OPT_STRING, offsetof(Config, partial_dir)}, + {"--suffix", NULL, OPT_STRING, offsetof(Config, suffix)}, + + {"--timeout", NULL, OPT_POS_INT, offsetof(Config, timeout)}, + {"--contimeout", NULL, OPT_POS_INT, offsetof(Config, contimeout)}, + {"--max-depth", NULL, OPT_NONNEG_INT, offsetof(Config, max_depth)}, + + {"--max-size", NULL, OPT_ULL, offsetof(Config, max_size)}, + {"--min-size", NULL, OPT_ULL, offsetof(Config, min_size)}, +}; + +static bool opt_is(const char* arg, const char* name, const char* alias) { + return strcmp(arg, name) == 0 || (alias && strcmp(arg, alias) == 0); +} + +static const OptionEntry* find_table_option(const char* arg) { + for (size_t i = 0; i < sizeof(OPTION_TABLE) / sizeof(OPTION_TABLE[0]); i++) + if (opt_is(arg, OPTION_TABLE[i].name, OPTION_TABLE[i].alias)) + return &OPTION_TABLE[i]; + return NULL; +} + +static int apply_table_option(Config* config, const OptionEntry* entry, const char* value) { + void* field = (char*)config + entry->offset; + switch (entry->kind) { + case OPT_FLAG: + *(bool*)field = true; + return 0; + case OPT_STRING: + return set_string_option((char**)field, value, entry->name); + case OPT_POS_INT: + return set_positive_int_option((int*)field, value, entry->name); + case OPT_NONNEG_INT: + return set_nonneg_int_option((int*)field, value, entry->name); + case OPT_ULL: { + unsigned long long v; + if (parse_ull_arg(value, &v, entry->name) != 0) + return -1; + *(unsigned long long*)field = v; + return 0; + } + } + return -1; +} + /* Parse CLI arguments into config. Returns 0 on success, -1 on error, 1 for help/clean-exit. */ int parse_args(Config* config, int argc, char* argv[], int* positional_args, int* positional_count) { for (int i = 1; i < argc; i++) { - if (strcmp(argv[i], "--help") == 0) { + const OptionEntry* entry = find_table_option(argv[i]); + if (entry) { + if (entry->kind != OPT_FLAG) { + if (i + 1 >= argc) { + fprintf(stderr, "Error: missing argument for %s\n", entry->name); + return -1; + } + if (apply_table_option(config, entry, argv[++i]) != 0) + return -1; + } else if (apply_table_option(config, entry, NULL) != 0) { + return -1; + } + continue; + } + + if (opt_is(argv[i], "--help", NULL)) { print_usage(); return 1; - } else if (strcmp(argv[i], "-V") == 0 || strcmp(argv[i], "--version") == 0) { + } else if (opt_is(argv[i], "-V", "--version")) { printf("fastsync version %s\n", PROTOCOL_VERSION); return 1; - } else if (strcmp(argv[i], "-a") == 0 || strcmp(argv[i], "--archive") == 0) { + } else if (opt_is(argv[i], "-a", "--archive")) { config->use_compression = true; config->use_multithreading = true; config->use_metadata = true; log_message(LOG_LEVEL_INFO, "Enabled archive mode (-c -m -M)"); - } else if (strcmp(argv[i], "-n") == 0 || strcmp(argv[i], "--dry-run") == 0) { - config->dry_run = true; - } else if (strcmp(argv[i], "-p") == 0 && i + 1 < argc) { + } else if (opt_is(argv[i], "-p", NULL) && i + 1 < argc) { if (set_positive_int_option(&config->ssh_port, argv[++i], "-p") != 0) return -1; if (config->ssh_port > 65535) { log_message(LOG_LEVEL_ERROR, "SSH port must be 1-65535\n"); return -1; } - } else if (strcmp(argv[i], "--delete") == 0) { - config->use_delete = true; - } else if (strcmp(argv[i], "--exclude") == 0 && i + 1 < argc) { - char** tmp = realloc(config->exclude_patterns, (config->exclude_count + 1) * sizeof(char*)); - if (!tmp) { - fprintf(stderr, "Error: memory allocation failed for --exclude\n"); + } else if (opt_is(argv[i], "--exclude", NULL) && i + 1 < argc) { + if (config_add_pattern(&config->exclude_patterns, &config->exclude_count, argv[++i], + "--exclude") != 0) return -1; - } - config->exclude_patterns = tmp; - char* dup = str_dup(argv[++i]); - if (!dup) { - fprintf(stderr, "Error: memory allocation failed for --exclude\n"); + } else if (opt_is(argv[i], "--include", NULL) && i + 1 < argc) { + if (config_add_pattern(&config->include_patterns, &config->include_count, argv[++i], + "--include") != 0) return -1; - } - config->exclude_patterns[config->exclude_count++] = dup; - } else if (strcmp(argv[i], "--include") == 0 && i + 1 < argc) { - char** tmp = realloc(config->include_patterns, (config->include_count + 1) * sizeof(char*)); - if (!tmp) { - fprintf(stderr, "Error: memory allocation failed for --include\n"); + } else if (opt_is(argv[i], "--delta-block", NULL) && i + 1 < argc) { + unsigned long long val; + if (parse_ull_arg(argv[++i], &val, "--delta-block") != 0) return -1; - } - config->include_patterns = tmp; - char* dup = str_dup(argv[++i]); - if (!dup) { - fprintf(stderr, "Error: memory allocation failed for --include\n"); - return -1; - } - config->include_patterns[config->include_count++] = dup; - } else if (strcmp(argv[i], "--max-size") == 0 && i + 1 < argc) { - char* end; - errno = 0; - unsigned long long val = strtoull(argv[++i], &end, 10); - if (errno != 0 || *end != '\0') { - fprintf(stderr, "Error: --max-size must be a non-negative integer\n"); - return -1; - } - config->max_size = val; - } else if (strcmp(argv[i], "--min-size") == 0 && i + 1 < argc) { - char* end; - errno = 0; - unsigned long long val = strtoull(argv[++i], &end, 10); - if (errno != 0 || *end != '\0') { - fprintf(stderr, "Error: --min-size must be a non-negative integer\n"); - return -1; - } - config->min_size = val; - } else if (strcmp(argv[i], "--incremental") == 0) { - config->use_incremental = true; - } else if (strcmp(argv[i], "--delta") == 0) { - config->use_delta = true; - } else if (strcmp(argv[i], "--delta-block") == 0 && i + 1 < argc) { - char* end; - errno = 0; - unsigned long long val = strtoull(argv[++i], &end, 10); - if (errno != 0 || *end != '\0') { - fprintf(stderr, "Error: --delta-block must be a positive integer\n"); - return -1; - } if (val >= DELTA_BLOCK_SIZE_MIN && val <= DELTA_BLOCK_SIZE_MAX) config->delta_block_size = (uint32_t)val; else fprintf(stderr, "Warning: --delta-block value %llu out of range, using default\n", val); - } else if (strcmp(argv[i], "--delta-max") == 0 && i + 1 < argc) { - char* end; - errno = 0; - unsigned long long val = strtoull(argv[++i], &end, 10); - if (errno != 0 || *end != '\0') { - fprintf(stderr, "Error: --delta-max must be a positive integer\n"); + } else if (opt_is(argv[i], "--delta-max", NULL) && i + 1 < argc) { + unsigned long long val; + if (parse_ull_arg(argv[++i], &val, "--delta-max") != 0) return -1; - } if (val >= DELTA_MIN_FILE_SIZE) config->delta_max_file_size = val; else fprintf(stderr, "Warning: --delta-max value %llu too small, using default\n", val); - } else if (strcmp(argv[i], "-c") == 0 || strcmp(argv[i], "-z") == 0) { + } else if (opt_is(argv[i], "-c", "-z")) { config->use_compression = true; log_message(LOG_LEVEL_INFO, "Enabled Compression"); if (i + 1 < argc) { @@ -201,30 +284,19 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args, i++; } } - } else if (strcmp(argv[i], "--source-dir") == 0 && i + 1 < argc) { - if (set_string_option(&config->send_directory, argv[++i], "--source-dir") != 0) - return -1; - } else if (strcmp(argv[i], "--dest-dir") == 0 && i + 1 < argc) { - if (set_string_option(&config->receive_root_directory, argv[++i], "--dest-dir") != 0) - return -1; - } else if (strcmp(argv[i], "--save-to-disk") == 0) { - config->save_to_disk = true; - } else if (strcmp(argv[i], "-M") == 0 || strcmp(argv[i], "--preserve") == 0) { + } else if (opt_is(argv[i], "-M", "--preserve")) { config->use_metadata = true; log_message(LOG_LEVEL_INFO, "Enabled metadata preservation"); - } else if (strcmp(argv[i], "-f") == 0 || strcmp(argv[i], "--sendfile") == 0) { + } else if (opt_is(argv[i], "-f", "--sendfile")) { config->use_sendfile = true; log_message(LOG_LEVEL_INFO, "Enabled sendfile"); - } else if (strcmp(argv[i], "-m") == 0) { + } else if (opt_is(argv[i], "-m", NULL)) { config->use_multithreading = true; log_message(LOG_LEVEL_INFO, "Enabled Multithreading"); - } else if (strcmp(argv[i], "-s") == 0) { + } else if (opt_is(argv[i], "-s", NULL)) { config->use_chunk_serialization = true; log_message(LOG_LEVEL_INFO, "Enabled Chunk Serialization"); - } else if (strcmp(argv[i], "--server-host") == 0 && i + 1 < argc) { - if (set_string_option(&config->server_host, argv[++i], "--server-host") != 0) - return -1; - } else if (strcmp(argv[i], "--server-port") == 0 && i + 1 < argc) { + } else if (opt_is(argv[i], "--server-port", NULL) && i + 1 < argc) { if (!parse_positive_int(argv[++i], &config->server_port)) { fprintf(stderr, "Error: invalid --server-port value: %s\n", argv[i]); return -1; @@ -233,11 +305,11 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args, fprintf(stderr, "Error: server port must be 1-65535\n"); return -1; } - } else if (strcmp(argv[i], "--bwlimit") == 0 && i + 1 < argc) { - char* end; - errno = 0; - unsigned long long kbps = strtoull(argv[++i], &end, 10); - if (errno != 0 || *end != '\0' || kbps == 0) { + } else if (opt_is(argv[i], "--bwlimit", NULL) && i + 1 < argc) { + unsigned long long kbps; + if (parse_ull_arg(argv[++i], &kbps, "--bwlimit") != 0) + return -1; + if (kbps == 0) { fprintf(stderr, "Error: --bwlimit must be a positive integer\n"); return -1; } @@ -247,45 +319,16 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args, } io_set_bwlimit(kbps * 1024); log_message(LOG_LEVEL_INFO, "Set bandwidth limit to %llu KB/s", kbps); - } else if (strcmp(argv[i], "--progress") == 0) { - config->show_progress = true; - } else if (strcmp(argv[i], "--chunk-size") == 0 && i + 1 < argc) { - char* end; - errno = 0; - unsigned long long val = strtoull(argv[++i], &end, 10); - if (errno != 0 || *end != '\0' || val == 0) { + } else if (opt_is(argv[i], "--chunk-size", NULL) && i + 1 < argc) { + unsigned long long val; + if (parse_ull_arg(argv[++i], &val, "--chunk-size") != 0) + return -1; + if (val == 0) { fprintf(stderr, "Error: --chunk-size must be a positive integer\n"); return -1; } config->chunk_size = val; - } else if (strcmp(argv[i], "--tls") == 0) { - config->use_tls = true; - } else if (strcmp(argv[i], "--cert") == 0 && i + 1 < argc) { - if (set_string_option(&config->tls_cert, argv[++i], "--cert") != 0) - return -1; - } else if (strcmp(argv[i], "--key") == 0 && i + 1 < argc) { - if (set_string_option(&config->tls_key, argv[++i], "--key") != 0) - return -1; - } else if (strcmp(argv[i], "--ca") == 0 && i + 1 < argc) { - if (set_string_option(&config->tls_ca, argv[++i], "--ca") != 0) - return -1; - } else if (strcmp(argv[i], "--timeout") == 0 && i + 1 < argc) { - if (set_positive_int_option(&config->timeout, argv[++i], "--timeout") != 0) - return -1; - } else if (strcmp(argv[i], "--contimeout") == 0 && i + 1 < argc) { - if (set_positive_int_option(&config->contimeout, argv[++i], "--contimeout") != 0) - return -1; - } else if (strcmp(argv[i], "--backup") == 0) { - config->backup = true; - } else if (strcmp(argv[i], "--backup-dir") == 0 && i + 1 < argc) { - if (set_string_option(&config->backup_dir, argv[++i], "--backup-dir") != 0) - return -1; - } else if (strcmp(argv[i], "--stats") == 0) { - config->stats = true; - } else if (strcmp(argv[i], "--max-depth") == 0 && i + 1 < argc) { - if (set_nonneg_int_option(&config->max_depth, argv[++i], "--max-depth") != 0) - return -1; - } else if (strcmp(argv[i], "--log-file") == 0 && i + 1 < argc) { + } else if (opt_is(argv[i], "--log-file", NULL) && i + 1 < argc) { if (config->log_file) { fclose(config->log_file); config->log_file = NULL; @@ -298,46 +341,20 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args, } config->log_file = lf; log_set_file(lf); - } else if (strcmp(argv[i], "--exclude-from") == 0 && i + 1 < argc) { + } else if (opt_is(argv[i], "--exclude-from", NULL) && i + 1 < argc) { if (read_patterns_from_file(argv[++i], &config->exclude_patterns, &config->exclude_count) != 0) return -1; - } else if (strcmp(argv[i], "--include-from") == 0 && i + 1 < argc) { + } else if (opt_is(argv[i], "--include-from", NULL) && i + 1 < argc) { if (read_patterns_from_file(argv[++i], &config->include_patterns, &config->include_count) != 0) return -1; - } else if (strcmp(argv[i], "--partial") == 0) { - config->partial = true; - } else if (strcmp(argv[i], "--fastsync-server-path") == 0 && i + 1 < argc) { - if (set_string_option(&config->fastsync_server_path, argv[++i], "--fastsync-server-path") != - 0) - return -1; - } else if (strcmp(argv[i], "-v") == 0 || strcmp(argv[i], "--verbose") == 0) { + } else if (opt_is(argv[i], "-v", "--verbose")) { set_log_level(LOG_LEVEL_DEBUG); - } else if (strcmp(argv[i], "-l") == 0 || strcmp(argv[i], "--links") == 0) { - config->follow_symlinks = true; - } else if (strcmp(argv[i], "--copy-links") == 0) { - config->copy_links = true; - } else if (strcmp(argv[i], "--safe-links") == 0) { - config->safe_links = true; - } else if (strcmp(argv[i], "--copy-unsafe-links") == 0) { - config->copy_unsafe_links = true; - } else if (strcmp(argv[i], "-S") == 0 || strcmp(argv[i], "--sparse") == 0) { - config->preserve_sparse = true; - } else if (strcmp(argv[i], "--inplace") == 0) { - config->inplace = true; - } else if (strcmp(argv[i], "--partial-dir") == 0 && i + 1 < argc) { - if (set_string_option(&config->partial_dir, argv[++i], "--partial-dir") != 0) - return -1; - } else if (strcmp(argv[i], "--suffix") == 0 && i + 1 < argc) { - if (set_string_option(&config->suffix, argv[++i], "--suffix") != 0) - return -1; - } else if (strcmp(argv[i], "-T") == 0 && i + 1 < argc) { + } else if (opt_is(argv[i], "-T", NULL) && i + 1 < argc) { if (set_positive_int_option(&config->timeout, argv[++i], "-T") != 0) return -1; - } else if (strcmp(argv[i], "--checksum") == 0) { - config->checksum = true; - } else if (strcmp(argv[i], "--compress-level") == 0 && i + 1 < argc) { + } else if (opt_is(argv[i], "--compress-level", NULL) && i + 1 < argc) { if (set_positive_int_option(&config->compression_level, argv[++i], "--compress-level") != 0) return -1; if (config->compression_level < 1 || config->compression_level > 22) { @@ -381,22 +398,11 @@ static int read_patterns_from_file(const char* filepath, char*** patterns, int* p[--len] = '\0'; if (len == 0) continue; - char** tmp = realloc(*patterns, (*count + 1) * sizeof(char*)); - if (!tmp) { - fprintf(stderr, "Error: memory allocation failed for pattern file\n"); + if (config_add_pattern(patterns, count, p, "pattern file") != 0) { free(line); fclose(fp); return -1; } - *patterns = tmp; - char* dup = str_dup(p); - if (!dup) { - fprintf(stderr, "Error: memory allocation failed for pattern file\n"); - free(line); - fclose(fp); - return -1; - } - (*patterns)[(*count)++] = dup; } free(line); fclose(fp); From 3fa3e150ce6ea734b1cb2e8c0875f7a03cda872c Mon Sep 17 00:00:00 2001 From: TapTap Date: Sun, 30 Aug 2026 13:56:27 +0200 Subject: [PATCH 3/7] refactor: split parallel_scanner_create_with_options into focused helpers - parallel_scanner_init(): result queue + sync primitive setup with unwinding - batch_files(): root-file chunk batching, reusable by other scan paths - scan_root_directory()/scan_root_entry(): root-dir scanning - spawn_parallel_workers(): worker thread creation with per-thread arg setup Main function reduced from ~230 to ~40 lines --- src/client/scanner.c | 402 +++++++++++++++++++++++-------------------- 1 file changed, 220 insertions(+), 182 deletions(-) diff --git a/src/client/scanner.c b/src/client/scanner.c index 9f99530..3c67145 100644 --- a/src/client/scanner.c +++ b/src/client/scanner.c @@ -412,18 +412,11 @@ static void parallel_scanner_creation_failed(ParallelScanner* ps) { mtx_unlock(&ps->result_mutex); } -ParallelScanner* parallel_scanner_create_with_options(const char* root_directory, - const ScannerOptions* options) { - if (!root_directory || !options) - return NULL; - ParallelScanner* ps = calloc(1, sizeof(ParallelScanner)); - if (!ps) - return NULL; +/* Initialize result queue and synchronization primitives. Returns true on success. */ +static bool parallel_scanner_init(ParallelScanner* ps) { ps->result_queue = queue_create(100, chunk_destroy); - if (!ps->result_queue) { - free(ps); - return NULL; - } + if (!ps->result_queue) + return false; atomic_init(&ps->cancelled, false); int init = 0; bool ok = true; @@ -448,14 +441,220 @@ ParallelScanner* parallel_scanner_create_with_options(const char* root_directory if (init >= 1) mtx_destroy(&ps->result_mutex); queue_destroy(ps->result_queue); - free(ps); + ps->result_queue = NULL; + return false; + } + return true; +} + +/* Split files into chunks of roughly chunk_size bytes. Returns the first chunk (also stored + * chunks beyond the first are enqueued on `queue`). Nulls out consumed entries in `files`. + * Sets *failed on allocation/enqueue errors. */ +static Chunk* batch_files(ArrayList* files, unsigned long long chunk_size, Queue* queue, + bool* failed) { + Chunk* first = NULL; + if (files->size <= 0) + return NULL; + ArrayList* batch = array_list_create(NULL); + if (!batch) { + *failed = true; return NULL; } + unsigned long long batch_size = 0; + for (int i = 0; i < files->size; i++) { + File* f = (File*)files->items[i]; + if (!array_list_add(batch, f)) { + *failed = true; + break; + } + batch_size += f->data->size; + if (batch_size >= chunk_size || i == files->size - 1) { + void** items = array_list_to_array(batch); + if (!items) { + *failed = true; + array_list_delete(batch); + batch = NULL; + break; + } + Chunk* c = chunk_create((File**)items, batch->size); + free(items); + if (!c) { + *failed = true; + array_list_delete(batch); + batch = NULL; + break; + } + int batch_start = i - batch->size + 1; + for (int j = batch_start; j <= i; j++) + files->items[j] = NULL; + batch->item_destroyer = NULL; + array_list_delete(batch); + batch = NULL; + if (!first) { + first = c; + } else { + if (!queue_enqueue(queue, c)) { + chunk_destroy(c); + *failed = true; + } + } + if (i < files->size - 1) { + batch = array_list_create(NULL); + if (!batch) { + *failed = true; + break; + } + batch_size = 0; + } + } + } + if (batch) { + batch->item_destroyer = NULL; + array_list_delete(batch); + } + return first; +} +/* Scan one root-directory entry into either the subdirs or files list. */ +static void scan_root_entry(const ScannerOptions* options, const char* root_directory, + const struct dirent* entry, ArrayList* root_files, + ArrayList* subdirs, ParallelScanner* ps) { + ScannerEntry inspected; + int inspection = + scanner_inspect_entry(options, root_directory, root_directory, entry->d_name, &inspected); + if (inspection < 0) { + ps->failed = true; + return; + } + if (inspection == 0) + return; + char* cur_path = inspected.path; + struct stat st = inspected.stats; + if (inspected.is_directory) { + if (!array_list_add(subdirs, cur_path)) { + free(cur_path); + ps->failed = true; + } + return; + } + File* file = file_create(cur_path); + free(cur_path); + if (!file) { + ps->failed = true; + return; + } + file->data->size = st.st_size; + if (options->use_metadata) + file->metadata = file_metadata_create(&st); + if (options->use_metadata && !file->metadata) { + file_destroy(file); + ps->failed = true; + return; + } + if (!array_list_add(root_files, file)) { + file_destroy(file); + ps->failed = true; + } +} + +/* Scan the root directory itself, collecting root files and subdirectories. + * Returns false if the root directory could not be opened. */ +static bool scan_root_directory(ParallelScanner* ps, const char* root_directory, + const ScannerOptions* options, ArrayList* root_files, + ArrayList* subdirs) { DIR* dir = opendir(root_directory); if (!dir) { perror("Could not open root directory for parallel scan"); - parallel_scanner_destroy(ps); + return false; + } + const struct dirent* entry; + while ((entry = readdir(dir)) != NULL) { + if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0) + continue; + scan_root_entry(options, root_directory, entry, root_files, subdirs, ps); + } + closedir(dir); + return true; +} + +/* Spawn worker threads, one per group of subdirectories. */ +static void spawn_parallel_workers(ParallelScanner* ps, ArrayList* subdirs, + const ScannerOptions* options, unsigned long long cs) { + if (subdirs->size <= 0) + return; + int n = options->num_threads > 0 ? options->num_threads : 4; + if (n > subdirs->size) + n = subdirs->size; + + ps->num_threads = n; + ps->expected_threads = n; + ps->threads = calloc(n, sizeof(thrd_t)); + if (!ps->threads) { + ps->num_threads = 0; + ps->expected_threads = 0; + ps->failed = true; + return; + } + int dirs_per_thread = subdirs->size / n; + int remainder = subdirs->size % n; + int start = 0; + ps->num_threads = 0; + for (int t = 0; t < n; t++) { + int count = dirs_per_thread + (t < remainder ? 1 : 0); + if (count == 0) + break; + ParallelWorkerArg* wa = calloc(1, sizeof(ParallelWorkerArg)); + if (!wa) { + parallel_scanner_creation_failed(ps); + break; + } + wa->ps = ps; + wa->dirs = calloc(count, sizeof(char*)); + if (!wa->dirs) { + free(wa); + parallel_scanner_creation_failed(ps); + break; + } + bool dup_ok = true; + for (int j = 0; j < count; j++) { + wa->dirs[j] = str_dup((char*)subdirs->items[start + j]); + if (!wa->dirs[j]) + dup_ok = false; + } + if (!dup_ok) { + for (int j = 0; j < count; j++) + free(wa->dirs[j]); + free(wa->dirs); + free(wa); + parallel_scanner_creation_failed(ps); + break; + } + wa->dir_count = count; + wa->options = *options; + wa->options.chunk_size = cs; + start += count; + if (thrd_create(&ps->threads[t], parallel_worker_thread, wa) != thrd_success) { + for (int j = 0; j < count; j++) + free(wa->dirs[j]); + free(wa->dirs); + free(wa); + parallel_scanner_creation_failed(ps); + break; + } + ps->num_threads++; + ps->created_threads++; + } +} + +ParallelScanner* parallel_scanner_create_with_options(const char* root_directory, + const ScannerOptions* options) { + if (!root_directory || !options) + return NULL; + ParallelScanner* ps = calloc(1, sizeof(ParallelScanner)); + if (!ps) + return NULL; + if (!parallel_scanner_init(ps)) { + free(ps); return NULL; } @@ -464,183 +663,22 @@ ParallelScanner* parallel_scanner_create_with_options(const char* root_directory if (!root_files || !subdirs) { array_list_delete(root_files); array_list_delete(subdirs); - closedir(dir); parallel_scanner_destroy(ps); return NULL; } - const struct dirent* entry; - while ((entry = readdir(dir)) != NULL) { - if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0) - continue; - ScannerEntry inspected; - int inspection = - scanner_inspect_entry(options, root_directory, root_directory, entry->d_name, &inspected); - if (inspection < 0) { - ps->failed = true; - continue; - } - if (inspection == 0) - continue; - char* cur_path = inspected.path; - struct stat st = inspected.stats; - if (inspected.is_directory) { - if (!array_list_add(subdirs, cur_path)) { - free(cur_path); - ps->failed = true; - } - } else { - File* file = file_create(cur_path); - free(cur_path); - if (!file) { - ps->failed = true; - continue; - } - file->data->size = st.st_size; - if (options->use_metadata) - file->metadata = file_metadata_create(&st); - if (options->use_metadata && !file->metadata) { - file_destroy(file); - ps->failed = true; - continue; - } - if (!array_list_add(root_files, file)) { - file_destroy(file); - ps->failed = true; - } - } + + if (!scan_root_directory(ps, root_directory, options, root_files, subdirs)) { + array_list_delete(root_files); + array_list_delete(subdirs); + parallel_scanner_destroy(ps); + return NULL; } - closedir(dir); unsigned long long cs = options->chunk_size > 0 ? options->chunk_size : DESIRED_CHUNK_SIZE; - if (root_files->size > 0) { - ArrayList* batch = array_list_create(NULL); - if (!batch) { - ps->failed = true; - array_list_delete(root_files); - array_list_delete(subdirs); - parallel_scanner_destroy(ps); - return NULL; - } - unsigned long long batch_size = 0; - Chunk* first = NULL; - for (int i = 0; i < root_files->size; i++) { - File* f = (File*)root_files->items[i]; - if (!array_list_add(batch, f)) { - ps->failed = true; - break; - } - batch_size += f->data->size; - if (batch_size >= cs || i == root_files->size - 1) { - void** items = array_list_to_array(batch); - if (!items) { - ps->failed = true; - array_list_delete(batch); - batch = NULL; - break; - } - Chunk* c = chunk_create((File**)items, batch->size); - free(items); - if (!c) { - ps->failed = true; - array_list_delete(batch); - batch = NULL; - break; - } - int batch_start = i - batch->size + 1; - for (int j = batch_start; j <= i; j++) - root_files->items[j] = NULL; - batch->item_destroyer = NULL; - array_list_delete(batch); - batch = NULL; - if (!first) { - first = c; - } else { - if (!queue_enqueue(ps->result_queue, c)) { - chunk_destroy(c); - ps->failed = true; - } - } - if (i < root_files->size - 1) { - batch = array_list_create(NULL); - if (!batch) { - ps->failed = true; - break; - } - batch_size = 0; - } - } - } - if (batch) { - batch->item_destroyer = NULL; - array_list_delete(batch); - } - ps->initial_chunk = first; - } + ps->initial_chunk = batch_files(root_files, cs, ps->result_queue, &ps->failed); array_list_delete(root_files); - int n = options->num_threads > 0 ? options->num_threads : 4; - if (n > subdirs->size) - n = subdirs->size > 0 ? subdirs->size : 1; - - if (subdirs->size > 0) { - ps->num_threads = n; - ps->expected_threads = n; - ps->threads = calloc(n, sizeof(thrd_t)); - if (!ps->threads) { - array_list_delete(subdirs); - parallel_scanner_destroy(ps); - return NULL; - } - int dirs_per_thread = subdirs->size / n; - int remainder = subdirs->size % n; - int start = 0; - ps->num_threads = 0; - for (int t = 0; t < n; t++) { - int count = dirs_per_thread + (t < remainder ? 1 : 0); - if (count == 0) - break; - ParallelWorkerArg* wa = calloc(1, sizeof(ParallelWorkerArg)); - if (!wa) { - parallel_scanner_creation_failed(ps); - break; - } - wa->ps = ps; - wa->dirs = calloc(count, sizeof(char*)); - if (!wa->dirs) { - free(wa); - parallel_scanner_creation_failed(ps); - break; - } - bool dup_ok = true; - for (int j = 0; j < count; j++) { - wa->dirs[j] = str_dup((char*)subdirs->items[start + j]); - if (!wa->dirs[j]) - dup_ok = false; - } - if (!dup_ok) { - for (int j = 0; j < count; j++) - free(wa->dirs[j]); - free(wa->dirs); - free(wa); - parallel_scanner_creation_failed(ps); - break; - } - wa->dir_count = count; - wa->options = *options; - wa->options.chunk_size = cs; - start += count; - if (thrd_create(&ps->threads[t], parallel_worker_thread, wa) != thrd_success) { - for (int j = 0; j < count; j++) - free(wa->dirs[j]); - free(wa->dirs); - free(wa); - parallel_scanner_creation_failed(ps); - break; - } - ps->num_threads++; - ps->created_threads++; - } - } + spawn_parallel_workers(ps, subdirs, options, cs); array_list_delete(subdirs); return ps; } From d639dfdc077e205a5cf8bcdca06e17ac40f1a4fd Mon Sep 17 00:00:00 2001 From: TapTap Date: Sun, 30 Aug 2026 13:59:43 +0200 Subject: [PATCH 4/7] refactor: split file.c into file_send.c and file_receive.c - file.c: File/FileMetadata lifecycle and local disk helpers (~110 lines) - file_send.c: client-side send path (file_send_single_calls, file_send_sendfile) - file_receive.c: server-side receive/save path (file_receive, receive_incremental_check, receive_manifest, file_save_to_disk) - file_types.h holds shared struct definitions; file.h remains an umbrella header so existing includes are unaffected Completes the transfer/protocol separation started in PR #212 --- src/shared/file.c | 707 +------------------------------------- src/shared/file.h | 36 +- src/shared/file_receive.c | 588 +++++++++++++++++++++++++++++++ src/shared/file_receive.h | 15 + src/shared/file_send.c | 136 ++++++++ src/shared/file_send.h | 14 + src/shared/file_types.h | 25 ++ 7 files changed, 789 insertions(+), 732 deletions(-) create mode 100644 src/shared/file_receive.c create mode 100644 src/shared/file_receive.h create mode 100644 src/shared/file_send.c create mode 100644 src/shared/file_send.h create mode 100644 src/shared/file_types.h diff --git a/src/shared/file.c b/src/shared/file.c index c8afb0e..17c65c0 100644 --- a/src/shared/file.c +++ b/src/shared/file.c @@ -1,27 +1,14 @@ -#include -#include -#include -#include -#include -#include -#include #include #include #include -#include #include #include -#include -#include "compression.h" -#include "delta.h" -#include "log.h" -#include "config.h" #include "data.h" +#include "delta.h" #include "file.h" #include "file_store.h" -#include "metadata.h" -#include "protocol.h" +#include "log.h" #include "utils.h" bool file_checksum(File* file, uint64_t* checksum) { @@ -124,664 +111,17 @@ bool file_load_data(File* file) { return true; } -bool file_send_single_calls(File* file, int file_descriptor, bool use_metadata, - int compression_level, bool send_path) { - if (!file || !file->path || !file->data) - return false; - const Data* data_to_send = file->data; - Data* compressed_data = NULL; - if (compression_level > 0 && !compression_should_skip(file->path)) { - compressed_data = data_compress(file->data, compression_level); - if (compressed_data == NULL) { - log_message(LOG_LEVEL_ERROR, "Failed to compress file data"); - return false; - } - data_to_send = compressed_data; - } - if (send_path && !send_str(file_descriptor, file->path)) { - data_destroy(compressed_data); - return false; - } - if (use_metadata && !metadata_send(file_descriptor, file->metadata)) { - data_destroy(compressed_data); - return false; - } - if (!send_data(file_descriptor, data_to_send)) { - data_destroy(compressed_data); - return false; - } - data_destroy(compressed_data); - return true; -} - bool file_set_authorized_root(int fd, const char* canonical_path) { return file_store_set_authorized_root(fd, canonical_path); } -static bool path_is_within_root(const char* root, const char* path) { - size_t n = strlen(root); - return strncmp(root, path, n) == 0 && (path[n] == '\0' || path[n] == '/'); -} - -bool file_save_to_disk(const char* root_directory, const File* file, const Config* config) { - bool backup_enabled = config && config->backup; - bool inplace = config && config->inplace; - bool sparse = config && config->preserve_sparse; - const char* backup_suffix = (config && config->suffix) ? config->suffix : "~"; - const char* backup_dir = (config && config->backup_dir) ? config->backup_dir : NULL; - const char* partial_dir = (config && config->partial_dir) ? config->partial_dir : NULL; - char *confined_backup = NULL, *confined_partial = NULL; - - if (!file || !file->path || !file->data || has_path_traversal(file->path) || - (backup_enabled && - (!backup_suffix || backup_suffix[0] == '\0' || strchr(backup_suffix, '/') != NULL || - strcmp(backup_suffix, ".") == 0 || strcmp(backup_suffix, "..") == 0))) { - log_message(LOG_LEVEL_ERROR, "Invalid file or path received"); - return false; - } - - /* These options arrive from the client. They are names below the server - root, never independent filesystem roots. */ - if ((backup_dir && (backup_dir[0] == '/' || has_path_traversal(backup_dir))) || - (partial_dir && (partial_dir[0] == '/' || has_path_traversal(partial_dir)))) - return false; - if (backup_dir && !(confined_backup = path_cat(root_directory, backup_dir))) - return false; - if (partial_dir && !(confined_partial = path_cat(root_directory, partial_dir))) { - free(confined_backup); - return false; - } - - char* resolved_root = NULL; - const char* actual_root = - (partial_dir && config && config->partial) ? confined_partial : root_directory; - resolved_root = realpath(actual_root, NULL); - if (resolved_root == NULL) { - if (mkdir_r(actual_root)) { - resolved_root = realpath(actual_root, NULL); - } - } - if (resolved_root == NULL) { - log_message(LOG_LEVEL_ERROR, "Failed to resolve destination root: %s", actual_root); - free(confined_backup); - free(confined_partial); - return false; - } - char* resolved_base = realpath(root_directory, NULL); - if (resolved_base == NULL || !path_is_within_root(resolved_base, resolved_root)) { - free(resolved_base); - free(confined_backup); - free(confined_partial); - free(resolved_root); - return false; - } - free(resolved_base); - - char* disk_path = path_cat(resolved_root, file->path); - if (disk_path == NULL) { - free(confined_backup); - free(confined_partial); - free(resolved_root); - return false; - } - - /* --update is receiver-side policy: never replace a newer destination. */ - if (config && config->update) { - struct stat destination_stat; - if (stat(disk_path, &destination_stat) == 0 && file->metadata && - destination_stat.st_mtime > file->metadata->mtime_sec) { - free(resolved_root); - free(confined_backup); - free(confined_partial); - free(disk_path); - return true; - } - } - - if (backup_enabled) { - struct stat backup_stat; - if (stat(disk_path, &backup_stat) == 0) { - char* backup_path = NULL; - if (backup_dir) { - char* resolved_backup_dir = realpath(confined_backup, NULL); - if (!resolved_backup_dir) { - mkdir_r(confined_backup); - resolved_backup_dir = realpath(confined_backup, NULL); - } - if (resolved_backup_dir) { - char* backup_base = realpath(root_directory, NULL); - if (backup_base && path_is_within_root(backup_base, resolved_backup_dir)) - backup_path = path_cat(resolved_backup_dir, file->path); - free(backup_base); - free(resolved_backup_dir); - } - } - if (!backup_path) { - size_t path_len = strlen(disk_path); - size_t suffix_len = strlen(backup_suffix); - backup_path = malloc(path_len + suffix_len + 1); - if (backup_path) { - memcpy(backup_path, disk_path, path_len); - memcpy(backup_path + path_len, backup_suffix, suffix_len + 1); - } - } - if (backup_path) { - char* backup_dir_path = str_dup(backup_path); - if (backup_dir_path) { - const char* bdir = dirname(backup_dir_path); - mkdir_r(bdir); - free(backup_dir_path); - } - if (!file_store_rename_secure(disk_path, backup_path)) { - free(backup_path); - free(resolved_root); - free(confined_backup); - free(confined_partial); - free(disk_path); - return false; - } - free(backup_path); - } - } - } - - char* dir_dup = str_dup(disk_path); - if (!dir_dup) { - free(confined_backup); - free(confined_partial); - free(resolved_root); - free(disk_path); - return false; - } - char* dir_str = dirname(dir_dup); - if (!mkdir_r(dir_str)) { - free(dir_dup); - free(confined_backup); - free(confined_partial); - free(resolved_root); - free(disk_path); - return false; - } - char* resolved_dir = realpath(dir_str, NULL); - free(dir_dup); - if (resolved_dir == NULL) { - log_message(LOG_LEVEL_ERROR, "Failed to resolve directory for: %s", disk_path); - free(confined_backup); - free(confined_partial); - free(resolved_root); - free(disk_path); - return false; - } - - size_t root_len = strlen(resolved_root); - if (strncmp(resolved_dir, resolved_root, root_len) != 0 || - (resolved_dir[root_len] != '\0' && resolved_dir[root_len] != '/')) { - log_message(LOG_LEVEL_ERROR, "Path escape detected: %s is outside %s", disk_path, actual_root); - free(resolved_dir); - free(confined_backup); - free(confined_partial); - free(resolved_root); - free(disk_path); - return false; - } - free(resolved_dir); - free(resolved_root); - - bool ok = file_store_write_secure(disk_path, file->data->data, file->data->size, inplace, sparse, - file->metadata); - free(confined_backup); - free(confined_partial); - free(disk_path); - return ok; -} - -static File* receive_delta_file(int fd, const Config* config, const char* check_path, - void* old_data, unsigned long long old_size, bool* failed) { - if (!old_data) - return NULL; - - DeltaSignature* sig = delta_signature_create(old_data, old_size, config->delta_block_size); - if (!sig) { - free(old_data); - *failed = true; - return NULL; - } - - Data* sig_data = delta_signature_serialize(sig); - if (!sig_data) { - delta_signature_destroy(sig); - free(old_data); - *failed = true; - return NULL; - } - - bool sig_sent = send_status(fd, STATUS_DELTA_SIGNATURE) && send_data(fd, sig_data); - data_destroy(sig_data); - - if (!sig_sent) { - delta_signature_destroy(sig); - free(old_data); - *failed = true; - return NULL; - } - - Status resp; - if (!receive_status(fd, &resp)) { - delta_signature_destroy(sig); - free(old_data); - *failed = true; - return NULL; - } - - if (resp == STATUS_DELTA_DATA) { - Data* delta_data = receive_data(fd); - if (!delta_data) { - delta_signature_destroy(sig); - free(old_data); - *failed = true; - return NULL; - } - - Data* raw_delta = delta_data; - if (config->use_compression) { - raw_delta = data_decompress(delta_data); - data_destroy(delta_data); - if (!raw_delta) { - free(old_data); - delta_signature_destroy(sig); - *failed = true; - return NULL; - } - } - - Delta* delta = delta_deserialize(raw_delta); - data_destroy(raw_delta); - if (!delta) { - free(old_data); - delta_signature_destroy(sig); - *failed = true; - return NULL; - } - - void* new_data = delta_apply(old_data, old_size, delta, config->delta_block_size); - uint64_t new_size = delta->new_file_size; - delta_destroy(delta); - - if (!new_data) { - free(old_data); - delta_signature_destroy(sig); - *failed = true; - return NULL; - } - - File* file = file_create(check_path); - if (!file) { - free(new_data); - free(old_data); - delta_signature_destroy(sig); - *failed = true; - return NULL; - } - - if (config->use_metadata) { - int meta_ok = 1; - file->metadata = metadata_receive(fd, &meta_ok); - if (!meta_ok) { - file_destroy(file); - free(new_data); - free(old_data); - delta_signature_destroy(sig); - *failed = true; - return NULL; - } - } - - data_destroy(file->data); - file->data = data_create(new_data, (size_t)new_size); - - free(old_data); - delta_signature_destroy(sig); - return file; - } - - if (resp == STATUS_NEXT) { - delta_signature_destroy(sig); - free(old_data); - - File* file = file_create(check_path); - if (!file) { - *failed = true; - return NULL; - } - - if (config->use_metadata) { - int meta_ok = 1; - file->metadata = metadata_receive(fd, &meta_ok); - if (!meta_ok) { - file_destroy(file); - *failed = true; - return NULL; - } - } - - Data* file_data = receive_data(fd); - if (file_data == NULL) { - file_destroy(file); - *failed = true; - return NULL; - } - - if (config->use_compression) { - Data* uncompressed = data_decompress(file_data); - data_destroy(file_data); - if (uncompressed == NULL) { - file_destroy(file); - *failed = true; - return NULL; - } - file_data = uncompressed; - } - - data_destroy(file->data); - file->data = file_data; - return file; - } - - delta_signature_destroy(sig); - free(old_data); - *failed = true; - return NULL; -} - -File* receive_incremental_check(int fd, const Config* config, bool* skipped) { - *skipped = false; - char* check_path = receive_str(fd); - if (check_path == NULL) { - return NULL; - } - - unsigned long long check_size; - long long check_mtime; - uint64_t check_checksum = 0; - if (!receive_n_data(fd, &check_size, sizeof(check_size)) || - !receive_n_data(fd, &check_mtime, sizeof(check_mtime))) { - free(check_path); - return NULL; - } - if (config->checksum && !receive_n_data(fd, &check_checksum, sizeof(check_checksum))) { - free(check_path); - return NULL; - } - - if (has_path_traversal(check_path)) { - log_message(LOG_LEVEL_ERROR, "Path traversal detected: %s", check_path); - free(check_path); - return NULL; - } - - char* full_path = path_cat(config->receive_root_directory, check_path); - struct stat st; - bool has_old_file = false; - int old_fd = -1; - if (full_path) { - char* leaf = NULL; - int parent_fd = file_store_open_secure_parent(full_path, &leaf); - if (parent_fd >= 0) { - old_fd = openat(parent_fd, leaf, O_RDONLY | O_CLOEXEC | O_NOFOLLOW); - free(leaf); - close(parent_fd); - has_old_file = old_fd >= 0 && fstat(old_fd, &st) == 0 && S_ISREG(st.st_mode); - } - } - unsigned long long old_size = has_old_file ? (unsigned long long)st.st_size : 0; - void* old_data = NULL; - if (has_old_file && old_size > 0) { - old_data = malloc((size_t)old_size); - if (old_data) { - size_t got = 0; - while (got < (size_t)old_size) { - ssize_t n = read(old_fd, (char*)old_data + got, (size_t)old_size - got); - if (n <= 0) { - free(old_data); - old_data = NULL; - break; - } - got += (size_t)n; - } - } - } - if (old_fd >= 0) { - close(old_fd); - } - - bool match = has_old_file && (unsigned long long)st.st_size == check_size; - if (match && config->checksum) { - uint64_t old_checksum = old_size == 0 ? delta_xxhash64("", 0) : 0; - if (old_data) - old_checksum = delta_xxhash64(old_data, (size_t)old_size); - match = (old_size == 0 || old_data) && old_checksum == check_checksum; - free(old_data); - old_data = NULL; - } else if (match) { - match = (long long)st.st_mtime == check_mtime; - } - - if (match) { - free(old_data); - if (!send_status(fd, STATUS_OK)) { - free(full_path); - free(check_path); - return NULL; - } - free(full_path); - free(check_path); - *skipped = true; - return NULL; - } - - bool try_delta = config->use_delta && has_old_file && - delta_should_attempt(old_size, check_size, config->delta_max_file_size); - - if (try_delta) { - bool delta_failed = false; - File* delta_file = - receive_delta_file(fd, config, check_path, old_data, old_size, &delta_failed); - old_data = NULL; /* receive_delta_file consumes the snapshot on every path */ - if (delta_file) { - free(full_path); - free(check_path); - return delta_file; - } - if (delta_failed) { - free(full_path); - free(check_path); - return NULL; - } - free(old_data); - old_data = NULL; - try_delta = false; - } - - if (!try_delta) { - if (!send_status(fd, STATUS_NEXT)) { - free(full_path); - free(check_path); - return NULL; - } - } - - File* file = file_create(check_path); - free(check_path); - free(full_path); - if (file == NULL) { - return NULL; - } - - if (config->use_metadata) { - int meta_ok = 1; - file->metadata = metadata_receive(fd, &meta_ok); - if (!meta_ok) { - file_destroy(file); - return NULL; - } - } - - Data* file_data = receive_data(fd); - if (file_data == NULL) { - file_destroy(file); - return NULL; - } - - if (config->use_compression) { - Data* uncompressed = data_decompress(file_data); - data_destroy(file_data); - if (uncompressed == NULL) { - file_destroy(file); - return NULL; - } - file_data = uncompressed; - } - - data_destroy(file->data); - file->data = file_data; - return file; -} - -bool file_write_to_disk(const char* path, const void* data, unsigned long long data_size, bool inplace, - bool sparse) { +bool file_write_to_disk(const char* path, const void* data, unsigned long long data_size, + bool inplace, bool sparse) { if (!path || (!data && data_size != 0) || has_path_traversal(path)) return false; return file_store_write_secure(path, data, data_size, inplace, sparse, NULL); } -bool file_send_sendfile(File* file, int file_descriptor, bool use_metadata, int compression_level, - bool send_path) { - if (!file || !file->path || !file->data) - return false; - if (compression_level > 0) - return file_send_single_calls(file, file_descriptor, use_metadata, compression_level, - send_path); - - if (send_path && !send_str(file_descriptor, file->path)) - return false; - if (use_metadata && !metadata_send(file_descriptor, file->metadata)) - return false; - - int fd = open(file->path, O_RDONLY); - if (fd == -1) { - perror("Could not open file for sendfile"); - return false; - } - - unsigned long long file_size = file->data->size; - struct stat source_stat; - if (fstat(fd, &source_stat) != 0 || !S_ISREG(source_stat.st_mode) || - (unsigned long long)source_stat.st_size < file_size) { - close(fd); - return false; - } - if (!send_n_data(file_descriptor, &file_size, sizeof(unsigned long long))) { - close(fd); - return false; - } - - /* sendfile cannot encrypt TLS records. Keep the framing identical but - route encrypted transfers through the deadline-aware IO layer. */ - if (io_get_ssl() != NULL) { - unsigned char buffer[64 * 1024]; - unsigned long long remaining = file_size; - bool ok = true; - while (remaining > 0) { - size_t want = remaining > sizeof(buffer) ? sizeof(buffer) : (size_t)remaining; - ssize_t got = read(fd, buffer, want); - if (got <= 0 || !send_n_data(file_descriptor, buffer, (size_t)got)) { - ok = false; - break; - } - remaining -= (unsigned long long)got; - } - close(fd); - return ok; - } - - off_t offset = 0; - struct timespec deadline; - clock_gettime(CLOCK_MONOTONIC, &deadline); - deadline.tv_sec += 60; - while ((unsigned long long)offset < file_size) { - struct timespec now; - clock_gettime(CLOCK_MONOTONIC, &now); - long long remaining = (long long)(deadline.tv_sec - now.tv_sec) * 1000LL + - (deadline.tv_nsec - now.tv_nsec) / 1000000LL; - if (remaining <= 0) { - close(fd); - return false; - } - struct pollfd pfd = {.fd = file_descriptor, .events = POLLOUT}; - int timeout = remaining > INT_MAX ? INT_MAX : (int)remaining; - int polled = poll(&pfd, 1, timeout); - if (polled <= 0 || (pfd.revents & (POLLERR | POLLHUP | POLLNVAL))) { - close(fd); - return false; - } - ssize_t sent = sendfile(file_descriptor, fd, &offset, file_size - offset); - if (sent == -1) { - if (errno == EAGAIN || errno == EINTR) - continue; - perror("sendfile failed"); - close(fd); - return false; - } - if (sent == 0) { - close(fd); - return false; - } - } - - close(fd); - return true; -} - -File* file_receive(const Config* config, int file_descriptor) { - char* path = receive_str(file_descriptor); - if (path == NULL) - return NULL; - if (path[0] == '\0' || has_path_traversal(path)) { - log_message(LOG_LEVEL_ERROR, "Invalid received file path: %s", path); - free(path); - return NULL; - } - File* file = file_create(path); - free(path); - if (file == NULL) - return NULL; - if (config->use_metadata) { - int meta_ok = 1; - file->metadata = metadata_receive(file_descriptor, &meta_ok); - if (!meta_ok) { - file_destroy(file); - return NULL; - } - } - Data* file_data = receive_data(file_descriptor); - if (file_data == NULL) { - file_destroy(file); - return NULL; - } - if (config->use_compression) { - Data* file_data_uncompressed = data_decompress(file_data); - data_destroy(file_data); - if (file_data_uncompressed == NULL) { - file_destroy(file); - return NULL; - } - file_data = file_data_uncompressed; - } - data_destroy(file->data); - file->data = file_data; - return file; -} - size_t file_content_to_buffer(File* file) { FILE* file_pointer = fopen(file->path, "rb"); if (file_pointer == NULL) { @@ -797,42 +137,3 @@ size_t file_content_to_buffer(File* file) { fclose(file_pointer); return bytes_read; } - -int receive_manifest(int fd, const Config* config, int* next_status) { - int received_status = STATUS_ERROR; - int* status_out = next_status ? next_status : &received_status; - int count; - if (!receive_int(fd, &count)) - return -1; - if (count < 0 || count > MAX_MANIFEST_ENTRIES) - return -1; - ArrayList* manifest = array_list_create(free); - if (!manifest) - return -1; - size_t manifest_bytes = 0; - for (int i = 0; i < count; i++) { - char* s = receive_str(fd); - size_t entry_size = s ? strlen(s) : 0; - if (!s || s[0] == '\0' || s[0] == '/' || has_path_traversal(s) || - entry_size > MAX_MANIFEST_BYTES - manifest_bytes || - (manifest_bytes += entry_size) > MAX_MANIFEST_BYTES || !array_list_add(manifest, s)) { - free(s); - array_list_delete(manifest); - return -1; - } - } - if (!receive_status(fd, status_out)) { - array_list_delete(manifest); - return -1; - } - /* Deletion is a commit operation: never perform it until the sender has - completed the manifest frame successfully. */ - if (*status_out != STATUS_FINISHED || !config->use_delete) { - array_list_delete(manifest); - return *status_out == STATUS_FINISHED ? 0 : -1; - } - fprintf(stderr, "Deleting files not in manifest...\n"); - bool deletion_ok = delete_extras(config->receive_root_directory, manifest); - array_list_delete(manifest); - return deletion_ok ? 0 : -1; -} diff --git a/src/shared/file.h b/src/shared/file.h index 3c388c2..7f72116 100644 --- a/src/shared/file.h +++ b/src/shared/file.h @@ -1,45 +1,23 @@ #ifndef FILE_H #define FILE_H -#include "config.h" -#include "data.h" +#include "file_send.h" +#include "file_receive.h" +#include "file_types.h" #include -#include +#include -typedef enum { FILE_TYPE_REGULAR, FILE_TYPE_SYMLINK, FILE_TYPE_DIR } FileType; - -typedef struct { - mode_t mode; - uid_t uid; - gid_t gid; - time_t mtime_sec; - long mtime_nsec; -} FileMetadata; - -typedef struct { - char* path; - Data* data; - FileMetadata* metadata; - bool skip; -} File; +/* File/FileMetadata lifecycle and local disk helpers. */ File* file_create(const char* path); void file_destroy(void* item); bool file_load_data(File* file); bool file_checksum(File* file, uint64_t* checksum); -File* file_receive(const Config* config, int file_descriptor); -bool file_send_single_calls(File* file, int file_descriptor, bool use_metadata, - int compression_level, bool send_path); -bool file_send_sendfile(File* file, int file_descriptor, bool use_metadata, int compression_level, - bool send_path); size_t file_content_to_buffer(File* file); FileMetadata* file_metadata_create(const struct stat* stats); void file_metadata_destroy(void* metadata); -bool file_write_to_disk(const char* path, const void* data, unsigned long long data_size, bool inplace, - bool sparse); -bool file_save_to_disk(const char* root_directory, const File* file, const Config* config); +bool file_write_to_disk(const char* path, const void* data, unsigned long long data_size, + bool inplace, bool sparse); bool file_set_authorized_root(int fd, const char* canonical_path); -File* receive_incremental_check(int fd, const Config* config, bool* skipped); -int receive_manifest(int fd, const Config* config, int* next_status); #endif diff --git a/src/shared/file_receive.c b/src/shared/file_receive.c new file mode 100644 index 0000000..f809b7c --- /dev/null +++ b/src/shared/file_receive.c @@ -0,0 +1,588 @@ +#include +#include +#include +#include +#include +#include +#include +#include + +#include "array_list.h" +#include "compression.h" +#include "config.h" +#include "data.h" +#include "delta.h" +#include "file.h" +#include "file_store.h" +#include "log.h" +#include "metadata.h" +#include "protocol.h" +#include "utils.h" + +static bool path_is_within_root(const char* root, const char* path) { + size_t n = strlen(root); + return strncmp(root, path, n) == 0 && (path[n] == '\0' || path[n] == '/'); +} + +bool file_save_to_disk(const char* root_directory, const File* file, const Config* config) { + bool backup_enabled = config && config->backup; + bool inplace = config && config->inplace; + bool sparse = config && config->preserve_sparse; + const char* backup_suffix = (config && config->suffix) ? config->suffix : "~"; + const char* backup_dir = (config && config->backup_dir) ? config->backup_dir : NULL; + const char* partial_dir = (config && config->partial_dir) ? config->partial_dir : NULL; + char *confined_backup = NULL, *confined_partial = NULL; + + if (!file || !file->path || !file->data || has_path_traversal(file->path) || + (backup_enabled && + (!backup_suffix || backup_suffix[0] == '\0' || strchr(backup_suffix, '/') != NULL || + strcmp(backup_suffix, ".") == 0 || strcmp(backup_suffix, "..") == 0))) { + log_message(LOG_LEVEL_ERROR, "Invalid file or path received"); + return false; + } + + /* These options arrive from the client. They are names below the server + root, never independent filesystem roots. */ + if ((backup_dir && (backup_dir[0] == '/' || has_path_traversal(backup_dir))) || + (partial_dir && (partial_dir[0] == '/' || has_path_traversal(partial_dir)))) + return false; + if (backup_dir && !(confined_backup = path_cat(root_directory, backup_dir))) + return false; + if (partial_dir && !(confined_partial = path_cat(root_directory, partial_dir))) { + free(confined_backup); + return false; + } + + char* resolved_root = NULL; + const char* actual_root = + (partial_dir && config && config->partial) ? confined_partial : root_directory; + resolved_root = realpath(actual_root, NULL); + if (resolved_root == NULL) { + if (mkdir_r(actual_root)) { + resolved_root = realpath(actual_root, NULL); + } + } + if (resolved_root == NULL) { + log_message(LOG_LEVEL_ERROR, "Failed to resolve destination root: %s", actual_root); + free(confined_backup); + free(confined_partial); + return false; + } + char* resolved_base = realpath(root_directory, NULL); + if (resolved_base == NULL || !path_is_within_root(resolved_base, resolved_root)) { + free(resolved_base); + free(confined_backup); + free(confined_partial); + free(resolved_root); + return false; + } + free(resolved_base); + + char* disk_path = path_cat(resolved_root, file->path); + if (disk_path == NULL) { + free(confined_backup); + free(confined_partial); + free(resolved_root); + return false; + } + + /* --update is receiver-side policy: never replace a newer destination. */ + if (config && config->update) { + struct stat destination_stat; + if (stat(disk_path, &destination_stat) == 0 && file->metadata && + destination_stat.st_mtime > file->metadata->mtime_sec) { + free(resolved_root); + free(confined_backup); + free(confined_partial); + free(disk_path); + return true; + } + } + + if (backup_enabled) { + struct stat backup_stat; + if (stat(disk_path, &backup_stat) == 0) { + char* backup_path = NULL; + if (backup_dir) { + char* resolved_backup_dir = realpath(confined_backup, NULL); + if (!resolved_backup_dir) { + mkdir_r(confined_backup); + resolved_backup_dir = realpath(confined_backup, NULL); + } + if (resolved_backup_dir) { + char* backup_base = realpath(root_directory, NULL); + if (backup_base && path_is_within_root(backup_base, resolved_backup_dir)) + backup_path = path_cat(resolved_backup_dir, file->path); + free(backup_base); + free(resolved_backup_dir); + } + } + if (!backup_path) { + size_t path_len = strlen(disk_path); + size_t suffix_len = strlen(backup_suffix); + backup_path = malloc(path_len + suffix_len + 1); + if (backup_path) { + memcpy(backup_path, disk_path, path_len); + memcpy(backup_path + path_len, backup_suffix, suffix_len + 1); + } + } + if (backup_path) { + char* backup_dir_path = str_dup(backup_path); + if (backup_dir_path) { + const char* bdir = dirname(backup_dir_path); + mkdir_r(bdir); + free(backup_dir_path); + } + if (!file_store_rename_secure(disk_path, backup_path)) { + free(backup_path); + free(resolved_root); + free(confined_backup); + free(confined_partial); + free(disk_path); + return false; + } + free(backup_path); + } + } + } + + char* dir_dup = str_dup(disk_path); + if (!dir_dup) { + free(confined_backup); + free(confined_partial); + free(resolved_root); + free(disk_path); + return false; + } + char* dir_str = dirname(dir_dup); + if (!mkdir_r(dir_str)) { + free(dir_dup); + free(confined_backup); + free(confined_partial); + free(resolved_root); + free(disk_path); + return false; + } + char* resolved_dir = realpath(dir_str, NULL); + free(dir_dup); + if (resolved_dir == NULL) { + log_message(LOG_LEVEL_ERROR, "Failed to resolve directory for: %s", disk_path); + free(confined_backup); + free(confined_partial); + free(resolved_root); + free(disk_path); + return false; + } + + size_t root_len = strlen(resolved_root); + if (strncmp(resolved_dir, resolved_root, root_len) != 0 || + (resolved_dir[root_len] != '\0' && resolved_dir[root_len] != '/')) { + log_message(LOG_LEVEL_ERROR, "Path escape detected: %s is outside %s", disk_path, actual_root); + free(resolved_dir); + free(confined_backup); + free(confined_partial); + free(resolved_root); + free(disk_path); + return false; + } + free(resolved_dir); + free(resolved_root); + + bool ok = file_store_write_secure(disk_path, file->data->data, file->data->size, inplace, sparse, + file->metadata); + free(confined_backup); + free(confined_partial); + free(disk_path); + return ok; +} + +static File* receive_delta_file(int fd, const Config* config, const char* check_path, + void* old_data, unsigned long long old_size, bool* failed) { + if (!old_data) + return NULL; + + DeltaSignature* sig = delta_signature_create(old_data, old_size, config->delta_block_size); + if (!sig) { + free(old_data); + *failed = true; + return NULL; + } + + Data* sig_data = delta_signature_serialize(sig); + if (!sig_data) { + delta_signature_destroy(sig); + free(old_data); + *failed = true; + return NULL; + } + + bool sig_sent = send_status(fd, STATUS_DELTA_SIGNATURE) && send_data(fd, sig_data); + data_destroy(sig_data); + + if (!sig_sent) { + delta_signature_destroy(sig); + free(old_data); + *failed = true; + return NULL; + } + + Status resp; + if (!receive_status(fd, &resp)) { + delta_signature_destroy(sig); + free(old_data); + *failed = true; + return NULL; + } + + if (resp == STATUS_DELTA_DATA) { + Data* delta_data = receive_data(fd); + if (!delta_data) { + delta_signature_destroy(sig); + free(old_data); + *failed = true; + return NULL; + } + + Data* raw_delta = delta_data; + if (config->use_compression) { + raw_delta = data_decompress(delta_data); + data_destroy(delta_data); + if (!raw_delta) { + free(old_data); + delta_signature_destroy(sig); + *failed = true; + return NULL; + } + } + + Delta* delta = delta_deserialize(raw_delta); + data_destroy(raw_delta); + if (!delta) { + free(old_data); + delta_signature_destroy(sig); + *failed = true; + return NULL; + } + + void* new_data = delta_apply(old_data, old_size, delta, config->delta_block_size); + uint64_t new_size = delta->new_file_size; + delta_destroy(delta); + + if (!new_data) { + free(old_data); + delta_signature_destroy(sig); + *failed = true; + return NULL; + } + + File* file = file_create(check_path); + if (!file) { + free(new_data); + free(old_data); + delta_signature_destroy(sig); + *failed = true; + return NULL; + } + + if (config->use_metadata) { + int meta_ok = 1; + file->metadata = metadata_receive(fd, &meta_ok); + if (!meta_ok) { + file_destroy(file); + free(new_data); + free(old_data); + delta_signature_destroy(sig); + *failed = true; + return NULL; + } + } + + data_destroy(file->data); + file->data = data_create(new_data, (size_t)new_size); + + free(old_data); + delta_signature_destroy(sig); + return file; + } + + if (resp == STATUS_NEXT) { + delta_signature_destroy(sig); + free(old_data); + + File* file = file_create(check_path); + if (!file) { + *failed = true; + return NULL; + } + + if (config->use_metadata) { + int meta_ok = 1; + file->metadata = metadata_receive(fd, &meta_ok); + if (!meta_ok) { + file_destroy(file); + *failed = true; + return NULL; + } + } + + Data* file_data = receive_data(fd); + if (file_data == NULL) { + file_destroy(file); + *failed = true; + return NULL; + } + + if (config->use_compression) { + Data* uncompressed = data_decompress(file_data); + data_destroy(file_data); + if (uncompressed == NULL) { + file_destroy(file); + *failed = true; + return NULL; + } + file_data = uncompressed; + } + + data_destroy(file->data); + file->data = file_data; + return file; + } + + delta_signature_destroy(sig); + free(old_data); + *failed = true; + return NULL; +} + +File* receive_incremental_check(int fd, const Config* config, bool* skipped) { + *skipped = false; + char* check_path = receive_str(fd); + if (check_path == NULL) { + return NULL; + } + + unsigned long long check_size; + long long check_mtime; + uint64_t check_checksum = 0; + if (!receive_n_data(fd, &check_size, sizeof(check_size)) || + !receive_n_data(fd, &check_mtime, sizeof(check_mtime))) { + free(check_path); + return NULL; + } + if (config->checksum && !receive_n_data(fd, &check_checksum, sizeof(check_checksum))) { + free(check_path); + return NULL; + } + + if (has_path_traversal(check_path)) { + log_message(LOG_LEVEL_ERROR, "Path traversal detected: %s", check_path); + free(check_path); + return NULL; + } + + char* full_path = path_cat(config->receive_root_directory, check_path); + struct stat st; + bool has_old_file = false; + int old_fd = -1; + if (full_path) { + char* leaf = NULL; + int parent_fd = file_store_open_secure_parent(full_path, &leaf); + if (parent_fd >= 0) { + old_fd = openat(parent_fd, leaf, O_RDONLY | O_CLOEXEC | O_NOFOLLOW); + free(leaf); + close(parent_fd); + has_old_file = old_fd >= 0 && fstat(old_fd, &st) == 0 && S_ISREG(st.st_mode); + } + } + unsigned long long old_size = has_old_file ? (unsigned long long)st.st_size : 0; + void* old_data = NULL; + if (has_old_file && old_size > 0) { + old_data = malloc((size_t)old_size); + if (old_data) { + size_t got = 0; + while (got < (size_t)old_size) { + ssize_t n = read(old_fd, (char*)old_data + got, (size_t)old_size - got); + if (n <= 0) { + free(old_data); + old_data = NULL; + break; + } + got += (size_t)n; + } + } + } + if (old_fd >= 0) { + close(old_fd); + } + + bool match = has_old_file && (unsigned long long)st.st_size == check_size; + if (match && config->checksum) { + uint64_t old_checksum = old_size == 0 ? delta_xxhash64("", 0) : 0; + if (old_data) + old_checksum = delta_xxhash64(old_data, (size_t)old_size); + match = (old_size == 0 || old_data) && old_checksum == check_checksum; + free(old_data); + old_data = NULL; + } else if (match) { + match = (long long)st.st_mtime == check_mtime; + } + + if (match) { + free(old_data); + if (!send_status(fd, STATUS_OK)) { + free(full_path); + free(check_path); + return NULL; + } + free(full_path); + free(check_path); + *skipped = true; + return NULL; + } + + bool try_delta = config->use_delta && has_old_file && + delta_should_attempt(old_size, check_size, config->delta_max_file_size); + + if (try_delta) { + bool delta_failed = false; + File* delta_file = + receive_delta_file(fd, config, check_path, old_data, old_size, &delta_failed); + old_data = NULL; /* receive_delta_file consumes the snapshot on every path */ + if (delta_file) { + free(full_path); + free(check_path); + return delta_file; + } + if (delta_failed) { + free(full_path); + free(check_path); + return NULL; + } + free(old_data); + old_data = NULL; + try_delta = false; + } + + if (!try_delta) { + if (!send_status(fd, STATUS_NEXT)) { + free(full_path); + free(check_path); + return NULL; + } + } + + File* file = file_create(check_path); + free(check_path); + free(full_path); + if (file == NULL) { + return NULL; + } + + if (config->use_metadata) { + int meta_ok = 1; + file->metadata = metadata_receive(fd, &meta_ok); + if (!meta_ok) { + file_destroy(file); + return NULL; + } + } + + Data* file_data = receive_data(fd); + if (file_data == NULL) { + file_destroy(file); + return NULL; + } + + if (config->use_compression) { + Data* uncompressed = data_decompress(file_data); + data_destroy(file_data); + if (uncompressed == NULL) { + file_destroy(file); + return NULL; + } + file_data = uncompressed; + } + + data_destroy(file->data); + file->data = file_data; + return file; +} + +File* file_receive(const Config* config, int file_descriptor) { + char* path = receive_str(file_descriptor); + if (path == NULL) + return NULL; + if (path[0] == '\0' || has_path_traversal(path)) { + log_message(LOG_LEVEL_ERROR, "Invalid received file path: %s", path); + free(path); + return NULL; + } + File* file = file_create(path); + free(path); + if (file == NULL) + return NULL; + if (config->use_metadata) { + int meta_ok = 1; + file->metadata = metadata_receive(file_descriptor, &meta_ok); + if (!meta_ok) { + file_destroy(file); + return NULL; + } + } + Data* file_data = receive_data(file_descriptor); + if (file_data == NULL) { + file_destroy(file); + return NULL; + } + if (config->use_compression) { + Data* file_data_uncompressed = data_decompress(file_data); + data_destroy(file_data); + if (file_data_uncompressed == NULL) { + file_destroy(file); + return NULL; + } + file_data = file_data_uncompressed; + } + data_destroy(file->data); + file->data = file_data; + return file; +} + +int receive_manifest(int fd, const Config* config, int* next_status) { + int received_status = STATUS_ERROR; + int* status_out = next_status ? next_status : &received_status; + int count; + if (!receive_int(fd, &count)) + return -1; + if (count < 0 || count > MAX_MANIFEST_ENTRIES) + return -1; + ArrayList* manifest = array_list_create(free); + if (!manifest) + return -1; + size_t manifest_bytes = 0; + for (int i = 0; i < count; i++) { + char* s = receive_str(fd); + size_t entry_size = s ? strlen(s) : 0; + if (!s || s[0] == '\0' || s[0] == '/' || has_path_traversal(s) || + entry_size > MAX_MANIFEST_BYTES - manifest_bytes || + (manifest_bytes += entry_size) > MAX_MANIFEST_BYTES || !array_list_add(manifest, s)) { + free(s); + array_list_delete(manifest); + return -1; + } + } + if (!receive_status(fd, status_out)) { + array_list_delete(manifest); + return -1; + } + /* Deletion is a commit operation: never perform it until the sender has + completed the manifest frame successfully. */ + if (*status_out != STATUS_FINISHED || !config->use_delete) { + array_list_delete(manifest); + return *status_out == STATUS_FINISHED ? 0 : -1; + } + fprintf(stderr, "Deleting files not in manifest...\n"); + bool deletion_ok = delete_extras(config->receive_root_directory, manifest); + array_list_delete(manifest); + return deletion_ok ? 0 : -1; +} diff --git a/src/shared/file_receive.h b/src/shared/file_receive.h new file mode 100644 index 0000000..5b2d2a4 --- /dev/null +++ b/src/shared/file_receive.h @@ -0,0 +1,15 @@ +#ifndef FILE_RECEIVE_H +#define FILE_RECEIVE_H + +#include "config.h" +#include "file_types.h" +#include + +/* Server-side file receive/save path. */ + +File* file_receive(const Config* config, int file_descriptor); +File* receive_incremental_check(int fd, const Config* config, bool* skipped); +int receive_manifest(int fd, const Config* config, int* next_status); +bool file_save_to_disk(const char* root_directory, const File* file, const Config* config); + +#endif diff --git a/src/shared/file_send.c b/src/shared/file_send.c new file mode 100644 index 0000000..61e1762 --- /dev/null +++ b/src/shared/file_send.c @@ -0,0 +1,136 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "compression.h" +#include "data.h" +#include "file.h" +#include "log.h" +#include "metadata.h" +#include "protocol.h" + +bool file_send_single_calls(File* file, int file_descriptor, bool use_metadata, + int compression_level, bool send_path) { + if (!file || !file->path || !file->data) + return false; + const Data* data_to_send = file->data; + Data* compressed_data = NULL; + if (compression_level > 0 && !compression_should_skip(file->path)) { + compressed_data = data_compress(file->data, compression_level); + if (compressed_data == NULL) { + log_message(LOG_LEVEL_ERROR, "Failed to compress file data"); + return false; + } + data_to_send = compressed_data; + } + if (send_path && !send_str(file_descriptor, file->path)) { + data_destroy(compressed_data); + return false; + } + if (use_metadata && !metadata_send(file_descriptor, file->metadata)) { + data_destroy(compressed_data); + return false; + } + if (!send_data(file_descriptor, data_to_send)) { + data_destroy(compressed_data); + return false; + } + data_destroy(compressed_data); + return true; +} + +bool file_send_sendfile(File* file, int file_descriptor, bool use_metadata, int compression_level, + bool send_path) { + if (!file || !file->path || !file->data) + return false; + if (compression_level > 0) + return file_send_single_calls(file, file_descriptor, use_metadata, compression_level, + send_path); + + if (send_path && !send_str(file_descriptor, file->path)) + return false; + if (use_metadata && !metadata_send(file_descriptor, file->metadata)) + return false; + + int fd = open(file->path, O_RDONLY); + if (fd == -1) { + perror("Could not open file for sendfile"); + return false; + } + + unsigned long long file_size = file->data->size; + struct stat source_stat; + if (fstat(fd, &source_stat) != 0 || !S_ISREG(source_stat.st_mode) || + (unsigned long long)source_stat.st_size < file_size) { + close(fd); + return false; + } + if (!send_n_data(file_descriptor, &file_size, sizeof(unsigned long long))) { + close(fd); + return false; + } + + /* sendfile cannot encrypt TLS records. Keep the framing identical but + route encrypted transfers through the deadline-aware IO layer. */ + if (io_get_ssl() != NULL) { + unsigned char buffer[64 * 1024]; + unsigned long long remaining = file_size; + bool ok = true; + while (remaining > 0) { + size_t want = remaining > sizeof(buffer) ? sizeof(buffer) : (size_t)remaining; + ssize_t got = read(fd, buffer, want); + if (got <= 0 || !send_n_data(file_descriptor, buffer, (size_t)got)) { + ok = false; + break; + } + remaining -= (unsigned long long)got; + } + close(fd); + return ok; + } + + off_t offset = 0; + struct timespec deadline; + clock_gettime(CLOCK_MONOTONIC, &deadline); + deadline.tv_sec += 60; + while ((unsigned long long)offset < file_size) { + struct timespec now; + clock_gettime(CLOCK_MONOTONIC, &now); + long long remaining = (long long)(deadline.tv_sec - now.tv_sec) * 1000LL + + (deadline.tv_nsec - now.tv_nsec) / 1000000LL; + if (remaining <= 0) { + close(fd); + return false; + } + struct pollfd pfd = {.fd = file_descriptor, .events = POLLOUT}; + int timeout = remaining > INT_MAX ? INT_MAX : (int)remaining; + int polled = poll(&pfd, 1, timeout); + if (polled <= 0 || (pfd.revents & (POLLERR | POLLHUP | POLLNVAL))) { + close(fd); + return false; + } + ssize_t sent = sendfile(file_descriptor, fd, &offset, file_size - offset); + if (sent == -1) { + if (errno == EAGAIN || errno == EINTR) + continue; + perror("sendfile failed"); + close(fd); + return false; + } + if (sent == 0) { + close(fd); + return false; + } + } + + close(fd); + return true; +} diff --git a/src/shared/file_send.h b/src/shared/file_send.h new file mode 100644 index 0000000..86c17ad --- /dev/null +++ b/src/shared/file_send.h @@ -0,0 +1,14 @@ +#ifndef FILE_SEND_H +#define FILE_SEND_H + +#include "file_types.h" +#include + +/* Client-side file send path. */ + +bool file_send_single_calls(File* file, int file_descriptor, bool use_metadata, + int compression_level, bool send_path); +bool file_send_sendfile(File* file, int file_descriptor, bool use_metadata, int compression_level, + bool send_path); + +#endif diff --git a/src/shared/file_types.h b/src/shared/file_types.h new file mode 100644 index 0000000..a791fe9 --- /dev/null +++ b/src/shared/file_types.h @@ -0,0 +1,25 @@ +#ifndef FILE_TYPES_H +#define FILE_TYPES_H + +#include "data.h" +#include +#include + +typedef enum { FILE_TYPE_REGULAR, FILE_TYPE_SYMLINK, FILE_TYPE_DIR } FileType; + +typedef struct { + mode_t mode; + uid_t uid; + gid_t gid; + time_t mtime_sec; + long mtime_nsec; +} FileMetadata; + +typedef struct { + char* path; + Data* data; + FileMetadata* metadata; + bool skip; +} File; + +#endif From ae95211a050a0a0991448a11392fc6d173681885 Mon Sep 17 00:00:00 2001 From: TapTap Date: Sun, 30 Aug 2026 14:02:12 +0200 Subject: [PATCH 5/7] refactor: unify send_files cleanup and share progress printing - Extract print_transfer_progress() shared by single-threaded loop and the multithreaded progress thread - Route all send_files() exits through a single send_fail cleanup path - Fix pre-existing manifest leak on success without --delete --- src/client/client_send.c | 81 +++++++++++++++++++--------------------- 1 file changed, 39 insertions(+), 42 deletions(-) diff --git a/src/client/client_send.c b/src/client/client_send.c index e883cf7..900a150 100644 --- a/src/client/client_send.c +++ b/src/client/client_send.c @@ -537,6 +537,17 @@ static int load_files_multithreaded(void* pipeline_context) { } } +/* Print a one-line transfer progress report to stderr. `suffix` ends the + line (e.g. "Done.\n") or is "" for in-place refresh. Shared by the + single-threaded loop and the multithreaded progress thread. */ +static void print_transfer_progress(unsigned long long total_bytes, time_t start, + const char* suffix) { + double elapsed = difftime(time(NULL), start); + double rate = elapsed > 0.0 ? total_bytes / (1048576.0 * elapsed) : 0.0; + fprintf(stderr, "\rSent %.1f MB (%.1f MB/s) %s", total_bytes / 1048576.0, rate, suffix); + fflush(stderr); +} + /* Progress-reporting thread for multithreaded send. Runs in parallel with the scanner/loader/sender threads and prints periodic progress to stderr. */ static int progress_thread_fn(void* arg) { @@ -551,20 +562,14 @@ static int progress_thread_fn(void* arg) { mtx_unlock(&context->mutex_progress); if (done) { - time_t now = time(NULL); - double elapsed = difftime(now, start); - double rate = elapsed > 0.0 ? total / (1048576.0 * elapsed) : 0.0; - fprintf(stderr, "\rSent %.1f MB (%.1f MB/s) Done.\n", total / 1048576.0, rate); + print_transfer_progress(total, start, "Done.\n"); break; } time_t now = time(NULL); if (now - last_progress >= 1) { last_progress = now; - double elapsed = difftime(now, start); - double rate = elapsed > 0.0 ? total / (1048576.0 * elapsed) : 0.0; - fprintf(stderr, "\rSent %.1f MB (%.1f MB/s) ", total / 1048576.0, rate); - fflush(stderr); + print_transfer_progress(total, start, ""); } struct timespec ts = {0, 100 * 1000000L}; /* 100 ms */ @@ -587,29 +592,21 @@ int send_files(Config* config) { protocol_session_init(&session, client->file_descriptor, client->file_descriptor); protocol_session_set_ssl(&session, (SSL*)client->ssl); protocol_session_bind(&session); - if (!config_send(client->file_descriptor, config)) { - disconnect_transfer_client(client); - protocol_session_unbind(); - return 1; - } + int ret = 1; + DirectoryScanner* scanner = NULL; + ArrayList* manifest = NULL; + if (!config_send(client->file_descriptor, config)) + goto send_fail; ScannerOptions scanner_options = scanner_options_from_config(config, 0); - DirectoryScanner* scanner = - directory_scanner_create_with_options(config->send_directory, &scanner_options); + scanner = directory_scanner_create_with_options(config->send_directory, &scanner_options); + manifest = create_transfer_manifest(config); + if (!scanner || (config->use_delete && !manifest)) + goto send_fail; Chunk* current_chunk; unsigned long long total_bytes = 0; int total_files = 0; time_t last_progress = 0; time_t start = time(NULL); - ArrayList* manifest = create_transfer_manifest(config); - if (!scanner || (config->use_delete && !manifest)) { - if (scanner) - directory_scanner_destroy(scanner); - if (manifest) - array_list_delete(manifest); - disconnect_transfer_client(client); - protocol_session_unbind(); - return 1; - } while ((current_chunk = directory_scanner_next(scanner)) != NULL) { unsigned long long chunk_bytes = 0; for (int i = 0; i < current_chunk->element_count; i++) { @@ -621,16 +618,21 @@ int send_files(Config* config) { goto send_fail; } if (!config->use_sendfile) { + bool load_ok = true; for (int i = 0; i < current_chunk->element_count; i++) { File* f = current_chunk->items[i]; if (f->data->size > STREAM_THRESHOLD) continue; if (!file_load_data(f)) { log_message(LOG_LEVEL_ERROR, "Failed to load file data"); - chunk_destroy(current_chunk); - goto send_fail; + load_ok = false; + break; } } + if (!load_ok) { + chunk_destroy(current_chunk); + goto send_fail; + } } if (send_chunk(client, current_chunk, config) != 0) { log_message(LOG_LEVEL_ERROR, "Failed to send chunk"); @@ -645,10 +647,7 @@ int send_files(Config* config) { time_t now = time(NULL); if (now - last_progress >= 1) { last_progress = now; - double elapsed = difftime(now, start); - double rate = elapsed > 0 ? total_bytes / (1048576.0 * elapsed) : 0; - fprintf(stderr, "\rSent %.1f MB (%.1f MB/s) ", total_bytes / 1048576.0, rate); - fflush(stderr); + print_transfer_progress(total_bytes, start, ""); } } chunk_destroy(current_chunk); @@ -665,28 +664,26 @@ int send_files(Config* config) { manifest = NULL; } bool ok = finalize_transfer(client); - double elapsed_total = difftime(time(NULL), start); - if (config->show_progress) { - double rate = elapsed_total > 0 ? total_bytes / (1048576.0 * elapsed_total) : 0; - fprintf(stderr, "\rSent %.1f MB (%.1f MB/s) Done.\n", total_bytes / 1048576.0, rate); - } + if (config->show_progress) + print_transfer_progress(total_bytes, start, "Done.\n"); if (config->stats) { + double elapsed_total = difftime(time(NULL), start); double rate = elapsed_total > 0 ? total_bytes / (1048576.0 * elapsed_total) : 0; fprintf(stderr, "Stats: %d files, %.1f MB, %.1f MB/s\n", total_files, total_bytes / 1048576.0, rate); } - directory_scanner_destroy(scanner); - disconnect_transfer_client(client); - protocol_session_unbind(); - return ok ? 0 : 1; + ret = ok ? 0 : 1; send_fail: + /* Single cleanup path for all exits. The manifest is intentionally deleted + here even on success without --delete, fixing a pre-existing leak. */ if (manifest) array_list_delete(manifest); - directory_scanner_destroy(scanner); + if (scanner) + directory_scanner_destroy(scanner); disconnect_transfer_client(client); protocol_session_unbind(); - return 1; + return ret; } int send_files_multithreaded(Config* config) { From 6d82967c688365bd5c916533afa206966cc45c57 Mon Sep 17 00:00:00 2001 From: TapTap Date: Sun, 30 Aug 2026 14:06:48 +0200 Subject: [PATCH 6/7] refactor: standardize error reporting on the log module - Add log_perror() helper (context + strerror(errno)) to the log module - Replace all bare perror() calls with log_perror() so errors are routed through the unified logger (stderr sink + optional --log-file sink) - Convert fprintf(stderr, "Error:/Warning: ...") in client code to log_message(); raw fprintf kept only for progress/stats output --- src/client/client_cli.c | 50 +++++++++++++++++----------------- src/client/client_send.c | 12 ++++---- src/client/client_validation.c | 19 +++++++------ src/client/scanner.c | 5 ++-- src/server/server.c | 2 +- src/shared/array_list.c | 7 +++-- src/shared/chunk.c | 6 ++-- src/shared/file.c | 10 +++---- src/shared/file_send.c | 4 +-- src/shared/log.c | 6 ++++ src/shared/log.h | 1 + src/shared/multiprocessing.c | 4 +-- src/shared/queue.c | 7 +++-- src/shared/transport_ssh.c | 9 +++--- src/shared/transport_tcp.c | 14 +++++----- src/shared/utils.c | 3 +- 16 files changed, 86 insertions(+), 73 deletions(-) diff --git a/src/client/client_cli.c b/src/client/client_cli.c index 533b49d..33f7c29 100644 --- a/src/client/client_cli.c +++ b/src/client/client_cli.c @@ -60,7 +60,7 @@ static bool parse_positive_int(const char* s, int* out_val) { static int set_string_option(char** dest, const char* value, const char* option_name) { char* dup = str_dup(value); if (!dup) { - fprintf(stderr, "Error: memory allocation failed for %s\n", option_name); + log_message(LOG_LEVEL_ERROR, "memory allocation failed for %s", option_name); return -1; } free(*dest); @@ -71,7 +71,7 @@ static int set_string_option(char** dest, const char* value, const char* option_ /* Parse a string as a positive integer into *dest. Returns true on success, false on error. */ static int set_positive_int_option(int* dest, const char* value, const char* option_name) { if (!parse_positive_int(value, dest)) { - fprintf(stderr, "Error: %s must be a positive integer\n", option_name); + log_message(LOG_LEVEL_ERROR, "%s must be a positive integer", option_name); return -1; } return 0; @@ -80,7 +80,7 @@ static int set_positive_int_option(int* dest, const char* value, const char* opt /* Parse a string as a non-negative integer into *dest. Returns true on success, false on error. */ static int set_nonneg_int_option(int* dest, const char* value, const char* option_name) { if (!parse_nonneg_int(value, dest)) { - fprintf(stderr, "Error: %s must be a non-negative integer\n", option_name); + log_message(LOG_LEVEL_ERROR, "%s must be a non-negative integer", option_name); return -1; } return 0; @@ -94,7 +94,7 @@ static int parse_ull_arg(const char* val, unsigned long long* out, const char* o errno = 0; unsigned long long v = strtoull(val, &end, 10); if (errno != 0 || *end != '\0') { - fprintf(stderr, "Error: %s must be a non-negative integer\n", optname); + log_message(LOG_LEVEL_ERROR, "%s must be a non-negative integer", optname); return -1; } *out = v; @@ -106,13 +106,13 @@ static int config_add_pattern(char*** patterns, int* count, const char* value, const char* optname) { char** tmp = realloc(*patterns, (*count + 1) * sizeof(char*)); if (!tmp) { - fprintf(stderr, "Error: memory allocation failed for %s\n", optname); + log_message(LOG_LEVEL_ERROR, "memory allocation failed for %s", optname); return -1; } *patterns = tmp; char* dup = str_dup(value); if (!dup) { - fprintf(stderr, "Error: memory allocation failed for %s\n", optname); + log_message(LOG_LEVEL_ERROR, "memory allocation failed for %s", optname); return -1; } (*patterns)[(*count)++] = dup; @@ -215,7 +215,7 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args, if (entry) { if (entry->kind != OPT_FLAG) { if (i + 1 >= argc) { - fprintf(stderr, "Error: missing argument for %s\n", entry->name); + log_message(LOG_LEVEL_ERROR, "missing argument for %s", entry->name); return -1; } if (apply_table_option(config, entry, argv[++i]) != 0) @@ -241,7 +241,7 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args, if (set_positive_int_option(&config->ssh_port, argv[++i], "-p") != 0) return -1; if (config->ssh_port > 65535) { - log_message(LOG_LEVEL_ERROR, "SSH port must be 1-65535\n"); + log_message(LOG_LEVEL_ERROR, "SSH port must be 1-65535"); return -1; } } else if (opt_is(argv[i], "--exclude", NULL) && i + 1 < argc) { @@ -259,7 +259,7 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args, if (val >= DELTA_BLOCK_SIZE_MIN && val <= DELTA_BLOCK_SIZE_MAX) config->delta_block_size = (uint32_t)val; else - fprintf(stderr, "Warning: --delta-block value %llu out of range, using default\n", val); + log_message(LOG_LEVEL_WARNING, "--delta-block value %llu out of range, using default", val); } else if (opt_is(argv[i], "--delta-max", NULL) && i + 1 < argc) { unsigned long long val; if (parse_ull_arg(argv[++i], &val, "--delta-max") != 0) @@ -267,7 +267,7 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args, if (val >= DELTA_MIN_FILE_SIZE) config->delta_max_file_size = val; else - fprintf(stderr, "Warning: --delta-max value %llu too small, using default\n", val); + log_message(LOG_LEVEL_WARNING, "--delta-max value %llu too small, using default", val); } else if (opt_is(argv[i], "-c", "-z")) { config->use_compression = true; log_message(LOG_LEVEL_INFO, "Enabled Compression"); @@ -276,7 +276,7 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args, long level = strtol(argv[i + 1], &end_ptr, 10); if (*end_ptr == '\0') { if (level < 1 || level > 22) { - fprintf(stderr, "Error: compression level must be 1-22\n"); + log_message(LOG_LEVEL_ERROR, "compression level must be 1-22"); return -1; } config->compression_level = (int)level; @@ -298,11 +298,11 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args, log_message(LOG_LEVEL_INFO, "Enabled Chunk Serialization"); } else if (opt_is(argv[i], "--server-port", NULL) && i + 1 < argc) { if (!parse_positive_int(argv[++i], &config->server_port)) { - fprintf(stderr, "Error: invalid --server-port value: %s\n", argv[i]); + log_message(LOG_LEVEL_ERROR, "invalid --server-port value: %s", argv[i]); return -1; } if (config->server_port > 65535) { - fprintf(stderr, "Error: server port must be 1-65535\n"); + log_message(LOG_LEVEL_ERROR, "server port must be 1-65535"); return -1; } } else if (opt_is(argv[i], "--bwlimit", NULL) && i + 1 < argc) { @@ -310,11 +310,11 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args, if (parse_ull_arg(argv[++i], &kbps, "--bwlimit") != 0) return -1; if (kbps == 0) { - fprintf(stderr, "Error: --bwlimit must be a positive integer\n"); + log_message(LOG_LEVEL_ERROR, "--bwlimit must be a positive integer"); return -1; } if (kbps > ULLONG_MAX / 1024) { - fprintf(stderr, "Error: --bwlimit value too large\n"); + log_message(LOG_LEVEL_ERROR, "--bwlimit value too large"); return -1; } io_set_bwlimit(kbps * 1024); @@ -324,7 +324,7 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args, if (parse_ull_arg(argv[++i], &val, "--chunk-size") != 0) return -1; if (val == 0) { - fprintf(stderr, "Error: --chunk-size must be a positive integer\n"); + log_message(LOG_LEVEL_ERROR, "--chunk-size must be a positive integer"); return -1; } config->chunk_size = val; @@ -336,7 +336,7 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args, } FILE* lf = fopen(argv[++i], "a"); if (!lf) { - fprintf(stderr, "Error: could not open log file '%s': %s\n", argv[i], strerror(errno)); + log_message(LOG_LEVEL_ERROR, "could not open log file '%s': %s", argv[i], strerror(errno)); return -1; } config->log_file = lf; @@ -358,7 +358,7 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args, if (set_positive_int_option(&config->compression_level, argv[++i], "--compress-level") != 0) return -1; if (config->compression_level < 1 || config->compression_level > 22) { - fprintf(stderr, "Error: --compress-level must be between 1 and 22\n"); + log_message(LOG_LEVEL_ERROR, "--compress-level must be between 1 and 22"); return -1; } } else if (argv[i][0] == '-') { @@ -381,7 +381,7 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args, static int read_patterns_from_file(const char* filepath, char*** patterns, int* count) { FILE* fp = fopen(filepath, "r"); if (!fp) { - fprintf(stderr, "Error: could not open pattern file '%s': %s\n", filepath, strerror(errno)); + log_message(LOG_LEVEL_ERROR, "could not open pattern file '%s': %s", filepath, strerror(errno)); return -1; } char* line = NULL; @@ -420,7 +420,7 @@ int main(int argc, char* argv[]) { bool config_owned_by_pipeline = false; Config* config = config_create(); if (!config) { - fprintf(stderr, "Error: failed to allocate config\n"); + log_message(LOG_LEVEL_ERROR, "failed to allocate config"); return 1; } config->save_to_disk = save_to_disk; @@ -441,20 +441,20 @@ int main(int argc, char* argv[]) { free(config->receive_root_directory); config->send_directory = str_dup(argv[positional_args[0]]); if (!config->send_directory) { - fprintf(stderr, "Error: memory allocation failed\n"); + log_message(LOG_LEVEL_ERROR, "memory allocation failed"); exit_code = 1; goto cleanup; } config->receive_root_directory = str_dup(argv[positional_args[1]]); if (!config->receive_root_directory) { - fprintf(stderr, "Error: memory allocation failed\n"); + log_message(LOG_LEVEL_ERROR, "memory allocation failed"); exit_code = 1; goto cleanup; } config->save_to_disk = true; config_parse_ssh_dest(config); } else if (positional_count == 1) { - fprintf(stderr, "Error: missing destination argument\n"); + log_message(LOG_LEVEL_ERROR, "missing destination argument"); print_usage(); exit_code = 1; goto cleanup; @@ -462,7 +462,7 @@ int main(int argc, char* argv[]) { if (!config->send_directory && env_source) { config->send_directory = str_dup(env_source); if (!config->send_directory) { - fprintf(stderr, "Error: memory allocation failed\n"); + log_message(LOG_LEVEL_ERROR, "memory allocation failed"); exit_code = 1; goto cleanup; } @@ -470,7 +470,7 @@ int main(int argc, char* argv[]) { if (!config->receive_root_directory && env_dest) { config->receive_root_directory = str_dup(env_dest); if (!config->receive_root_directory) { - fprintf(stderr, "Error: memory allocation failed\n"); + log_message(LOG_LEVEL_ERROR, "memory allocation failed"); exit_code = 1; goto cleanup; } diff --git a/src/client/client_send.c b/src/client/client_send.c index 900a150..b138bc4 100644 --- a/src/client/client_send.c +++ b/src/client/client_send.c @@ -42,7 +42,7 @@ static ScannerOptions scanner_options_from_config(const Config* config, int num_ static Client* connect_transfer_client(const Config* config) { if (config->transport == TRANSPORT_SSH) { if (config->use_sendfile) { - fprintf(stderr, "Error: -f/--sendfile is not supported with SSH transport\n"); + log_message(LOG_LEVEL_ERROR, "-f/--sendfile is not supported with SSH transport"); return NULL; } return client_connect_ssh(config->ssh_destination, config->ssh_port, @@ -377,7 +377,7 @@ static int send_chunks_multithreaded(void* pipeline_context) { Client* client = connect_transfer_client(context->config); if (!client) { if (context->config->transport == TRANSPORT_TCP) - fprintf(stderr, "Error: could not connect to server%s\n", + log_message(LOG_LEVEL_ERROR, "could not connect to server%s", context->config->use_tls ? " via TLS" : ""); pipeline_cancel(context); mark_sender_done(context); @@ -425,7 +425,7 @@ static int send_chunks_multithreaded(void* pipeline_context) { return thrd_error; } if (send_chunk(client, current_chunk, context->config) != 0) { - fprintf(stderr, "Error: unexpected error while sending chunk\n"); + log_message(LOG_LEVEL_ERROR, "unexpected error while sending chunk"); chunk_destroy(current_chunk); pipeline_cancel(context); disconnect_transfer_client(client); @@ -585,7 +585,7 @@ int send_files(Config* config) { Client* client = connect_transfer_client(config); if (!client) { if (config->transport == TRANSPORT_TCP) - fprintf(stderr, "Error: could not connect to server%s\n", config->use_tls ? " via TLS" : ""); + log_message(LOG_LEVEL_ERROR, "could not connect to server%s", config->use_tls ? " via TLS" : ""); return 1; } ProtocolSession session; @@ -736,7 +736,7 @@ int send_files_multithreaded(Config* config) { sender_created = (thrd_create(&sender, send_chunks_multithreaded, context) == thrd_success); if (!scanner_created || !loader_created || !sender_created) { - perror("Error creating threads"); + log_perror("Error creating threads"); pipeline_cancel(context); mtx_lock(&context->mutex_progress); context->sender_done = true; @@ -756,7 +756,7 @@ int send_files_multithreaded(Config* config) { if (config->show_progress) { progress_created = (thrd_create(&progress, progress_thread_fn, context) == thrd_success); if (!progress_created) { - perror("Error creating progress thread"); + log_perror("Error creating progress thread"); /* Non-fatal; continue without progress reporting */ } } diff --git a/src/client/client_validation.c b/src/client/client_validation.c index 0ac4138..eb890fd 100644 --- a/src/client/client_validation.c +++ b/src/client/client_validation.c @@ -1,37 +1,38 @@ #include "client_validation.h" +#include "log.h" #include "usage.h" #include /* Validate config after parsing. Returns true if valid. */ bool validate_config(const Config* config) { if (!config->send_directory || !config->receive_root_directory) { - fprintf(stderr, "Error: source and destination directories are required\n"); + log_message(LOG_LEVEL_ERROR, "source and destination directories are required"); print_usage(); return false; } if (config->use_sendfile && (config->use_chunk_serialization || config->use_compression)) { - fprintf(stderr, "Error: -f/--sendfile cannot be combined with -c (compression) or -s (chunk " - "serialization)\n"); + log_message(LOG_LEVEL_ERROR, "-f/--sendfile cannot be combined with -c (compression) or -s " + "(chunk serialization)"); return false; } if (config->transport == TRANSPORT_SSH && config->use_sendfile) { - fprintf(stderr, "Error: -f/--sendfile is not supported with SSH transport\n"); + log_message(LOG_LEVEL_ERROR, "-f/--sendfile is not supported with SSH transport"); return false; } if (config->use_incremental && config->use_chunk_serialization) { - fprintf(stderr, "Error: --incremental is not supported with -s (chunk serialization)\n"); + log_message(LOG_LEVEL_ERROR, "--incremental is not supported with -s (chunk serialization)"); return false; } if (config->use_delta && !config->use_incremental) { - fprintf(stderr, "Error: --delta requires --incremental\n"); + log_message(LOG_LEVEL_ERROR, "--delta requires --incremental"); return false; } if (config->use_delta && config->use_chunk_serialization) { - fprintf(stderr, "Error: --delta cannot be combined with -s (chunk serialization)\n"); + log_message(LOG_LEVEL_ERROR, "--delta cannot be combined with -s (chunk serialization)"); return false; } if (config->use_delta && config->use_sendfile) { - fprintf(stderr, "Error: --delta cannot be combined with -f (sendfile)\n"); + log_message(LOG_LEVEL_ERROR, "--delta cannot be combined with -f (sendfile)"); return false; } if (config->append || config->append_verify) { @@ -42,7 +43,7 @@ bool validate_config(const Config* config) { } if (config->use_tls) { if (!config->tls_cert || !config->tls_key) { - fprintf(stderr, "Error: --tls requires --cert and --key\n"); + log_message(LOG_LEVEL_ERROR, "--tls requires --cert and --key"); return false; } } diff --git a/src/client/scanner.c b/src/client/scanner.c index 3c67145..beadb85 100644 --- a/src/client/scanner.c +++ b/src/client/scanner.c @@ -1,3 +1,4 @@ +#include "log.h" #include "scanner.h" #include "array_list.h" #include "chunk.h" @@ -226,7 +227,7 @@ static int open_next_directory(DirectoryScanner* scanner) { free(de); scanner->current_dir = opendir(scanner->current_path); if (scanner->current_dir == NULL) { - perror("Could not open directory"); + log_perror("Could not open directory"); free(scanner->current_path); scanner->current_path = NULL; scanner->failed = true; @@ -564,7 +565,7 @@ static bool scan_root_directory(ParallelScanner* ps, const char* root_directory, ArrayList* subdirs) { DIR* dir = opendir(root_directory); if (!dir) { - perror("Could not open root directory for parallel scan"); + log_perror("Could not open root directory for parallel scan"); return false; } const struct dirent* entry; diff --git a/src/server/server.c b/src/server/server.c index 1485210..510d0a3 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -129,7 +129,7 @@ void handler(int file_descriptor) { if (receiver_created) writer_created = thrd_create(&writer, write_thread, context) == thrd_success; if (!receiver_created || !writer_created) { - perror("Error creating Threads"); + log_perror("Error creating Threads"); if (receiver_created) { mtx_lock(&context->mutex); atomic_store(&context->cancelled, true); diff --git a/src/shared/array_list.c b/src/shared/array_list.c index 6dc785e..0974fd4 100644 --- a/src/shared/array_list.c +++ b/src/shared/array_list.c @@ -1,3 +1,4 @@ +#include "log.h" #include "array_list.h" #include #include @@ -6,7 +7,7 @@ ArrayList* array_list_create(void (*item_destroyer)(void* item)) { ArrayList* list = (ArrayList*)malloc(sizeof(ArrayList)); if (list == NULL) { - perror("ERROR: Could not allocate memory for array list struct"); + log_perror("ERROR: Could not allocate memory for array list struct"); return NULL; } @@ -42,7 +43,7 @@ static bool array_list_extend(ArrayList* array_list) { new_capacity = INITIAL_ARRAY_SIZE; void* new_items = realloc(array_list->items, new_capacity * sizeof(void*)); if (new_items == NULL) { - perror("ERROR: Could not reallocate memory for array list items"); + log_perror("ERROR: Could not reallocate memory for array list items"); return false; } array_list->items = new_items; @@ -68,7 +69,7 @@ void** array_list_to_array(const ArrayList* array_list) { } void** array = malloc(array_list->size * sizeof(void*)); if (array == NULL) { - perror("Could not malloc space for array from array list!"); + log_perror("Could not malloc space for array from array list!"); return NULL; } memcpy(array, array_list->items, array_list->size * sizeof(void*)); diff --git a/src/shared/chunk.c b/src/shared/chunk.c index a181e8e..158503c 100644 --- a/src/shared/chunk.c +++ b/src/shared/chunk.c @@ -20,7 +20,7 @@ Chunk* chunk_create(File** items, int element_count) { Chunk* chunk = (Chunk*)malloc(sizeof(Chunk)); if (chunk == NULL) { - perror("ERROR: Could not allocate memory for chunk structure"); + log_perror("ERROR: Could not allocate memory for chunk structure"); return NULL; } @@ -118,7 +118,7 @@ Chunk* chunk_deserialize(Data* data, bool use_metadata) { char* path = malloc(path_len + 1); if (path == NULL) { - perror("Could not allocate memory for file path"); + log_perror("Could not allocate memory for file path"); array_list_delete(files); return NULL; } @@ -191,7 +191,7 @@ Chunk* chunk_deserialize(Data* data, bool use_metadata) { void* file_data = malloc(file_data_size > 0 ? file_data_size : 1); if (file_data == NULL) { - perror("Could not allocate memory for file data"); + log_perror("Could not allocate memory for file data"); file_destroy(file); array_list_delete(files); return NULL; diff --git a/src/shared/file.c b/src/shared/file.c index 17c65c0..8dc17e5 100644 --- a/src/shared/file.c +++ b/src/shared/file.c @@ -29,7 +29,7 @@ File* file_create(const char* path) { return NULL; File* file = (File*)malloc(sizeof(File)); if (file == NULL) { - perror("ERROR: Could not allocate memory for file struct"); + log_perror("ERROR: Could not allocate memory for file struct"); return NULL; } @@ -69,7 +69,7 @@ void file_destroy(void* item) { FileMetadata* file_metadata_create(const struct stat* stats) { FileMetadata* m = malloc(sizeof(FileMetadata)); if (m == NULL) { - perror("ERROR: Could not allocate memory for file metadata"); + log_perror("ERROR: Could not allocate memory for file metadata"); return NULL; } m->mode = stats->st_mode; @@ -96,7 +96,7 @@ bool file_load_data(File* file) { return true; file->data->data = malloc(file->data->size); if (file->data->data == NULL) { - perror("Could not allocate memory for file data"); + log_perror("Could not allocate memory for file data"); return false; } } @@ -125,13 +125,13 @@ bool file_write_to_disk(const char* path, const void* data, unsigned long long d size_t file_content_to_buffer(File* file) { FILE* file_pointer = fopen(file->path, "rb"); if (file_pointer == NULL) { - perror("Could not open the file!"); + log_perror("Could not open the file!"); return 0; } size_t bytes_read = fread(file->data->data, 1, file->data->size, file_pointer); if (bytes_read != (size_t)file->data->size) { fclose(file_pointer); - perror("Read unexpected number of bytes from File!"); + log_perror("Read unexpected number of bytes from File!"); return 0; } fclose(file_pointer); diff --git a/src/shared/file_send.c b/src/shared/file_send.c index 61e1762..95fcaeb 100644 --- a/src/shared/file_send.c +++ b/src/shared/file_send.c @@ -62,7 +62,7 @@ bool file_send_sendfile(File* file, int file_descriptor, bool use_metadata, int int fd = open(file->path, O_RDONLY); if (fd == -1) { - perror("Could not open file for sendfile"); + log_perror("Could not open file for sendfile"); return false; } @@ -121,7 +121,7 @@ bool file_send_sendfile(File* file, int file_descriptor, bool use_metadata, int if (sent == -1) { if (errno == EAGAIN || errno == EINTR) continue; - perror("sendfile failed"); + log_perror("sendfile failed"); close(fd); return false; } diff --git a/src/shared/log.c b/src/shared/log.c index 37a1a73..29bbe0d 100644 --- a/src/shared/log.c +++ b/src/shared/log.c @@ -1,6 +1,8 @@ #include "log.h" +#include #include #include +#include #include static const char* log_level_strings[] = {"DEBUG", "INFO", "WARN", "ERROR"}; @@ -50,3 +52,7 @@ void log_message(LogLevel log_level, const char* format, ...) { va_end(args); } } + +void log_perror(const char* context) { + log_message(LOG_LEVEL_ERROR, "%s: %s", context, strerror(errno)); +} diff --git a/src/shared/log.h b/src/shared/log.h index 0aa622b..acea629 100644 --- a/src/shared/log.h +++ b/src/shared/log.h @@ -6,6 +6,7 @@ typedef enum { LOG_LEVEL_DEBUG, LOG_LEVEL_INFO, LOG_LEVEL_WARNING, LOG_LEVEL_ERROR } LogLevel; void log_message(LogLevel log_level, const char* message, ...); +void log_perror(const char* context); void set_log_level(LogLevel level); void log_set_file(FILE* fp); diff --git a/src/shared/multiprocessing.c b/src/shared/multiprocessing.c index 49de752..6b677f3 100644 --- a/src/shared/multiprocessing.c +++ b/src/shared/multiprocessing.c @@ -55,7 +55,7 @@ PipelineContextSender* pipeline_context_sender_create(Config* config, Queue* que return context; fail: - perror("Error initializing synchronization objects"); + log_perror("Error initializing synchronization objects"); if (init >= 6) cnd_destroy(&context->condition_not_empty_loader); if (init >= 5) @@ -116,7 +116,7 @@ PipelineContextReceiver* pipeline_context_receiver_create(Config* config, Queue* return context; fail: - perror("Error initializing synchronization objects"); + log_perror("Error initializing synchronization objects"); if (init >= 3) cnd_destroy(&context->condition_not_empty); if (init >= 2) diff --git a/src/shared/queue.c b/src/shared/queue.c index b9930dd..e4e5f09 100644 --- a/src/shared/queue.c +++ b/src/shared/queue.c @@ -1,3 +1,4 @@ +#include "log.h" #include #include #include @@ -13,7 +14,7 @@ Queue* queue_create(int capacity, void (*destroyer)(void* item)) { Queue* queue = (Queue*)malloc(sizeof(Queue)); if (queue == NULL) { - perror("ERROR: Could not allocate memory for queue structure"); + log_perror("ERROR: Could not allocate memory for queue structure"); return NULL; } @@ -72,7 +73,7 @@ static bool queue_double_capacity(Queue* queue) { new_capacity = 100; void** new_items = malloc(new_capacity * sizeof(void*)); if (new_items == NULL) { - perror("ERROR: Could not allocate memory for doubling capacity of queue."); + log_perror("ERROR: Could not allocate memory for doubling capacity of queue."); return false; } for (int i = 0; i < queue->size; i++) @@ -127,7 +128,7 @@ bool queue_enqueue_multithreaded_cancel(Queue* queue, void* item, mtx_t* mutex, void* queue_dequeue(Queue* queue) { if (queue == NULL || queue_is_empty(queue)) { - perror("ERROR: Could not dequeue from null or empty queue."); + log_perror("ERROR: Could not dequeue from null or empty queue."); return NULL; } diff --git a/src/shared/transport_ssh.c b/src/shared/transport_ssh.c index eb5f96e..4941526 100644 --- a/src/shared/transport_ssh.c +++ b/src/shared/transport_ssh.c @@ -1,3 +1,4 @@ +#include "log.h" #include "transport_ssh.h" #include "utils.h" #include @@ -76,7 +77,7 @@ Client* client_connect_ssh(const char* destination, int port, const char* server int sv[2]; if (socketpair(AF_UNIX, SOCK_STREAM, 0, sv) < 0) { - perror("socketpair failed"); + log_perror("socketpair failed"); remote_dest_destroy(&r); return NULL; } @@ -89,7 +90,7 @@ Client* client_connect_ssh(const char* destination, int port, const char* server int exec_pipe[2]; if (pipe(exec_pipe) < 0) { - perror("pipe failed"); + log_perror("pipe failed"); close(sv[0]); close(sv[1]); remote_dest_destroy(&r); @@ -98,7 +99,7 @@ Client* client_connect_ssh(const char* destination, int port, const char* server pid_t pid = fork(); if (pid < 0) { - perror("fork failed"); + log_perror("fork failed"); close(sv[0]); close(sv[1]); close(exec_pipe[0]); @@ -151,7 +152,7 @@ Client* client_connect_ssh(const char* destination, int port, const char* server ssh_argv[ac++] = "--stdio"; ssh_argv[ac] = NULL; execvp("ssh", ssh_argv); - perror("exec of ssh failed"); + log_perror("exec of ssh failed"); ssize_t wret = write(exec_pipe[1], "x", 1); (void)wret; _exit(1); diff --git a/src/shared/transport_tcp.c b/src/shared/transport_tcp.c index 062d60c..3c9c9a1 100644 --- a/src/shared/transport_tcp.c +++ b/src/shared/transport_tcp.c @@ -30,20 +30,20 @@ static void sigchld_handler(int sig) { Server* server_create(int port) { Server* server = (Server*)malloc(sizeof(Server)); if (server == NULL) { - perror("Could not allocate space for Server"); + log_perror("Could not allocate space for Server"); return NULL; } int file_descriptor = socket(AF_INET, SOCK_STREAM, 0); if (file_descriptor < 0) { - perror("Could not create Socket!"); + log_perror("Could not create Socket!"); free(server); return NULL; } server->file_descriptor = file_descriptor; int opt = 1; if (setsockopt(server->file_descriptor, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt))) { - perror("Error setting a socket option!"); + log_perror("Error setting a socket option!"); close(server->file_descriptor); free(server); return NULL; @@ -59,7 +59,7 @@ Server* server_create(int port) { if (bind(server->file_descriptor, (struct sockaddr*)&server->address, server->address_length) < 0) { - perror("Could not bind server"); + log_perror("Could not bind server"); close(server->file_descriptor); free(server); return NULL; @@ -83,7 +83,7 @@ void server_delete(Server** server) { static void accept_loop(Server* server, void (*child_fn)(int, void*), void* child_ctx, const char* log_fmt) { if (listen(server->file_descriptor, SOMAXCONN) < 0) { - perror("Could not listen on port!"); + log_perror("Could not listen on port!"); return; } signal(SIGCHLD, sigchld_handler); @@ -92,7 +92,7 @@ static void accept_loop(Server* server, void (*child_fn)(int, void*), void* chil socklen_t client_len = sizeof(client_addr); int fd = accept(server->file_descriptor, (struct sockaddr*)&client_addr, &client_len); if (fd < 0) { - perror("Could not accept the connection"); + log_perror("Could not accept the connection"); continue; } tcp_apply_socket_timeout(fd); @@ -222,7 +222,7 @@ bool tcp_connect_socket(Client* client, char* host, int port) { freeaddrinfo(result); if (!connected) { - perror("Could not connect to Server!"); + log_perror("Could not connect to Server!"); return false; } diff --git a/src/shared/utils.c b/src/shared/utils.c index 2926fca..26a6213 100644 --- a/src/shared/utils.c +++ b/src/shared/utils.c @@ -1,3 +1,4 @@ +#include "log.h" #include "utils.h" #include "array_list.h" #include "libgen.h" @@ -54,7 +55,7 @@ bool mkdir_r(const char* path) { struct stat st; if (stat(path_current, &st) != 0) { if (mkdir(path_current, 0755) != 0) { - perror("Could not create directory"); + log_perror("Could not create directory"); ok = false; break; } From 047a9a19063183dff24289be48e4851307de47d1 Mon Sep 17 00:00:00 2001 From: TapTap Date: Sun, 30 Aug 2026 14:20:33 +0200 Subject: [PATCH 7/7] style: apply clang-format 18 --- src/client/client_send.c | 5 +++-- src/client/scanner.c | 4 ++-- tests/test_file.c | 9 ++++++--- tests/test_scanner.c | 3 ++- 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/client/client_send.c b/src/client/client_send.c index b138bc4..f8fb31e 100644 --- a/src/client/client_send.c +++ b/src/client/client_send.c @@ -378,7 +378,7 @@ static int send_chunks_multithreaded(void* pipeline_context) { if (!client) { if (context->config->transport == TRANSPORT_TCP) log_message(LOG_LEVEL_ERROR, "could not connect to server%s", - context->config->use_tls ? " via TLS" : ""); + context->config->use_tls ? " via TLS" : ""); pipeline_cancel(context); mark_sender_done(context); return thrd_error; @@ -585,7 +585,8 @@ int send_files(Config* config) { Client* client = connect_transfer_client(config); if (!client) { if (config->transport == TRANSPORT_TCP) - log_message(LOG_LEVEL_ERROR, "could not connect to server%s", config->use_tls ? " via TLS" : ""); + log_message(LOG_LEVEL_ERROR, "could not connect to server%s", + config->use_tls ? " via TLS" : ""); return 1; } ProtocolSession session; diff --git a/src/client/scanner.c b/src/client/scanner.c index beadb85..46b70c7 100644 --- a/src/client/scanner.c +++ b/src/client/scanner.c @@ -518,8 +518,8 @@ static Chunk* batch_files(ArrayList* files, unsigned long long chunk_size, Queue /* Scan one root-directory entry into either the subdirs or files list. */ static void scan_root_entry(const ScannerOptions* options, const char* root_directory, - const struct dirent* entry, ArrayList* root_files, - ArrayList* subdirs, ParallelScanner* ps) { + const struct dirent* entry, ArrayList* root_files, ArrayList* subdirs, + ParallelScanner* ps) { ScannerEntry inspected; int inspection = scanner_inspect_entry(options, root_directory, root_directory, entry->d_name, &inspected); diff --git a/tests/test_file.c b/tests/test_file.c index ad82584..6b8469e 100644 --- a/tests/test_file.c +++ b/tests/test_file.c @@ -34,7 +34,8 @@ static void test_file_destroy_normal() { static void test_file_load_data() { const char* content = "Hello Load Test"; - EXPECT_TRUE(file_write_to_disk("test_file_load_data.txt", content, strlen(content), false, false)); + EXPECT_TRUE( + file_write_to_disk("test_file_load_data.txt", content, strlen(content), false, false)); struct stat st; EXPECT_EQ_INT(stat("test_file_load_data.txt", &st), 0); @@ -89,7 +90,8 @@ static void test_file_save_to_disk() { static void test_file_write_to_disk_basic() { const char* content = "Basic file_write_to_disk test"; - EXPECT_TRUE(file_write_to_disk("test_file_write_to_disk_basic.txt", content, strlen(content), false, false)); + EXPECT_TRUE(file_write_to_disk("test_file_write_to_disk_basic.txt", content, strlen(content), + false, false)); struct stat st; EXPECT_EQ_INT(stat("test_file_write_to_disk_basic.txt", &st), 0); @@ -108,7 +110,8 @@ static void test_file_write_to_disk_basic() { static void test_file_write_to_disk_creates_dirs() { const char* content = "Nested dir test"; - EXPECT_TRUE(file_write_to_disk("test_nested_tmp/nested/file.txt", content, strlen(content), false, false)); + EXPECT_TRUE(file_write_to_disk("test_nested_tmp/nested/file.txt", content, strlen(content), false, + false)); struct stat st; EXPECT_EQ_INT(stat("test_nested_tmp/nested/file.txt", &st), 0); diff --git a/tests/test_scanner.c b/tests/test_scanner.c index 8667fd5..71d8a20 100644 --- a/tests/test_scanner.c +++ b/tests/test_scanner.c @@ -394,7 +394,8 @@ static void test_parallel_scanner_root_chunks_without_workers() { create_test_file(file1, "a"); create_test_file(file2, "b"); - ScannerOptions options = {false, 1, NULL, 0, NULL, 0, 0, 0, 0, 0, false, false, false, false, false}; + ScannerOptions options = {false, 1, NULL, 0, NULL, 0, 0, 0, + 0, 0, false, false, false, false, false}; ParallelScanner* scanner = parallel_scanner_create_with_options(dir, &options); EXPECT_NOT_NULL(scanner);