quality: send_chunk has deeply nested if-else with duplicated file-sending loops #46

New Issue

2026-07-20T14:59:20+02:00

TapTap commented

2026-07-20 14:59:20 +02:00

In src/client/client_send.c:128-181, the send_chunk function has three separate code paths with near-identical loops for sending individual files:

Path A (chunk serialization, lines 129-144): Compresses/serializes the entire chunk and sends as one block
Path B (sendfile, lines 145-161): Iterates over files using file_send_sendfile, with incremental check option
Path C (single calls, lines 162-179): Iterates over files using file_send_single_calls, with incremental check option

Paths B and C have identical structure:

for (int i = 0; i < chunk->element_count; i++) {
    if (config->use_incremental) {
        int rc = send_file_incremental(client, chunk->items[i], config, send_fn);
        if (rc == 1) continue;
        if (rc < 0) return -1;
    } else {
        if (!send_status(client->file_descriptor, STATUS_NEXT))
            return -1;
        if (!send_fn(...))
            return -1;
    }
}

The only difference is the send function pointer. This duplicated code should be unified. Additionally, the function has too many responsibilities (serialization dispatch + per-file sending + incremental logic).

Suggested Fix: Extract a shared helper function for per-file sending that accepts the send function as a parameter, eliminating the duplicated loop structure.

Severity: low

In src/client/client_send.c:128-181, the send_chunk function has three separate code paths with near-identical loops for sending individual files: 1. **Path A (chunk serialization, lines 129-144)**: Compresses/serializes the entire chunk and sends as one block 2. **Path B (sendfile, lines 145-161)**: Iterates over files using file_send_sendfile, with incremental check option 3. **Path C (single calls, lines 162-179)**: Iterates over files using file_send_single_calls, with incremental check option Paths B and C have identical structure: ```c for (int i = 0; i < chunk->element_count; i++) { if (config->use_incremental) { int rc = send_file_incremental(client, chunk->items[i], config, send_fn); if (rc == 1) continue; if (rc < 0) return -1; } else { if (!send_status(client->file_descriptor, STATUS_NEXT)) return -1; if (!send_fn(...)) return -1; } } ``` The only difference is the send function pointer. This duplicated code should be unified. Additionally, the function has too many responsibilities (serialization dispatch + per-file sending + incremental logic). Suggested Fix: Extract a shared helper function for per-file sending that accepts the send function as a parameter, eliminating the duplicated loop structure. Severity: low

TapTap referenced this issue from a commit

2026-07-20 17:09:16 +02:00

Fix 11 Gitea issues (#29, #30, #31, #35, #38, #41, #42, #43, #44, #45, #46)

TapTap referenced this issue

2026-07-20 17:13:19 +02:00

Fix 11 Gitea issues — bugs, quality, security, and refactoring #47

TapTap closed this issue

2026-07-20 17:51:46 +02:00

Sign in to join this conversation.