From 8f4ea62ae72c481fe13a9547a53a615c1cb3580d Mon Sep 17 00:00:00 2001 From: TapTap Date: Thu, 16 Jul 2026 12:08:09 +0200 Subject: [PATCH] Phase 1+2: bug fixes and dead code cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 1 — Bugs: - receive_data: fix unsigned long long vs size_t mismatch - chunk_deserialize: add NULL check in multiprocessing.c and server.c - file_content_to_buffer: add missing fclose on error path - send_files_multithreaded: propagate sender thread result - server: add SIGPIPE handler - to_disk: check fwrite return value - scanner: use S_ISDIR instead of !S_ISREG - scanner: free cur_path before early return Phase 2 — Cleanup: - Remove unused chunk_decompress function - Remove unused array_list_clear function - Remove unused num_connections config field - Remove duplicate FILE_METADATA_WIRE_SIZE macro - Update tests for removed APIs --- src/client/client_cli.c | 2 +- src/client/client_send.c | 5 +++-- src/client/scanner.c | 6 ++++-- src/server/server.c | 7 +++++++ src/shared/array_list.c | 8 -------- src/shared/array_list.h | 1 - src/shared/chunk.c | 19 ------------------- src/shared/chunk.h | 2 -- src/shared/config.c | 5 +---- src/shared/config.h | 3 +-- src/shared/file.c | 9 +++++++-- src/shared/metadata.c | 2 -- src/shared/multiprocessing.c | 4 ++++ src/shared/protocol.c | 8 ++++---- tests/test_array_list.c | 12 ------------ tests/test_compression.c | 6 +++++- tests/test_config.c | 13 ++++++------- 17 files changed, 43 insertions(+), 69 deletions(-) diff --git a/src/client/client_cli.c b/src/client/client_cli.c index f26fd62..8da09eb 100644 --- a/src/client/client_cli.c +++ b/src/client/client_cli.c @@ -46,7 +46,7 @@ int main(int argc, char *argv[]) { } Config *config = config_create(str_dup("1.0.0"), NULL, NULL, - save_to_disk, false, false, false, false, 5, 20, false); + save_to_disk, false, false, false, false, 5, false); int positional_args[2]; int positional_count = 0; diff --git a/src/client/client_send.c b/src/client/client_send.c index c89f9b0..9556929 100644 --- a/src/client/client_send.c +++ b/src/client/client_send.c @@ -171,10 +171,11 @@ int send_files_multithreaded(Config *config) { return 1; } + int sender_result; thrd_join(scanner, NULL); thrd_join(loader, NULL); - thrd_join(sender, NULL); + thrd_join(sender, &sender_result); pipeline_context_sender_destroy(context); - return 0; + return sender_result == thrd_success ? 0 : -1; } diff --git a/src/client/scanner.c b/src/client/scanner.c index 386849c..d4a331a 100644 --- a/src/client/scanner.c +++ b/src/client/scanner.c @@ -90,7 +90,7 @@ Chunk *directory_scanner_next(DirectoryScanner *scanner) { continue; } - if (!S_ISREG(stats.st_mode)) { + if (S_ISDIR(stats.st_mode)) { queue_enqueue(scanner->directories, (void *)cur_path); } else { File *file = file_create(cur_path); @@ -99,8 +99,10 @@ Chunk *directory_scanner_next(DirectoryScanner *scanner) { file->metadata = file_metadata_create(&stats); array_list_add(chunk_data, file); chunk_data_size += file->data->size; - if (chunk_data_size > DESIRED_CHUNK_SIZE) + if (chunk_data_size > DESIRED_CHUNK_SIZE) { + free(cur_path); return chunk_data_to_chunk(chunk_data); + } free(cur_path); } } diff --git a/src/server/server.c b/src/server/server.c index 3af90ae..db35a07 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -11,6 +11,7 @@ #include "transport_tcp.h" #include "unistd.h" #include "utils.h" +#include #include #include #include @@ -27,6 +28,11 @@ int receive_files(Config *config, int file_descriptor) { } Chunk *chunk = chunk_deserialize(data_to_process, config->use_metadata); data_destroy(data_to_process); + if (chunk == NULL) { + log_message(LOG_LEVEL_ERROR, "Failed to deserialize chunk, skipping"); + send_status(file_descriptor, STATUS_ERROR); + return -1; + } for (int i = 0; i < chunk->element_count; i++) { if (config->save_to_disk) { @@ -79,6 +85,7 @@ void handler(int file_descriptor) { } int main(int argc, char *argv[]) { + signal(SIGPIPE, SIG_IGN); for (int i = 1; i < argc; i++) { if (strcmp(argv[i], "--stdio") == 0) { io_set_fds(STDIN_FILENO, STDOUT_FILENO); diff --git a/src/shared/array_list.c b/src/shared/array_list.c index 771d074..1c33c06 100644 --- a/src/shared/array_list.c +++ b/src/shared/array_list.c @@ -35,14 +35,6 @@ void array_list_delete(ArrayList *array_list) { free(array_list); } -void array_list_clear(ArrayList *array_list) { - if (array_list == NULL) - return; - for (int i = 0; i < array_list->size; i++) - array_list->items[i] = NULL; - array_list->size = 0; -} - void array_list_extend(ArrayList *array_list) { if (array_list == NULL) return; diff --git a/src/shared/array_list.h b/src/shared/array_list.h index 966449f..e0e4afd 100644 --- a/src/shared/array_list.h +++ b/src/shared/array_list.h @@ -12,7 +12,6 @@ typedef struct ArrayList { ArrayList *array_list_create(void (*item_destroyer)(void *item)); void array_list_delete(ArrayList *array_list); -void array_list_clear(ArrayList *array_list); void array_list_extend(ArrayList *array_list); void array_list_add(ArrayList *array_list, void *item); void **array_list_to_array(ArrayList *array_list); diff --git a/src/shared/chunk.c b/src/shared/chunk.c index 9740643..cd26510 100644 --- a/src/shared/chunk.c +++ b/src/shared/chunk.c @@ -178,24 +178,5 @@ Data *chunk_compress(Chunk *chunk, int compression_level, bool use_metadata) { return compressed; } -Chunk *chunk_decompress(Data *compressed_data, bool use_metadata) { - log_message(LOG_LEVEL_DEBUG, "Starting to decompress chunk"); - Data *uncompressed_data = data_decompress(compressed_data); - if (uncompressed_data == NULL) { - log_message(LOG_LEVEL_ERROR, "Failed to decompress chunk data"); - return NULL; - } - - Chunk *chunk = chunk_deserialize(uncompressed_data, use_metadata); - if (chunk == NULL) { - log_message(LOG_LEVEL_ERROR, "Failed to deserialize chunk data"); - data_destroy(uncompressed_data); - return NULL; - } - - data_destroy(uncompressed_data); - log_message(LOG_LEVEL_DEBUG, "Chunk successfully decompressed"); - return chunk; -} diff --git a/src/shared/chunk.h b/src/shared/chunk.h index 1d5c80d..a411e71 100644 --- a/src/shared/chunk.h +++ b/src/shared/chunk.h @@ -18,7 +18,5 @@ void chunk_destroy(void *chunk); Data *chunk_serialize(Chunk *chunk, bool use_metadata); Chunk *chunk_deserialize(Data *data, bool use_metadata); Data *chunk_compress(Chunk *chunk, int compression_level, bool use_metadata); -Chunk *chunk_decompress(Data *compressed_data, bool use_metadata); - #endif diff --git a/src/shared/config.c b/src/shared/config.c index 6267c1a..a51bdc8 100644 --- a/src/shared/config.c +++ b/src/shared/config.c @@ -10,7 +10,7 @@ Config *config_create(char *version, char *send_directory, char *receive_directory, bool save_to_disk, bool use_multithreading, bool use_chunk_serialization, bool use_compression, bool use_metadata, - int compression_level, int num_connections, bool use_sendfile) { + int compression_level, bool use_sendfile) { Config *config = malloc(sizeof(Config)); config->version = version; @@ -22,7 +22,6 @@ Config *config_create(char *version, char *send_directory, config->use_compression = use_compression; config->use_metadata = use_metadata; config->compression_level = compression_level; - config->num_connections = num_connections; config->use_sendfile = use_sendfile; config->transport = TRANSPORT_TCP; config->ssh_destination = NULL; @@ -68,7 +67,6 @@ void config_send(int file_descriptor, Config *config) { send_int(file_descriptor, config->use_compression); send_int(file_descriptor, config->use_metadata); send_int(file_descriptor, config->compression_level); - send_int(file_descriptor, config->num_connections); send_int(file_descriptor, config->use_sendfile); if (receive_status(file_descriptor) != STATUS_OK) { perror("Error transmitting config!"); @@ -87,7 +85,6 @@ Config *config_receive(int file_descriptor) { config->use_compression = receive_int(file_descriptor); config->use_metadata = receive_int(file_descriptor); config->compression_level = receive_int(file_descriptor); - config->num_connections = receive_int(file_descriptor); config->use_sendfile = receive_int(file_descriptor); config->transport = TRANSPORT_TCP; config->ssh_destination = NULL; diff --git a/src/shared/config.h b/src/shared/config.h index db12612..93358cb 100644 --- a/src/shared/config.h +++ b/src/shared/config.h @@ -19,7 +19,6 @@ typedef struct Config { bool use_sendfile; bool use_metadata; int compression_level; - int num_connections; TransportType transport; char *ssh_destination; } Config; @@ -28,7 +27,7 @@ Config *config_create(char *version, char *send_directory, char *receive_directory, bool save_to_disk, bool use_multithreading, bool use_chunk_serialization, bool use_compression, bool use_metadata, - int compression_level, int num_connections, bool use_sendfile); + int compression_level, bool use_sendfile); void config_delete(Config *config); void config_send(int file_descriptor, Config *config); Config *config_receive(int file_descriptor); diff --git a/src/shared/file.c b/src/shared/file.c index 604ce98..a04b177 100644 --- a/src/shared/file.c +++ b/src/shared/file.c @@ -113,7 +113,11 @@ void to_disk(const char *path, const void *data, unsigned long long data_size) { perror("Could not open File"); exit(EXIT_FAILURE); } - fwrite(data, 1, data_size, file_pointer); + if (fwrite(data, 1, data_size, file_pointer) != data_size) { + perror("Failed to write all data to disk"); + fclose(file_pointer); + exit(EXIT_FAILURE); + } fclose(file_pointer); free(dir_to_free); } @@ -171,7 +175,8 @@ size_t file_content_to_buffer(File *file) { size_t bytes_read = fread(file->data->data, 1, file->data->size, file_pointer); if (bytes_read != (size_t)file->data->size) { - perror("Read to many or to less bytes from File!"); + fclose(file_pointer); + perror("Read unexpected number of bytes from File!"); return 0; } fclose(file_pointer); diff --git a/src/shared/metadata.c b/src/shared/metadata.c index 02c453b..3fe822d 100644 --- a/src/shared/metadata.c +++ b/src/shared/metadata.c @@ -8,8 +8,6 @@ #include #include -#define FILE_METADATA_WIRE_SIZE (sizeof(mode_t) + sizeof(uid_t) + sizeof(gid_t) + sizeof(time_t) + sizeof(long)) - void metadata_to_buf(char **buf, FileMetadata *m) { int present = (m != NULL) ? 1 : 0; memcpy(*buf, &present, sizeof(int)); diff --git a/src/shared/multiprocessing.c b/src/shared/multiprocessing.c index 9d57c8f..cf2b0a5 100644 --- a/src/shared/multiprocessing.c +++ b/src/shared/multiprocessing.c @@ -84,6 +84,10 @@ static void receive_chunk_enqueue(int file_descriptor, } Chunk *chunk = chunk_deserialize(data_to_process, context->config->use_metadata); data_destroy(data_to_process); + if (chunk == NULL) { + log_message(LOG_LEVEL_ERROR, "Failed to deserialize chunk, skipping"); + return; + } for (int i = 0; i < chunk->element_count; i++) { File *file = chunk->items[i]; diff --git a/src/shared/protocol.c b/src/shared/protocol.c index 734387a..5802912 100644 --- a/src/shared/protocol.c +++ b/src/shared/protocol.c @@ -91,12 +91,12 @@ void send_data(int file_descriptor, Data *data) { } Data *receive_data(int file_descriptor) { - size_t size = 0; + unsigned long long size = 0; receive_n_data(file_descriptor, &size, sizeof(unsigned long long)); - void *data = malloc(size); - receive_n_data(file_descriptor, data, size); + void *data = malloc((size_t)size); + receive_n_data(file_descriptor, data, (size_t)size); log_message(LOG_LEVEL_DEBUG, "Received %lld data", size); - return data_create(data, size); + return data_create(data, (size_t)size); } void send_int(int file_descriptor, int data) { diff --git a/tests/test_array_list.c b/tests/test_array_list.c index 03c11ac..d7bc9ea 100644 --- a/tests/test_array_list.c +++ b/tests/test_array_list.c @@ -49,16 +49,4 @@ void test_array_list() { list->item_destroyer = test_destroyer; array_list_delete(list); EXPECT_EQ_INT(destroyer_calls, 106); - - // Test clear with NULL destroyer - list = array_list_create(NULL); - int a = 1, b = 2; - array_list_add(list, &a); - array_list_add(list, &b); - EXPECT_EQ_INT(list->size, 2); - array_list_clear(list); - EXPECT_EQ_INT(list->size, 0); - EXPECT_NULL(list->items[0]); - EXPECT_NULL(list->items[1]); - array_list_delete(list); } diff --git a/tests/test_compression.c b/tests/test_compression.c index e793941..46ab006 100644 --- a/tests/test_compression.c +++ b/tests/test_compression.c @@ -86,7 +86,10 @@ static void test_chunk_compress_decompress_roundtrip() { Data *compressed = chunk_compress(chunk, 3, false); EXPECT_NOT_NULL(compressed); - Chunk *decompressed_chunk = chunk_decompress(compressed, false); + Data *decompressed_data = data_decompress(compressed); + EXPECT_NOT_NULL(decompressed_data); + + Chunk *decompressed_chunk = chunk_deserialize(decompressed_data, false); EXPECT_NOT_NULL(decompressed_chunk); EXPECT_EQ_INT(decompressed_chunk->element_count, 2); @@ -100,6 +103,7 @@ static void test_chunk_compress_decompress_roundtrip() { chunk_destroy(chunk); data_destroy(compressed); + data_destroy(decompressed_data); chunk_destroy(decompressed_chunk); unlink(path1); diff --git a/tests/test_config.c b/tests/test_config.c index 2b22e1e..266fd38 100644 --- a/tests/test_config.c +++ b/tests/test_config.c @@ -8,7 +8,7 @@ static void test_config_lifecycle() { Config *cfg = config_create(str_dup("1.0"), str_dup("/src"), str_dup("/dst"), - true, true, false, false, false, 1, 4, false); + true, true, false, false, false, 1, false); EXPECT_NOT_NULL(cfg); EXPECT_EQ_STR(cfg->version, "1.0"); EXPECT_EQ_STR(cfg->send_directory, "/src"); @@ -17,7 +17,6 @@ static void test_config_lifecycle() { EXPECT_TRUE(cfg->use_multithreading); EXPECT_FALSE(cfg->use_chunk_serialization); EXPECT_FALSE(cfg->use_compression); - EXPECT_EQ_INT(cfg->num_connections, 4); EXPECT_EQ_INT(cfg->transport, TRANSPORT_TCP); EXPECT_NULL(cfg->ssh_destination); config_delete(cfg); @@ -25,7 +24,7 @@ static void test_config_lifecycle() { static void test_config_ssh_dest() { Config *cfg = config_create(str_dup("1.0"), str_dup("/src"), str_dup("user@host:/dst"), - true, false, false, false, false, 1, 4, false); + true, false, false, false, false, 1, false); EXPECT_NOT_NULL(cfg); EXPECT_EQ_INT(cfg->transport, TRANSPORT_TCP); EXPECT_NULL(cfg->ssh_destination); @@ -40,7 +39,7 @@ static void test_config_ssh_dest() { static void test_config_ssh_dest_local_path() { Config *cfg = config_create(str_dup("1.0"), str_dup("/src"), str_dup("/local/path"), - true, false, false, false, false, 1, 4, false); + true, false, false, false, false, 1, false); config_parse_ssh_dest(cfg); EXPECT_EQ_INT(cfg->transport, TRANSPORT_TCP); EXPECT_NULL(cfg->ssh_destination); @@ -50,7 +49,7 @@ static void test_config_ssh_dest_local_path() { static void test_config_ssh_dest_no_user() { Config *cfg = config_create(str_dup("1.0"), str_dup("/src"), str_dup("host:/remote"), - true, false, false, false, false, 1, 4, false); + true, false, false, false, false, 1, false); config_parse_ssh_dest(cfg); EXPECT_EQ_INT(cfg->transport, TRANSPORT_SSH); EXPECT_EQ_STR(cfg->ssh_destination, "host:/remote"); @@ -60,7 +59,7 @@ static void test_config_ssh_dest_no_user() { static void test_pipeline_sender_lifecycle() { Config *cfg = config_create(str_dup("2.0"), str_dup("/src2"), - str_dup("/dst2"), false, false, true, true, false, 1, 8, false); + str_dup("/dst2"), false, false, true, true, false, 1, false); Queue *q1 = queue_create(5, NULL); Queue *q2 = queue_create(15, NULL); @@ -77,7 +76,7 @@ static void test_pipeline_sender_lifecycle() { static void test_pipeline_receiver_lifecycle() { Config *cfg = config_create(str_dup("3.0"), str_dup("/src3"), - str_dup("/dst3"), true, true, true, true, false, 1, 2, false); + str_dup("/dst3"), true, true, true, true, false, 1, false); Queue *q = queue_create(20, NULL); PipelineContextReceiver *pcr = pipeline_context_receiver_create(cfg, q, 42); -- 2.52.0