fix: address PR review - rolling adler32, extract helpers, configurable max file size
CI / build-and-test (push) Successful in 29s
CI / build-and-test (pull_request) Successful in 28s

- Fix rolling adler32: add missing -1 in s2 update formula (was producing
  wrong checksums, causing zero block matches)
- Fix file_send_sendfile signature to match typedef (add unused
  compression_level param)
- Extract receive_delta_file() from receive_incremental_check() to reduce
  nesting depth
- Extract send_file_incremental() helper in client_send.c
- Add delta_max_file_size to Config, serialized over wire
- Add --delta-max CLI flag
- Add test_large_file_delta (200KB) and extra delta_should_attempt cases
- Remove unused pos_in_block variable from delta_compute
This commit is contained in:
2026-07-19 02:02:28 +02:00
parent e16db56580
commit 598e4e7514
9 changed files with 339 additions and 261 deletions
+7
View File
@@ -39,6 +39,7 @@ static void print_usage(void) {
printf(" --incremental Skip files unchanged since last transfer\n");
printf(" --delta Delta transfer for changed files (requires --incremental)\n");
printf(" --delta-block <n> Delta block size in bytes (default: %d)\n", DELTA_BLOCK_SIZE_DEFAULT);
printf(" --delta-max <n> Max file size for delta transfer (default: %llu)\n", DELTA_MAX_FILE_SIZE);
printf(" -m Enable multithreading\n");
printf(" -s Enable chunk serialization\n");
printf(" -f Enable sendfile (TCP only, not with -c or -s)\n");
@@ -112,6 +113,12 @@ int main(int argc, char *argv[]) {
config->delta_block_size = (uint32_t)val;
else
fprintf(stderr, "Warning: --delta-block value %llu out of range, using default\n", val);
} else if (strcmp(argv[i], "--delta-max") == 0 && i + 1 < argc) {
unsigned long long val = strtoull(argv[++i], NULL, 10);
if (val >= DELTA_MIN_FILE_SIZE)
config->delta_max_file_size = val;
else
fprintf(stderr, "Warning: --delta-max value %llu too small, using default\n", val);
} else if (strcmp(argv[i], "-c") == 0 || strcmp(argv[i], "-z") == 0) {
config->use_compression = true;
log_message(LOG_LEVEL_INFO, "Enabled Compression");
+31 -32
View File
@@ -87,6 +87,28 @@ static int send_delta(Client *client, File *file, DeltaSignature *sig,
return ok ? 0 : -1;
}
typedef bool (*file_send_fn)(File *, int, bool, int, bool);
static int send_file_incremental(Client *client, File *file, Config *config,
file_send_fn send_fn) {
DeltaSignature *sig = NULL;
int rc = incremental_check(client, file, &sig);
if (rc < 0) { delta_signature_destroy(sig); return -1; }
if (rc == 1) { delta_signature_destroy(sig); return 1; }
if (rc == 2 && config->use_delta) {
int drc = send_delta(client, file, sig, config);
delta_signature_destroy(sig);
if (drc == 0) return 0;
if (drc < 0) return -1;
} else {
delta_signature_destroy(sig);
}
if (!send_fn(file, client->file_descriptor, config->use_metadata,
config->use_compression ? config->compression_level : 0, false))
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;
@@ -102,46 +124,23 @@ 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) {
DeltaSignature *sig = NULL;
int rc = incremental_check(client, chunk->items[i], &sig);
if (rc < 0) { delta_signature_destroy(sig); return -1; }
if (rc == 1) { delta_signature_destroy(sig); continue; }
if (rc == 2 && config->use_delta) {
int drc = send_delta(client, chunk->items[i], sig, config);
delta_signature_destroy(sig);
if (drc == 0) continue;
if (drc < 0) return -1;
} else {
delta_signature_destroy(sig);
}
if (!file_send_sendfile(chunk->items[i], client->file_descriptor, config->use_metadata, false))
return -1;
int rc = send_file_incremental(client, chunk->items[i], config,
(file_send_fn)file_send_sendfile);
if (rc == 1) continue;
if (rc < 0) return -1;
} else {
if (!send_status(client->file_descriptor, STATUS_NEXT)) return -1;
if (!file_send_sendfile(chunk->items[i], client->file_descriptor, config->use_metadata, true))
if (!file_send_sendfile(chunk->items[i], client->file_descriptor, config->use_metadata, 0, true))
return -1;
}
}
} else {
for (int i = 0; i < chunk->element_count; i++) {
if (config->use_incremental) {
DeltaSignature *sig = NULL;
int rc = incremental_check(client, chunk->items[i], &sig);
if (rc < 0) { delta_signature_destroy(sig); return -1; }
if (rc == 1) { delta_signature_destroy(sig); continue; }
if (rc == 2 && config->use_delta) {
int drc = send_delta(client, chunk->items[i], sig, config);
delta_signature_destroy(sig);
if (drc == 0) continue;
if (drc < 0) return -1;
} else {
delta_signature_destroy(sig);
}
if (!file_send_single_calls(chunk->items[i], client->file_descriptor,
config->use_metadata,
config->use_compression ? config->compression_level : 0,
false))
return -1;
int rc = send_file_incremental(client, chunk->items[i], config,
(file_send_fn)file_send_single_calls);
if (rc == 1) continue;
if (rc < 0) return -1;
} else {
if (!send_status(client->file_descriptor, STATUS_NEXT)) return -1;
if (!file_send_single_calls(chunk->items[i], client->file_descriptor,
+3
View File
@@ -42,6 +42,7 @@ Config *config_create(char *version, char *send_directory,
config->use_incremental = false;
config->use_delta = false;
config->delta_block_size = DELTA_BLOCK_SIZE_DEFAULT;
config->delta_max_file_size = DELTA_MAX_FILE_SIZE;
config->use_tls = false;
config->tls_cert = NULL;
config->tls_key = NULL;
@@ -103,6 +104,7 @@ bool config_send(int file_descriptor, Config *config) {
if (!send_int(file_descriptor, config->use_incremental)) return false;
if (!send_int(file_descriptor, config->use_delta)) return false;
if (!send_int(file_descriptor, (int)config->delta_block_size)) return false;
if (!send_n_data(file_descriptor, &config->delta_max_file_size, sizeof(unsigned long long))) return false;
Status status;
if (!receive_status(file_descriptor, &status)) return false;
if (status != STATUS_OK) {
@@ -154,6 +156,7 @@ Config *config_receive(int file_descriptor) {
config->use_delta = tmp;
if (!receive_int(file_descriptor, &tmp)) goto error;
config->delta_block_size = (uint32_t)tmp;
if (!receive_n_data(file_descriptor, &config->delta_max_file_size, sizeof(unsigned long long))) goto error;
config->show_progress = false;
config->dry_run = false;
config->ssh_port = 22;
+1
View File
@@ -36,6 +36,7 @@ typedef struct Config {
bool use_incremental;
bool use_delta;
uint32_t delta_block_size;
unsigned long long delta_max_file_size;
bool use_tls;
char *tls_cert;
char *tls_key;
+83 -47
View File
@@ -128,6 +128,33 @@ void delta_signature_destroy(DeltaSignature *sig) {
free(sig);
}
static bool ensure_capacity(DeltaInstruction **instrs, uint32_t *capacity,
uint32_t count) {
if (count < *capacity) return true;
uint32_t new_cap = *capacity * 2;
DeltaInstruction *tmp = realloc(*instrs, new_cap * sizeof(DeltaInstruction));
if (!tmp) return false;
*instrs = tmp;
*capacity = new_cap;
return true;
}
static bool flush_literal(DeltaInstruction **instrs, uint32_t *capacity,
uint32_t *count, const uint8_t *data,
uint64_t start, uint64_t end) {
if (start >= end) return true;
uint32_t lit_len = (uint32_t)(end - start);
if (!ensure_capacity(instrs, capacity, *count)) return false;
uint8_t *lit_data = malloc(lit_len);
if (!lit_data) return false;
memcpy(lit_data, data + start, lit_len);
(*instrs)[*count].type = DELTA_INSTR_LITERAL;
(*instrs)[*count].literal.data = lit_data;
(*instrs)[*count].literal.length = lit_len;
(*count)++;
return true;
}
Delta *delta_compute(const void *new_file_data, uint64_t new_file_size,
const DeltaSignature *sig, uint32_t block_size) {
if (!new_file_data || !sig || new_file_size == 0 || block_size == 0)
@@ -144,49 +171,67 @@ Delta *delta_compute(const void *new_file_data, uint64_t new_file_size,
bool has_literal = false;
uint64_t i = 0;
uint32_t s1 = 1, s2 = 0;
bool rolling_valid = false;
while (i < new_file_size) {
uint32_t window_len = (uint32_t)((new_file_size - i < block_size)
? (new_file_size - i)
: block_size);
uint32_t adler = delta_adler32(new_data + i, window_len);
uint32_t xxh = delta_xxhash32(new_data + i, window_len);
bool full_window = (window_len == block_size);
uint32_t adler;
if (rolling_valid && full_window) {
uint8_t old_byte = new_data[i - 1];
uint8_t new_byte = new_data[i + block_size - 1];
s1 = (s1 + DELTA_ADLER32_MODULUS - old_byte + new_byte) %
DELTA_ADLER32_MODULUS;
s2 = (s2 + DELTA_ADLER32_MODULUS -
(uint32_t)((uint64_t)block_size * old_byte % DELTA_ADLER32_MODULUS) +
s1 - 1) %
DELTA_ADLER32_MODULUS;
adler = (s2 << 16) | s1;
} else {
s1 = 1;
s2 = 0;
for (uint32_t k = 0; k < window_len; k++) {
s1 = (s1 + new_data[i + k]) % DELTA_ADLER32_MODULUS;
s2 = (s2 + s1) % DELTA_ADLER32_MODULUS;
}
adler = (s2 << 16) | s1;
rolling_valid = full_window;
}
bool matched = false;
for (uint32_t j = 0; j < sig->block_count; j++) {
if (adler == sig->blocks[j].adler32 && xxh == sig->blocks[j].xxhash) {
if (has_literal) {
uint32_t lit_len = (uint32_t)(i - literal_start);
if (count == capacity) {
capacity *= 2;
DeltaInstruction *tmp = realloc(instrs, capacity * sizeof(DeltaInstruction));
if (!tmp) { free(instrs); return NULL; }
instrs = tmp;
if (adler == sig->blocks[j].adler32 && full_window) {
uint32_t xxh = delta_xxhash32(new_data + i, window_len);
if (xxh == sig->blocks[j].xxhash) {
if (has_literal) {
if (!flush_literal(&instrs, &capacity, &count, new_data,
literal_start, i)) {
free(instrs);
return NULL;
}
has_literal = false;
}
uint8_t *lit_data = malloc(lit_len);
if (!lit_data) { free(instrs); return NULL; }
memcpy(lit_data, new_data + literal_start, lit_len);
instrs[count].type = DELTA_INSTR_LITERAL;
instrs[count].literal.data = lit_data;
instrs[count].literal.length = lit_len;
if (!ensure_capacity(&instrs, &capacity, count)) {
free(instrs);
return NULL;
}
instrs[count].type = DELTA_INSTR_BLOCK_MATCH;
instrs[count].match.block_index = j;
instrs[count].match.block_offset = 0;
instrs[count].match.length = window_len;
count++;
has_literal = false;
}
if (count == capacity) {
capacity *= 2;
DeltaInstruction *tmp = realloc(instrs, capacity * sizeof(DeltaInstruction));
if (!tmp) { free(instrs); return NULL; }
instrs = tmp;
i += window_len;
rolling_valid = false;
matched = true;
break;
}
instrs[count].type = DELTA_INSTR_BLOCK_MATCH;
instrs[count].match.block_index = j;
instrs[count].match.block_offset = 0;
instrs[count].match.length = window_len;
count++;
i += window_len;
matched = true;
break;
}
}
@@ -200,20 +245,11 @@ Delta *delta_compute(const void *new_file_data, uint64_t new_file_size,
}
if (has_literal) {
uint32_t lit_len = (uint32_t)(new_file_size - literal_start);
if (count == capacity) {
capacity *= 2;
DeltaInstruction *tmp = realloc(instrs, capacity * sizeof(DeltaInstruction));
if (!tmp) { free(instrs); return NULL; }
instrs = tmp;
if (!flush_literal(&instrs, &capacity, &count, new_data,
literal_start, new_file_size)) {
free(instrs);
return NULL;
}
uint8_t *lit_data = malloc(lit_len);
if (!lit_data) { free(instrs); return NULL; }
memcpy(lit_data, new_data + literal_start, lit_len);
instrs[count].type = DELTA_INSTR_LITERAL;
instrs[count].literal.data = lit_data;
instrs[count].literal.length = lit_len;
count++;
}
Delta *delta = malloc(sizeof(Delta));
@@ -421,10 +457,10 @@ void delta_destroy(Delta *delta) {
free(delta);
}
bool delta_should_attempt(uint64_t old_size, uint64_t new_size) {
bool delta_should_attempt(uint64_t old_size, uint64_t new_size, uint64_t max_file_size) {
if (old_size < DELTA_MIN_FILE_SIZE || new_size < DELTA_MIN_FILE_SIZE)
return false;
if (old_size > DELTA_MAX_FILE_SIZE || new_size > DELTA_MAX_FILE_SIZE)
if (old_size > max_file_size || new_size > max_file_size)
return false;
double large = (old_size > new_size) ? (double)old_size : (double)new_size;
double small = (old_size > new_size) ? (double)new_size : (double)old_size;
+1 -1
View File
@@ -72,7 +72,7 @@ void *delta_apply(const void *old_data, uint64_t old_size, const Delta *delta,
uint32_t block_size);
void delta_destroy(Delta *delta);
bool delta_should_attempt(uint64_t old_size, uint64_t new_size);
bool delta_should_attempt(uint64_t old_size, uint64_t new_size, uint64_t max_file_size);
bool delta_is_worthwhile(const Delta *delta, uint64_t new_file_size);
uint32_t delta_adler32(const void *data, uint32_t len);
+155 -174
View File
@@ -133,6 +133,150 @@ bool file_save_to_disk(const char *root_directory, File *file) {
return ok;
}
static void *old_data_from_path(const char *full_path, unsigned long long old_size) {
void *data = malloc((size_t)old_size);
if (!data) return NULL;
FILE *fp = fopen(full_path, "rb");
if (!fp) { free(data); return NULL; }
size_t nread = fread(data, 1, (size_t)old_size, fp);
fclose(fp);
if (nread != (size_t)old_size) { free(data); return NULL; }
return data;
}
static File *receive_delta_file(int fd, Config *config, const char *check_path,
void *old_data, unsigned long long old_size) {
if (!old_data) return NULL;
DeltaSignature *sig = delta_signature_create(old_data, old_size,
config->delta_block_size);
if (!sig) { free(old_data); return NULL; }
Data *sig_data = delta_signature_serialize(sig);
if (!sig_data) { delta_signature_destroy(sig); free(old_data); return NULL; }
bool sig_sent = send_status(fd, STATUS_DELTA_SIGNATURE) &&
send_data(fd, sig_data);
data_destroy(sig_data);
if (!sig_sent) { delta_signature_destroy(sig); free(old_data); return NULL; }
Status resp;
if (!receive_status(fd, &resp)) {
delta_signature_destroy(sig);
free(old_data);
return NULL;
}
if (resp == STATUS_DELTA_DATA) {
Data *delta_data = receive_data(fd);
if (!delta_data) {
delta_signature_destroy(sig);
free(old_data);
send_status(fd, STATUS_ERROR);
return NULL;
}
Data *raw_delta = delta_data;
if (config->use_compression) {
raw_delta = data_decompress(delta_data);
data_destroy(delta_data);
if (!raw_delta) {
free(old_data);
delta_signature_destroy(sig);
send_status(fd, STATUS_ERROR);
return NULL;
}
}
Delta *delta = delta_deserialize(raw_delta);
data_destroy(raw_delta);
if (!delta) {
free(old_data);
delta_signature_destroy(sig);
send_status(fd, STATUS_ERROR);
return NULL;
}
void *new_data = delta_apply(old_data, old_size, delta,
config->delta_block_size);
uint64_t new_size = delta->new_file_size;
delta_destroy(delta);
if (!new_data) {
free(old_data);
delta_signature_destroy(sig);
send_status(fd, STATUS_ERROR);
return NULL;
}
File *file = file_create(check_path);
if (!file) {
free(new_data);
free(old_data);
delta_signature_destroy(sig);
send_status(fd, STATUS_ERROR);
return NULL;
}
if (config->use_metadata) {
int meta_ok = 1;
file->metadata = metadata_receive(fd, &meta_ok);
if (!meta_ok) {
file_destroy(file);
free(new_data);
free(old_data);
delta_signature_destroy(sig);
send_status(fd, STATUS_ERROR);
return NULL;
}
}
data_destroy(file->data);
file->data = data_create(new_data, (size_t)new_size);
free(old_data);
delta_signature_destroy(sig);
return file;
}
if (resp == STATUS_NEXT) {
delta_signature_destroy(sig);
free(old_data);
File *file = file_create(check_path);
if (!file) { send_status(fd, STATUS_ERROR); return NULL; }
if (config->use_metadata) {
int meta_ok = 1;
file->metadata = metadata_receive(fd, &meta_ok);
if (!meta_ok) { file_destroy(file); send_status(fd, STATUS_ERROR); return NULL; }
}
Data *file_data = receive_data(fd);
if (file_data == NULL) {
file_destroy(file);
send_status(fd, STATUS_ERROR);
return NULL;
}
if (config->use_compression) {
Data *uncompressed = data_decompress(file_data);
data_destroy(file_data);
if (uncompressed == NULL) { file_destroy(file); send_status(fd, STATUS_ERROR); return NULL; }
file_data = uncompressed;
}
data_destroy(file->data);
file->data = file_data;
return file;
}
delta_signature_destroy(sig);
free(old_data);
return NULL;
}
File *receive_incremental_check(int fd, Config *config, bool *skipped) {
*skipped = false;
char *check_path = receive_str(fd);
@@ -165,182 +309,18 @@ File *receive_incremental_check(int fd, Config *config, bool *skipped) {
}
bool try_delta = config->use_delta && has_old_file &&
delta_should_attempt(old_size, check_size);
delta_should_attempt(old_size, check_size, config->delta_max_file_size);
if (try_delta) {
void *old_data = malloc((size_t)old_size);
if (!old_data) {
try_delta = false;
} else {
FILE *fp = fopen(full_path, "rb");
if (!fp) {
free(old_data);
try_delta = false;
} else {
size_t nread = fread(old_data, 1, (size_t)old_size, fp);
fclose(fp);
if (nread != (size_t)old_size) {
free(old_data);
try_delta = false;
}
}
}
if (try_delta) {
DeltaSignature *sig = delta_signature_create(old_data, old_size,
config->delta_block_size);
if (!sig) {
free(old_data);
try_delta = false;
} else {
Data *sig_data = delta_signature_serialize(sig);
if (!sig_data) {
delta_signature_destroy(sig);
free(old_data);
try_delta = false;
} else {
bool sig_sent = send_status(fd, STATUS_DELTA_SIGNATURE) &&
send_data(fd, sig_data);
data_destroy(sig_data);
if (!sig_sent) {
delta_signature_destroy(sig);
free(old_data);
try_delta = false;
} else {
Status resp;
if (!receive_status(fd, &resp)) {
delta_signature_destroy(sig);
free(old_data);
free(full_path);
free(check_path);
return NULL;
}
if (resp == STATUS_DELTA_DATA) {
Data *delta_data = receive_data(fd);
if (!delta_data) {
delta_signature_destroy(sig);
free(old_data);
send_status(fd, STATUS_ERROR);
free(full_path);
free(check_path);
return NULL;
}
Data *raw_delta = delta_data;
if (config->use_compression) {
raw_delta = data_decompress(delta_data);
data_destroy(delta_data);
if (!raw_delta) {
free(old_data);
delta_signature_destroy(sig);
send_status(fd, STATUS_ERROR);
free(full_path);
free(check_path);
return NULL;
}
}
Delta *delta = delta_deserialize(raw_delta);
data_destroy(raw_delta);
if (!delta) {
free(old_data);
delta_signature_destroy(sig);
send_status(fd, STATUS_ERROR);
free(full_path);
free(check_path);
return NULL;
}
void *new_data = delta_apply(old_data, old_size, delta,
config->delta_block_size);
uint64_t new_size = delta->new_file_size;
delta_destroy(delta);
if (!new_data) {
free(old_data);
delta_signature_destroy(sig);
send_status(fd, STATUS_ERROR);
free(full_path);
free(check_path);
return NULL;
}
File *file = file_create(check_path);
free(check_path);
free(full_path);
if (!file) {
free(new_data);
free(old_data);
delta_signature_destroy(sig);
send_status(fd, STATUS_ERROR);
return NULL;
}
if (config->use_metadata) {
int meta_ok = 1;
file->metadata = metadata_receive(fd, &meta_ok);
if (!meta_ok) {
file_destroy(file);
free(new_data);
free(old_data);
delta_signature_destroy(sig);
send_status(fd, STATUS_ERROR);
return NULL;
}
}
data_destroy(file->data);
file->data = data_create(new_data, (size_t)new_size);
free(old_data);
delta_signature_destroy(sig);
return file;
}
if (resp == STATUS_NEXT) {
delta_signature_destroy(sig);
free(old_data);
File *file = file_create(check_path);
free(check_path);
free(full_path);
if (!file) { send_status(fd, STATUS_ERROR); return NULL; }
if (config->use_metadata) {
int meta_ok = 1;
file->metadata = metadata_receive(fd, &meta_ok);
if (!meta_ok) { file_destroy(file); send_status(fd, STATUS_ERROR); return NULL; }
}
Data *file_data = receive_data(fd);
if (file_data == NULL) {
file_destroy(file);
send_status(fd, STATUS_ERROR);
return NULL;
}
if (config->use_compression) {
Data *uncompressed = data_decompress(file_data);
data_destroy(file_data);
if (uncompressed == NULL) { file_destroy(file); send_status(fd, STATUS_ERROR); return NULL; }
file_data = uncompressed;
}
data_destroy(file->data);
file->data = file_data;
return file;
}
delta_signature_destroy(sig);
free(old_data);
try_delta = false;
}
}
}
void *old_data = old_data_from_path(full_path, old_size);
File *delta_file = receive_delta_file(fd, config, check_path,
old_data, old_size);
if (delta_file) {
free(full_path);
free(check_path);
return delta_file;
}
try_delta = false;
}
if (!try_delta) {
@@ -406,7 +386,8 @@ bool to_disk(const char *path, const void *data, unsigned long long data_size) {
return true;
}
bool file_send_sendfile(File *file, int file_descriptor, bool use_metadata, bool send_path) {
bool file_send_sendfile(File *file, int file_descriptor, bool use_metadata, int compression_level, bool send_path) {
(void)compression_level;
if (send_path && !send_str(file_descriptor, file->path)) return false;
if (use_metadata && !metadata_send(file_descriptor, file->metadata)) return false;
+1 -1
View File
@@ -25,7 +25,7 @@ void file_destroy(void *item);
bool file_load_data(File *file);
File *file_receive(Config *config, int file_descriptor);
bool file_send_single_calls(File *file, int file_descriptor, bool use_metadata, int compression_level, bool send_path);
bool file_send_sendfile(File *file, int file_descriptor, bool use_metadata, bool send_path);
bool file_send_sendfile(File *file, int file_descriptor, bool use_metadata, int compression_level, bool send_path);
size_t file_content_to_buffer(File *file);
FileMetadata *file_metadata_create(struct stat *stats);
void file_metadata_destroy(void *metadata);