From 9a26eb187e0231fe512263cae84d57fce4bd45a3 Mon Sep 17 00:00:00 2001 From: TapTap Date: Fri, 17 Jul 2026 10:06:27 +0200 Subject: [PATCH] fix: address review #69 - STATUS_ERROR sends, metadata_receive error handling, extract incremental_check helper --- src/client/client_send.c | 49 ++++++++++++++++++++---------------- src/server/server.c | 4 ++- src/shared/file.c | 4 ++- src/shared/metadata.c | 14 ++++++++--- src/shared/metadata.h | 2 +- src/shared/multiprocessing.c | 7 ++++-- 6 files changed, 49 insertions(+), 31 deletions(-) diff --git a/src/client/client_send.c b/src/client/client_send.c index c0ee3b2..093184e 100644 --- a/src/client/client_send.c +++ b/src/client/client_send.c @@ -18,6 +18,27 @@ #include #include +static int incremental_check(Client *client, File *file) { + if (!send_status(client->file_descriptor, STATUS_CHECK)) return -1; + if (!send_str(client->file_descriptor, file->path)) return -1; + unsigned long long fsize = file->data->size; + long long mtime = file->metadata ? file->metadata->mtime_sec : 0; + if (!send_n_data(client->file_descriptor, &fsize, sizeof(fsize))) return -1; + if (!send_n_data(client->file_descriptor, &mtime, sizeof(mtime))) return -1; + Status s; + if (!receive_status(client->file_descriptor, &s)) return -1; + if (s == STATUS_ERROR) { + log_message(LOG_LEVEL_ERROR, "Server reported error for file"); + return -1; + } + if (s == STATUS_OK) return 1; + if (s != STATUS_NEXT) { + log_message(LOG_LEVEL_ERROR, "Unexpected server status"); + return -1; + } + return 0; +} + int send_chunk(Client *client, Chunk *chunk, Config *config) { if (config->use_chunk_serialization) { if (!send_status(client->file_descriptor, STATUS_CHUNK)) return -1; @@ -33,17 +54,9 @@ int send_chunk(Client *client, Chunk *chunk, Config *config) { } else if (config->use_sendfile && !config->use_compression) { for (int i = 0; i < chunk->element_count; i++) { if (config->use_incremental) { - if (!send_status(client->file_descriptor, STATUS_CHECK)) return -1; - if (!send_str(client->file_descriptor, chunk->items[i]->path)) return -1; - unsigned long long fsize = chunk->items[i]->data->size; - long long mtime = chunk->items[i]->metadata ? chunk->items[i]->metadata->mtime_sec : 0; - if (!send_n_data(client->file_descriptor, &fsize, sizeof(fsize))) return -1; - if (!send_n_data(client->file_descriptor, &mtime, sizeof(mtime))) return -1; - Status s; - if (!receive_status(client->file_descriptor, &s)) return -1; - if (s == STATUS_ERROR) { log_message(LOG_LEVEL_ERROR, "Server reported error for file"); return -1; } - if (s == STATUS_OK) continue; - if (s != STATUS_NEXT) { log_message(LOG_LEVEL_ERROR, "Unexpected server status"); return -1; } + int rc = incremental_check(client, chunk->items[i]); + if (rc < 0) return -1; + if (rc > 0) continue; if (!file_send_sendfile_no_path(chunk->items[i], client->file_descriptor, config->use_metadata)) return -1; } else { @@ -55,17 +68,9 @@ int send_chunk(Client *client, Chunk *chunk, Config *config) { } else { for (int i = 0; i < chunk->element_count; i++) { if (config->use_incremental) { - if (!send_status(client->file_descriptor, STATUS_CHECK)) return -1; - if (!send_str(client->file_descriptor, chunk->items[i]->path)) return -1; - unsigned long long fsize = chunk->items[i]->data->size; - long long mtime = chunk->items[i]->metadata ? chunk->items[i]->metadata->mtime_sec : 0; - if (!send_n_data(client->file_descriptor, &fsize, sizeof(fsize))) return -1; - if (!send_n_data(client->file_descriptor, &mtime, sizeof(mtime))) return -1; - Status s; - if (!receive_status(client->file_descriptor, &s)) return -1; - if (s == STATUS_ERROR) { log_message(LOG_LEVEL_ERROR, "Server reported error for file"); return -1; } - if (s == STATUS_OK) continue; - if (s != STATUS_NEXT) { log_message(LOG_LEVEL_ERROR, "Unexpected server status"); return -1; } + int rc = incremental_check(client, chunk->items[i]); + if (rc < 0) return -1; + if (rc > 0) continue; if (!file_send_single_calls_no_path(chunk->items[i], client->file_descriptor, config->use_metadata, config->use_compression ? config->compression_level : 0)) diff --git a/src/server/server.c b/src/server/server.c index c4e528e..8ebd3e4 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -51,7 +51,9 @@ int receive_files(Config *config, int file_descriptor) { free(check_path); if (file == NULL) { send_status(file_descriptor, STATUS_ERROR); return -1; } if (config->use_metadata) { - file->metadata = metadata_receive(file_descriptor); + int meta_ok = 1; + file->metadata = metadata_receive(file_descriptor, &meta_ok); + if (!meta_ok) { file_destroy(file); send_status(file_descriptor, STATUS_ERROR); return -1; } } Data *file_data = receive_data(file_descriptor); if (file_data == NULL) { diff --git a/src/shared/file.c b/src/shared/file.c index fd403d7..94a1e6a 100644 --- a/src/shared/file.c +++ b/src/shared/file.c @@ -237,7 +237,9 @@ File *file_receive(Config *config, int file_descriptor) { free(path); if (file == NULL) return NULL; if (config->use_metadata) { - file->metadata = metadata_receive(file_descriptor); + 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) { diff --git a/src/shared/metadata.c b/src/shared/metadata.c index 41b4acd..6ca6c43 100644 --- a/src/shared/metadata.c +++ b/src/shared/metadata.c @@ -50,22 +50,28 @@ bool metadata_send(int file_descriptor, FileMetadata *m) { send_n_data(file_descriptor, &m->mtime_nsec, sizeof(long)); } -FileMetadata *metadata_receive(int file_descriptor) { +FileMetadata *metadata_receive(int file_descriptor, int *ok) { int present; - if (!receive_n_data(file_descriptor, &present, sizeof(int))) + if (!receive_n_data(file_descriptor, &present, sizeof(int))) { + if (ok) *ok = 0; return NULL; - if (!present) + } + if (!present) { + if (ok) *ok = 1; return NULL; + } FileMetadata *m = malloc(sizeof(FileMetadata)); - if (m == NULL) return NULL; + if (m == NULL) { if (ok) *ok = 0; return NULL; } if (!receive_n_data(file_descriptor, &m->mode, sizeof(mode_t)) || !receive_n_data(file_descriptor, &m->uid, sizeof(uid_t)) || !receive_n_data(file_descriptor, &m->gid, sizeof(gid_t)) || !receive_n_data(file_descriptor, &m->mtime_sec, sizeof(time_t)) || !receive_n_data(file_descriptor, &m->mtime_nsec, sizeof(long))) { free(m); + if (ok) *ok = 0; return NULL; } + if (ok) *ok = 1; return m; } diff --git a/src/shared/metadata.h b/src/shared/metadata.h index b4399e1..0107c56 100644 --- a/src/shared/metadata.h +++ b/src/shared/metadata.h @@ -10,7 +10,7 @@ void metadata_to_buf(char **buf, FileMetadata *m); FileMetadata *metadata_from_buf(char **buf); bool metadata_send(int file_descriptor, FileMetadata *m); -FileMetadata *metadata_receive(int file_descriptor); +FileMetadata *metadata_receive(int file_descriptor, int *ok); void file_restore_metadata(const char *path, FileMetadata *metadata); #endif diff --git a/src/shared/multiprocessing.c b/src/shared/multiprocessing.c index 5e912d0..63126ab 100644 --- a/src/shared/multiprocessing.c +++ b/src/shared/multiprocessing.c @@ -130,12 +130,13 @@ int receive_thread(void *pipeline_context) { while (status == STATUS_NEXT || status == STATUS_CHUNK || status == STATUS_CHECK) { if (status == STATUS_CHECK) { char *check_path = receive_str(file_descriptor); - if (check_path == NULL) return thrd_error; + if (check_path == NULL) { send_status(file_descriptor, STATUS_ERROR); return thrd_error; } unsigned long long check_size; long long check_mtime; if (!receive_n_data(file_descriptor, &check_size, sizeof(check_size)) || !receive_n_data(file_descriptor, &check_mtime, sizeof(check_mtime))) { free(check_path); + send_status(file_descriptor, STATUS_ERROR); return thrd_error; } char *full_path = path_cat(config->receive_root_directory, check_path); @@ -156,7 +157,9 @@ int receive_thread(void *pipeline_context) { free(check_path); if (file == NULL) { send_status(file_descriptor, STATUS_ERROR); return thrd_error; } if (config->use_metadata) { - file->metadata = metadata_receive(file_descriptor); + int meta_ok = 1; + file->metadata = metadata_receive(file_descriptor, &meta_ok); + if (!meta_ok) { file_destroy(file); send_status(file_descriptor, STATUS_ERROR); return thrd_error; } } Data *file_data = receive_data(file_descriptor); if (file_data == NULL) {