From 5fa0dc11e17ab7d8d229bfefc3c4118ff79600c7 Mon Sep 17 00:00:00 2001 From: TapTap Date: Sun, 5 Jul 2026 20:45:34 +0200 Subject: [PATCH] fix: address PR review comments - Rename receive_file_receive -> file_receive (comment 62) - Guard metadata receive with config->use_metadata in server.c (comment 63) - Add NULL check after malloc in file_load_data (comment 66) - Make chunk_serialize/chunk_deserialize metadata conditional on use_metadata param, thread through chunk_compress/chunk_decompress (comment 65) - Add missing stdbool.h include to chunk.h --- src/client/client.c | 4 ++-- src/server/server.c | 13 +++++++------ src/shared/chunk.c | 33 ++++++++++++++++++--------------- src/shared/chunk.h | 9 +++++---- src/shared/file.c | 7 ++++++- tests/test_chunk.c | 4 ++-- tests/test_compression.c | 4 ++-- 7 files changed, 42 insertions(+), 32 deletions(-) diff --git a/src/client/client.c b/src/client/client.c index c7cb3cd..11a5447 100644 --- a/src/client/client.c +++ b/src/client/client.c @@ -21,11 +21,11 @@ int send_chunk(Client *client, Chunk *chunk, Config *config) { send_status(client->file_descriptor, STATUS_CHUNK); Data *data; if (config->use_compression) { - data = chunk_compress(chunk, config->compression_level); + data = chunk_compress(chunk, config->compression_level, config->use_metadata); } else { for (int i = 0; i < chunk->element_count; i++) file_load_data(chunk->items[i]); - data = chunk_serialize(chunk); + data = chunk_serialize(chunk, config->use_metadata); } send_data(client->file_descriptor, data->data, data->size); data_destroy(data); diff --git a/src/server/server.c b/src/server/server.c index 923b3d7..0a5fb3b 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -12,11 +12,12 @@ #include #include -File *receive_file_receive(Config *config, int file_descriptor) { +File *file_receive(Config *config, int file_descriptor) { char *path = (char *)receive_str(file_descriptor); File *file = file_create(path); free(path); - file->metadata = file_receive_metadata(file_descriptor); + if (config->use_metadata) + file->metadata = file_receive_metadata(file_descriptor); Data *file_data = receive_data(file_descriptor); if (config->use_compression) { Data *file_data_uncompressed = data_decompress(file_data); @@ -37,7 +38,7 @@ static void receive_chunk_enqueue(int file_descriptor, Config *config, data_to_process = data_decompress(chunk_data); data_destroy(chunk_data); } - Chunk *chunk = chunk_deserialize(data_to_process); + Chunk *chunk = chunk_deserialize(data_to_process, config->use_metadata); data_destroy(data_to_process); for (int i = 0; i < chunk->element_count; i++) { @@ -63,7 +64,7 @@ int receive_thread(void *pipeline_context) { if (status == STATUS_CHUNK) { receive_chunk_enqueue(file_descriptor, config, context); } else { - File *file = receive_file_receive(config, file_descriptor); + File *file = file_receive(config, file_descriptor); queue_enqueue_multithreaded(context->queue, file, &context->mutex, &context->condition_not_empty, &context->condition_not_full); @@ -113,7 +114,7 @@ int receive_files(Config *config, int file_descriptor) { data_to_process = data_decompress(chunk_data); data_destroy(chunk_data); } - Chunk *chunk = chunk_deserialize(data_to_process); + Chunk *chunk = chunk_deserialize(data_to_process, config->use_metadata); data_destroy(data_to_process); for (int i = 0; i < chunk->element_count; i++) { @@ -126,7 +127,7 @@ int receive_files(Config *config, int file_descriptor) { } chunk_destroy(chunk); } else { - File *file = receive_file_receive(config, file_descriptor); + File *file = file_receive(config, file_descriptor); if (config->save_to_disk) { char *disk_path = path_cat(config->receive_root_directory, file->path); to_disk(disk_path, file->data->data, file->data->size); diff --git a/src/shared/chunk.c b/src/shared/chunk.c index 2ca73d7..6157ece 100644 --- a/src/shared/chunk.c +++ b/src/shared/chunk.c @@ -133,16 +133,16 @@ Data *chunk_format(Chunk *chunk) { return chunk_data_create(data, buffer_size); } -static unsigned long long per_file_serialize_size(File *file) { - return sizeof(size_t) + strlen(file->path) + sizeof(int) + - (file->metadata ? FILE_METADATA_WIRE_SIZE : 0) + +static unsigned long long per_file_serialize_size(File *file, bool use_metadata) { + return sizeof(size_t) + strlen(file->path) + + (use_metadata ? sizeof(int) + (file->metadata ? FILE_METADATA_WIRE_SIZE : 0) : 0) + sizeof(size_t) + file->data->size; } -Data *chunk_serialize(Chunk *chunk) { +Data *chunk_serialize(Chunk *chunk, bool use_metadata) { unsigned long long data_size = 0; for (int i = 0; i < chunk->element_count; i++) { - data_size += per_file_serialize_size(chunk->items[i]); + data_size += per_file_serialize_size(chunk->items[i], use_metadata); } Data *data = data_create_empty(data_size); if (data == NULL) { @@ -159,7 +159,8 @@ Data *chunk_serialize(Chunk *chunk) { memcpy(data_pointer, file->path, path_len); data_pointer += path_len; - metadata_to_buf(&data_pointer, file->metadata); + if (use_metadata) + metadata_to_buf(&data_pointer, file->metadata); size_t file_data_size = file->data->size; memcpy(data_pointer, &file_data_size, sizeof(size_t)); @@ -170,7 +171,7 @@ Data *chunk_serialize(Chunk *chunk) { return data; } -Chunk *chunk_deserialize(Data *data) { +Chunk *chunk_deserialize(Data *data, bool use_metadata) { ArrayList *files = array_list_create(file_destroy); char *data_pointer = data->data; size_t remaining_size = data->size; @@ -206,10 +207,12 @@ Chunk *chunk_deserialize(Data *data) { File *file = file_create(path); free(path); - file->metadata = metadata_from_buf(&data_pointer); - remaining_size -= sizeof(int); - if (file->metadata) - remaining_size -= FILE_METADATA_WIRE_SIZE; + if (use_metadata) { + file->metadata = metadata_from_buf(&data_pointer); + remaining_size -= sizeof(int); + if (file->metadata) + remaining_size -= FILE_METADATA_WIRE_SIZE; + } if (remaining_size < sizeof(size_t)) { log_message(LOG_LEVEL_ERROR, "Invalid chunk format: not enough data for data size"); @@ -252,16 +255,16 @@ Chunk *chunk_deserialize(Data *data) { return chunk; } -Data *chunk_compress(Chunk *chunk, int compression_level) { +Data *chunk_compress(Chunk *chunk, int compression_level, bool use_metadata) { log_message(LOG_LEVEL_DEBUG, "Starting to compress chunk"); - Data *serialized = chunk_serialize(chunk); + Data *serialized = chunk_serialize(chunk, use_metadata); Data *compressed = data_compress(serialized, compression_level); data_destroy(serialized); log_message(LOG_LEVEL_DEBUG, "Chunk successfully compressed"); return compressed; } -Chunk *chunk_decompress(Data *compressed_data) { +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) { @@ -269,7 +272,7 @@ Chunk *chunk_decompress(Data *compressed_data) { return NULL; } - Chunk *chunk = chunk_deserialize(uncompressed_data); + 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); diff --git a/src/shared/chunk.h b/src/shared/chunk.h index 41b62b8..b89896d 100644 --- a/src/shared/chunk.h +++ b/src/shared/chunk.h @@ -3,6 +3,7 @@ #include "data.h" #include "file.h" +#include #include #define DESIRED_CHUNK_SIZE 10 * 1024 * 1024 @@ -16,10 +17,10 @@ Chunk *chunk_create(File **items, int element_count); void chunk_destroy(void *chunk); void chunk_print(void *chunk); Data *chunk_format(Chunk *chunk); -Data *chunk_serialize(Chunk *chunk); -Chunk *chunk_deserialize(Data *data); -Data *chunk_compress(Chunk *chunk, int compression_level); -Chunk *chunk_decompress(Data *compressed_data); +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); Data *chunk_data_create(void *data, unsigned long long data_size); void chunk_data_delete(void *chunk); diff --git a/src/shared/file.c b/src/shared/file.c index 66128ad..4b274f3 100644 --- a/src/shared/file.c +++ b/src/shared/file.c @@ -73,8 +73,13 @@ void file_metadata_destroy(void *metadata) { void file_load_data(File *file) { if (file == NULL) return; - if (file->data->data == NULL) + if (file->data->data == NULL) { file->data->data = malloc(file->data->size); + if (file->data->data == NULL) { + perror("Could not allocate memory for file data"); + exit(EXIT_FAILURE); + } + } printf("%ld is file big", file->data->size); size_t bytes_read = file_content_to_buffer(file); if (bytes_read != file->data->size) { diff --git a/tests/test_chunk.c b/tests/test_chunk.c index a1bcb86..2d7ab8d 100644 --- a/tests/test_chunk.c +++ b/tests/test_chunk.c @@ -67,10 +67,10 @@ static void test_chunk_operations() { file_load_data(f2); // Test chunk_serialize / chunk_deserialize round-trip - Data *serialized = chunk_serialize(chunk); + Data *serialized = chunk_serialize(chunk, false); EXPECT_NOT_NULL(serialized); - Chunk *deserialized = chunk_deserialize(serialized); + Chunk *deserialized = chunk_deserialize(serialized, false); EXPECT_NOT_NULL(deserialized); EXPECT_EQ_INT(deserialized->element_count, 2); EXPECT_EQ_STR(deserialized->items[0]->path, path1); diff --git a/tests/test_compression.c b/tests/test_compression.c index b50f545..869b163 100644 --- a/tests/test_compression.c +++ b/tests/test_compression.c @@ -82,10 +82,10 @@ static void test_chunk_compress_decompress_roundtrip() { Chunk *chunk = chunk_create(files, 2); EXPECT_NOT_NULL(chunk); - Data *compressed = chunk_compress(chunk, 3); + Data *compressed = chunk_compress(chunk, 3, false); EXPECT_NOT_NULL(compressed); - Chunk *decompressed_chunk = chunk_decompress(compressed); + Chunk *decompressed_chunk = chunk_decompress(compressed, false); EXPECT_NOT_NULL(decompressed_chunk); EXPECT_EQ_INT(decompressed_chunk->element_count, 2);