fix: address review findings — localtime_r, test_scanner cleanup

Reviewed-on: #86

src/client/client_cli.c

@@ -56,6 +56,7 @@ static void print_usage(void) {
   printf("  --key <path>        TLS private key file (PEM)\n");
   printf("  --ca <path>         TLS CA certificate file (PEM)\n");
   printf("  --help              Show this help\n");
+  printf("  -V, --version       Show version and exit\n");
 }
 
 int main(int argc, char* argv[]) {
@@ -80,6 +81,9 @@ int main(int argc, char* argv[]) {
     if (strcmp(argv[i], "--help") == 0) {
       print_usage();
       goto cleanup;
+    } else if (strcmp(argv[i], "-V") == 0 || strcmp(argv[i], "--version") == 0) {
+      printf("fastsync version %s\n", PROTOCOL_VERSION);
+      goto cleanup;
     } else if (strcmp(argv[i], "-a") == 0 || strcmp(argv[i], "--archive") == 0) {
       config->use_compression = true;
       config->use_multithreading = true;

src/server/server.c

@@ -135,6 +135,7 @@ static void print_server_usage(void) {
   printf("  --ca <path>         TLS CA certificate file (PEM)\n");
   printf("  -v, --verbose       Enable debug logging\n");
   printf("  --help              Show this help\n");
+  printf("  -V, --version       Show version and exit\n");
 }
 
 int main(int argc, char* argv[]) {
@@ -149,6 +150,9 @@ int main(int argc, char* argv[]) {
     if (strcmp(argv[i], "--help") == 0) {
       print_server_usage();
       return 0;
+    } else if (strcmp(argv[i], "-V") == 0 || strcmp(argv[i], "--version") == 0) {
+      printf("fastsync-server version %s\n", PROTOCOL_VERSION);
+      return 0;
     } else if (strcmp(argv[i], "--stdio") == 0) {
       io_set_fds(STDIN_FILENO, STDOUT_FILENO);
       handler(STDIN_FILENO);

src/shared/config.c

@@ -14,6 +14,8 @@ Config* config_create(char* version, char* send_directory, char* receive_directo
                       bool use_sendfile, unsigned long long chunk_size) {
 
   Config* config = malloc(sizeof(Config));
+  if (config == NULL)
+    return NULL;
   config->version = version;
   config->send_directory = send_directory;
   config->receive_root_directory = receive_directory;

src/shared/config.h

@@ -42,7 +42,7 @@ typedef struct Config {
   char* tls_ca;
 } Config;
 
-#define PROTOCOL_VERSION "1.3.0"
+#define PROTOCOL_VERSION "2.0.0"
 #define DEFAULT_CHUNK_SIZE (10 * 1024 * 1024)
 
 Config* config_create(char* version, char* send_directory, char* receive_directory,

src/shared/data.h

@@ -1,7 +1,7 @@
 #ifndef DATA_H
 #define DATA_H
 
-#include "stdlib.h"
+#include <stdlib.h>
 
 typedef struct {
   void* data;

src/shared/file.c

@@ -1,4 +1,5 @@
 #include <dirent.h>
+#include <errno.h>
 #include <fcntl.h>
 #include <libgen.h>
 #include <stddef.h>
@@ -34,7 +35,7 @@ File* file_create(const char* path) {
     return NULL;
   }
 
-  strcpy(file->path, path);
+  memcpy(file->path, path, path_len + 1);
   file->data = data_create_reserve(0);
   if (file->data == NULL) {
     free(file->path);

src/shared/log.c

@@ -14,7 +14,8 @@ void log_message(LogLevel log_level, char* format, ...) {
   if (log_level < current_log_level)
     return;
   time_t now = time(NULL);
-  const struct tm* t = localtime(&now);
+  struct tm result_buf;
+  const struct tm* t = localtime_r(&now, &result_buf);
 
   fprintf(stderr, "%04d-%02d-%02d %02d:%02d:%02d [%s]: ", t->tm_year + 1900, t->tm_mon + 1,
           t->tm_mday, t->tm_hour, t->tm_min, t->tm_sec, log_level_strings[log_level]);

src/shared/metadata.c

@@ -4,67 +4,103 @@
 #include "protocol.h"
 #include <errno.h>
 #include <fcntl.h>
+#include <stdint.h>
 #include <stdlib.h>
 #include <string.h>
 #include <sys/stat.h>
 #include <time.h>
 #include <unistd.h>
 
+/*
+ * Wire format serialization (protocol version 2.0.0+):
+ * All metadata fields are serialized as fixed-width integers (int32_t / int64_t)
+ * to ensure cross-platform binary compatiblity.  See metadata.h for the
+ * exact wire layout.
+ *
+ * Compile-time assertions verify that the native platform types fit within
+ * the chosen fixed-width representations.
+ */
+typedef char static_assert_mode_t_fits[(sizeof(mode_t) <= sizeof(int32_t)) ? 1 : -1];
+typedef char static_assert_uid_t_fits[(sizeof(uid_t) <= sizeof(int32_t)) ? 1 : -1];
+typedef char static_assert_gid_t_fits[(sizeof(gid_t) <= sizeof(int32_t)) ? 1 : -1];
+
 void metadata_to_buf(char** buf, const FileMetadata* m) {
-  int present = (m != NULL) ? 1 : 0;
-  memcpy(*buf, &present, sizeof(int));
-  *buf += sizeof(int);
+  int32_t present = (m != NULL) ? 1 : 0;
+  memcpy(*buf, &present, sizeof(present));
+  *buf += sizeof(present);
   if (m == NULL)
     return;
-  memcpy(*buf, &m->mode, sizeof(mode_t));
-  *buf += sizeof(mode_t);
-  memcpy(*buf, &m->uid, sizeof(uid_t));
-  *buf += sizeof(uid_t);
-  memcpy(*buf, &m->gid, sizeof(gid_t));
-  *buf += sizeof(gid_t);
-  memcpy(*buf, &m->mtime_sec, sizeof(time_t));
-  *buf += sizeof(time_t);
-  memcpy(*buf, &m->mtime_nsec, sizeof(long));
-  *buf += sizeof(long);
+  int32_t mode = (int32_t)m->mode;
+  memcpy(*buf, &mode, sizeof(mode));
+  *buf += sizeof(mode);
+  int32_t uid = (int32_t)m->uid;
+  memcpy(*buf, &uid, sizeof(uid));
+  *buf += sizeof(uid);
+  int32_t gid = (int32_t)m->gid;
+  memcpy(*buf, &gid, sizeof(gid));
+  *buf += sizeof(gid);
+  int64_t mtime_sec = (int64_t)m->mtime_sec;
+  memcpy(*buf, &mtime_sec, sizeof(mtime_sec));
+  *buf += sizeof(mtime_sec);
+  int64_t mtime_nsec = (int64_t)m->mtime_nsec;
+  memcpy(*buf, &mtime_nsec, sizeof(mtime_nsec));
+  *buf += sizeof(mtime_nsec);
 }
 
 FileMetadata* metadata_from_buf(char** buf) {
-  int present;
-  memcpy(&present, *buf, sizeof(int));
-  *buf += sizeof(int);
+  int32_t present;
+  memcpy(&present, *buf, sizeof(present));
+  *buf += sizeof(present);
   if (!present)
     return NULL;
   FileMetadata* m = malloc(sizeof(FileMetadata));
-  memcpy(&m->mode, *buf, sizeof(mode_t));
-  *buf += sizeof(mode_t);
-  memcpy(&m->uid, *buf, sizeof(uid_t));
-  *buf += sizeof(uid_t);
-  memcpy(&m->gid, *buf, sizeof(gid_t));
-  *buf += sizeof(gid_t);
-  memcpy(&m->mtime_sec, *buf, sizeof(time_t));
-  *buf += sizeof(time_t);
-  memcpy(&m->mtime_nsec, *buf, sizeof(long));
-  *buf += sizeof(long);
+  if (m == NULL)
+    return NULL;
+  int32_t mode;
+  memcpy(&mode, *buf, sizeof(mode));
+  *buf += sizeof(mode);
+  m->mode = (mode_t)mode;
+  int32_t uid;
+  memcpy(&uid, *buf, sizeof(uid));
+  *buf += sizeof(uid);
+  m->uid = (uid_t)uid;
+  int32_t gid;
+  memcpy(&gid, *buf, sizeof(gid));
+  *buf += sizeof(gid);
+  m->gid = (gid_t)gid;
+  int64_t mtime_sec;
+  memcpy(&mtime_sec, *buf, sizeof(mtime_sec));
+  *buf += sizeof(mtime_sec);
+  m->mtime_sec = (time_t)mtime_sec;
+  int64_t mtime_nsec;
+  memcpy(&mtime_nsec, *buf, sizeof(mtime_nsec));
+  *buf += sizeof(mtime_nsec);
+  m->mtime_nsec = (long)mtime_nsec;
   return m;
 }
 
 bool metadata_send(int file_descriptor, FileMetadata* m) {
   if (m == NULL) {
-    int zero = 0;
-    return send_n_data(file_descriptor, &zero, sizeof(int));
+    int32_t zero = 0;
+    return send_n_data(file_descriptor, &zero, sizeof(zero));
   }
-  int present = 1;
-  return send_n_data(file_descriptor, &present, sizeof(int)) &&
-         send_n_data(file_descriptor, &m->mode, sizeof(mode_t)) &&
-         send_n_data(file_descriptor, &m->uid, sizeof(uid_t)) &&
-         send_n_data(file_descriptor, &m->gid, sizeof(gid_t)) &&
-         send_n_data(file_descriptor, &m->mtime_sec, sizeof(time_t)) &&
-         send_n_data(file_descriptor, &m->mtime_nsec, sizeof(long));
+  int32_t present = 1;
+  int32_t mode = (int32_t)m->mode;
+  int32_t uid = (int32_t)m->uid;
+  int32_t gid = (int32_t)m->gid;
+  int64_t mtime_sec = (int64_t)m->mtime_sec;
+  int64_t mtime_nsec = (int64_t)m->mtime_nsec;
+  return send_n_data(file_descriptor, &present, sizeof(present)) &&
+         send_n_data(file_descriptor, &mode, sizeof(mode)) &&
+         send_n_data(file_descriptor, &uid, sizeof(uid)) &&
+         send_n_data(file_descriptor, &gid, sizeof(gid)) &&
+         send_n_data(file_descriptor, &mtime_sec, sizeof(mtime_sec)) &&
+         send_n_data(file_descriptor, &mtime_nsec, sizeof(mtime_nsec));
 }
 
 FileMetadata* metadata_receive(int file_descriptor, int* ok) {
-  int present;
-  if (!receive_n_data(file_descriptor, &present, sizeof(int))) {
+  int32_t present;
+  if (!receive_n_data(file_descriptor, &present, sizeof(present))) {
     if (ok)
       *ok = 0;
     return NULL;
@@ -80,16 +116,46 @@ FileMetadata* metadata_receive(int file_descriptor, int* 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))) {
+  int32_t mode;
+  if (!receive_n_data(file_descriptor, &mode, sizeof(mode))) {
     free(m);
     if (ok)
       *ok = 0;
     return NULL;
   }
+  m->mode = (mode_t)mode;
+  int32_t uid;
+  if (!receive_n_data(file_descriptor, &uid, sizeof(uid))) {
+    free(m);
+    if (ok)
+      *ok = 0;
+    return NULL;
+  }
+  m->uid = (uid_t)uid;
+  int32_t gid;
+  if (!receive_n_data(file_descriptor, &gid, sizeof(gid))) {
+    free(m);
+    if (ok)
+      *ok = 0;
+    return NULL;
+  }
+  m->gid = (gid_t)gid;
+  int64_t mtime_sec;
+  if (!receive_n_data(file_descriptor, &mtime_sec, sizeof(mtime_sec))) {
+    free(m);
+    if (ok)
+      *ok = 0;
+    return NULL;
+  }
+  m->mtime_sec = (time_t)mtime_sec;
+  int64_t mtime_nsec;
+  if (!receive_n_data(file_descriptor, &mtime_nsec, sizeof(mtime_nsec))) {
+    free(m);
+    if (ok)
+      *ok = 0;
+    return NULL;
+  }
+  m->mtime_nsec = (long)mtime_nsec;
   if (ok)
     *ok = 1;
   return m;

src/shared/metadata.h

@@ -3,10 +3,24 @@
 
 #include "file.h"
 #include <stdbool.h>
+#include <stdint.h>
 #include <sys/stat.h>
 
-#define FILE_METADATA_WIRE_SIZE                                                                    \
-  (sizeof(mode_t) + sizeof(uid_t) + sizeof(gid_t) + sizeof(time_t) + sizeof(long))
+/*
+ * Wire format (introduced in protocol version 2.0.0):
+ *   int32_t present
+ *   int32_t mode    (was mode_t, platform-dependent)
+ *   int32_t uid     (was uid_t,  platform-dependent)
+ *   int32_t gid     (was gid_t,  platform-dependent)
+ *   int64_t mtime_sec (was time_t, platform-dependent)
+ *   int64_t mtime_nsec (was long,  platform-dependent)
+ *
+ * Prior to 2.0.0 the wire format used the raw platform-dependent types,
+ * which broke compatiblity across different systems.  All fields are now
+ * serialized as fixed-width integers.
+ */
+
+#define FILE_METADATA_WIRE_SIZE (sizeof(int32_t) * 3 + sizeof(int64_t) * 2)
 
 void metadata_to_buf(char** buf, const FileMetadata* m);
 FileMetadata* metadata_from_buf(char** buf);

src/shared/utils.c

@@ -54,7 +54,7 @@ char* str_dup(const char* string) {
   if (string == NULL)
     return NULL;
   char* new_string = (char*)malloc(strlen(string) + 1);
-  strcpy(new_string, string);
+  memcpy(new_string, string, strlen(string) + 1);
   return new_string;
 }
 

tests/test_file.c

@@ -271,7 +271,7 @@ void test_file() {
   test_to_disk_basic();
   test_to_disk_creates_dirs();
   test_file_content_to_buffer();
-  if (!getenv("FASTSYNC_UNDER_VALGRIND")) {
+  if (!is_running_under_valgrind()) {
     // Fork tests are skipped under valgrind because the parent process runs
     // orders of magnitude slower than the child (parent is instrumented, child
     // is not), which causes pipe-based protocol handshake timeouts. The parent

tests/test_utils.h

@@ -2,9 +2,24 @@
 #define TEST_UTILS_H
 
 #include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
 #include <stdbool.h>
 
+// Detect if running under valgrind by checking /proc/self/maps for vgpreload.
+// This is used to skip fork-based tests that are incompatible with valgrind
+// (the instrumented parent runs too slowly, causing pipe timeouts).
+static inline bool is_running_under_valgrind(void) {
+  FILE* f = fopen("/proc/self/maps", "r");
+  if (!f)
+    return false;
+  char buf[4096];
+  size_t n = fread(buf, 1, sizeof(buf) - 1, f);
+  fclose(f);
+  buf[n] = '\0';
+  return strstr(buf, "vgpreload") != NULL;
+}
+
 // Global test suite status
 extern int tests_run;
 extern int tests_failed;