fix: address all remaining review warnings and style issues
CI / lint (push) Successful in 7s
CI / lint (pull_request) Successful in 7s
CI / build-and-test (push) Successful in 56s
CI / sanitizers (address) (push) Successful in 1m0s
CI / clang-tidy (push) Successful in 6s
CI / build-and-test (pull_request) Successful in 54s
CI / sanitizers (address) (pull_request) Successful in 59s
CI / clang-tidy (pull_request) Successful in 9s

- config.h: bump PROTOCOL_VERSION from "1.2.0" to "1.3.0" (WARNING-1)
- client_cli.c: fix realloc leak on exclude/include patterns (WARNING-2)
- file.c: document sendfile fallback as safety net only (WARNING-4)
- metadata.c: add warning log for utimensat failure (WARNING-5/STYLE-1)
- utils.c: fix is_dir_in_manifest false prefix match (WARNING-1)
- utils.c: fix double rmdir on recursive collapse, add errno.h (WARNING-2)
- client_send.c: guard unchecked send calls for manifest/finished (WARNING-3)
- client_send.c: send STATUS_NEXT on rc==2 without delta (WARNING-4)
This commit is contained in:
2026-07-20 17:48:03 +02:00
parent fd7e98cb25
commit 6a5a61b59f
6 changed files with 74 additions and 24 deletions
+16 -8
View File
@@ -92,15 +92,23 @@ int main(int argc, char* argv[]) {
} else if (strcmp(argv[i], "--delete") == 0) {
config->use_delete = true;
} else if (strcmp(argv[i], "--exclude") == 0 && i + 1 < argc) {
int idx = config->exclude_count++;
config->exclude_patterns =
realloc(config->exclude_patterns, config->exclude_count * sizeof(char*));
config->exclude_patterns[idx] = str_dup(argv[++i]);
char** tmp = realloc(config->exclude_patterns, (config->exclude_count + 1) * sizeof(char*));
if (!tmp) {
fprintf(stderr, "Error: memory allocation failed for --exclude\n");
exit_code = 1;
goto cleanup;
}
config->exclude_patterns = tmp;
config->exclude_patterns[config->exclude_count++] = str_dup(argv[++i]);
} else if (strcmp(argv[i], "--include") == 0 && i + 1 < argc) {
int idx = config->include_count++;
config->include_patterns =
realloc(config->include_patterns, config->include_count * sizeof(char*));
config->include_patterns[idx] = str_dup(argv[++i]);
char** tmp = realloc(config->include_patterns, (config->include_count + 1) * sizeof(char*));
if (!tmp) {
fprintf(stderr, "Error: memory allocation failed for --include\n");
exit_code = 1;
goto cleanup;
}
config->include_patterns = tmp;
config->include_patterns[config->include_count++] = str_dup(argv[++i]);
} else if (strcmp(argv[i], "--max-size") == 0 && i + 1 < argc) {
config->max_size = strtoull(argv[++i], NULL, 10);
} else if (strcmp(argv[i], "--min-size") == 0 && i + 1 < argc) {
+44 -10
View File
@@ -175,6 +175,13 @@ static int send_single_file(Client* client, File* file, Config* config, bool use
return -1;
} else {
delta_signature_destroy(sig);
// rc == 2 can happen if server sends STATUS_DELTA_SIGNATURE but
// use_delta is false on the client side. Send STATUS_NEXT to
// tell the server to proceed with the full file transfer.
if (rc == 2) {
if (!send_status(client->file_descriptor, STATUS_NEXT))
return -1;
}
}
if (!send_fn(file, client->file_descriptor, config->use_metadata, compression_level, false))
return -1;
@@ -254,17 +261,27 @@ static int send_chunks_multithreaded(void* pipeline_context) {
&context->condition_not_full_loader, &context->loader_done);
if (current_chunk == NULL) {
if (context->config->use_delete) {
send_status(client->file_descriptor, STATUS_MANIFEST);
send_int(client->file_descriptor, context->manifest->size);
for (int i = 0; i < context->manifest->size; i++)
send_str(client->file_descriptor, (char*)context->manifest->items[i]);
if (!send_status(client->file_descriptor, STATUS_MANIFEST))
goto send_fail;
if (!send_int(client->file_descriptor, context->manifest->size))
goto send_fail;
for (int i = 0; i < context->manifest->size; i++) {
if (!send_str(client->file_descriptor, (char*)context->manifest->items[i]))
goto send_fail;
}
}
send_status(client->file_descriptor, STATUS_FINISHED);
if (!send_status(client->file_descriptor, STATUS_FINISHED))
goto send_fail;
Status s;
int ok = receive_status(client->file_descriptor, &s) && s == STATUS_OK;
client_disconnect(client);
client_delete(client);
return ok ? thrd_success : thrd_error;
send_fail:
client_disconnect(client);
client_delete(client);
return thrd_error;
}
if (send_chunk(client, current_chunk, context->config) != 0) {
fprintf(stderr, "Error: unexpected error while sending chunk\n");
@@ -441,13 +458,24 @@ int send_files(Config* config) {
chunk_destroy(current_chunk);
}
if (config->use_delete) {
send_status(client->file_descriptor, STATUS_MANIFEST);
send_int(client->file_descriptor, manifest->size);
for (int i = 0; i < manifest->size; i++)
send_str(client->file_descriptor, (char*)manifest->items[i]);
if (!send_status(client->file_descriptor, STATUS_MANIFEST)) {
array_list_delete(manifest);
goto send_fail;
}
if (!send_int(client->file_descriptor, manifest->size)) {
array_list_delete(manifest);
goto send_fail;
}
for (int i = 0; i < manifest->size; i++) {
if (!send_str(client->file_descriptor, (char*)manifest->items[i])) {
array_list_delete(manifest);
goto send_fail;
}
}
array_list_delete(manifest);
}
send_status(client->file_descriptor, STATUS_FINISHED);
if (!send_status(client->file_descriptor, STATUS_FINISHED))
goto send_fail;
Status s;
int ok = receive_status(client->file_descriptor, &s) && s == STATUS_OK;
if (config->show_progress) {
@@ -459,6 +487,12 @@ int send_files(Config* config) {
client_disconnect(client);
client_delete(client);
return ok ? 0 : -1;
send_fail:
directory_scanner_destroy(scanner);
client_disconnect(client);
client_delete(client);
return -1;
}
int send_files_multithreaded(Config* config) {
+1 -1
View File
@@ -42,7 +42,7 @@ typedef struct Config {
char* tls_ca;
} Config;
#define PROTOCOL_VERSION "1.2.0"
#define PROTOCOL_VERSION "1.3.0"
#define DEFAULT_CHUNK_SIZE (10 * 1024 * 1024)
Config* config_create(char* version, char* send_directory, char* receive_directory,
+5
View File
@@ -448,6 +448,11 @@ bool file_send_sendfile(File* file, int file_descriptor, bool use_metadata, int
bool send_path) {
// sendfile is incompatible with compression (kernel zero-copy).
// If compression is requested, fall back to the regular send path.
// NOTE: This is a safety net only — callers must ensure compression_level == 0
// before calling file_send_sendfile. The fallback to file_send_single_calls
// preserves the send_path contract, but callers should not rely on it for
// correctness (the --sendfile flag is validated to be mutually exclusive with
// -c/--compress at the CLI layer).
if (compression_level > 0)
return file_send_single_calls(file, file_descriptor, use_metadata, compression_level,
send_path);
+2 -1
View File
@@ -107,5 +107,6 @@ void file_restore_metadata(const char* path, FileMetadata* metadata) {
times[0].tv_nsec = UTIME_OMIT;
times[1].tv_sec = metadata->mtime_sec;
times[1].tv_nsec = metadata->mtime_nsec;
utimensat(AT_FDCWD, path, times, 0);
if (utimensat(AT_FDCWD, path, times, 0) != 0)
log_message(LOG_LEVEL_WARNING, "Failed to set timestamps on %s: %s", path, strerror(errno));
}
+6 -4
View File
@@ -2,6 +2,7 @@
#include "array_list.h"
#include "libgen.h"
#include <dirent.h>
#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
@@ -86,8 +87,8 @@ static bool is_dir_in_manifest(const char* rel_path, ArrayList* manifest) {
size_t len = strlen(rel_path);
for (int i = 0; i < manifest->size; i++) {
const char* entry = (const char*)manifest->items[i];
// Check if entry starts with rel_path + '/'
if (strncmp(entry, rel_path, len) == 0 && entry[len] == '/')
// Check if entry starts with rel_path + '/' or matches exactly
if (strncmp(entry, rel_path, len) == 0 && (entry[len] == '/' || entry[len] == '\0'))
return true;
}
return false;
@@ -112,8 +113,9 @@ static void delete_extras_walk(const char* abs_path, const char* rel_path, Array
}
if (S_ISDIR(st.st_mode)) {
delete_extras_walk(child_abs, child_rel, manifest);
// After recursion, try to remove the subdirectory if it's now empty
if (rmdir(child_abs) != 0) {
// After recursion, try to remove the subdirectory if it's now empty.
// Ignore ENOENT: the recursive call may have already removed it.
if (rmdir(child_abs) != 0 && errno != ENOENT) {
all_removed = false;
}
} else {