fix: address review #69 - STATUS_ERROR sends, metadata_receive error handling, extract incremental_check helper

src/client/client_send.c

@@ -18,6 +18,27 @@
 #include <threads.h>
 #include <time.h>
 
+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;
+        int rc = incremental_check(client, chunk->items[i]);
-        if (!send_str(client->file_descriptor, chunk->items[i]->path)) return -1;
+        if (rc < 0) return -1;
-        unsigned long long fsize = chunk->items[i]->data->size;
+        if (rc > 0) continue;
-        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; }
         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;
+        int rc = incremental_check(client, chunk->items[i]);
-        if (!send_str(client->file_descriptor, chunk->items[i]->path)) return -1;
+        if (rc < 0) return -1;
-        unsigned long long fsize = chunk->items[i]->data->size;
+        if (rc > 0) continue;
-        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; }
         if (!file_send_single_calls_no_path(chunk->items[i], client->file_descriptor,
                                        config->use_metadata,
                                        config->use_compression ? config->compression_level : 0))

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) {

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) {

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;
 }
 

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

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) {