refactor: merge sendfile and single_calls _no_path variants with send_path bool #19

2026-07-18T16:25:33+02:00

TapTap commented

2026-07-18 16:25:33 +02:00

Replace four send functions (file_send_sendfile, file_send_sendfile_no_path, file_send_single_calls, file_send_single_calls_no_path) with two functions that accept a send_path boolean parameter.

Removes ~50 lines of duplicated code. The only behavioral difference between each pair was whether send_str() was called to transmit the file path, so a single send_path bool covers both cases cleanly.

Replace four send functions (file_send_sendfile, file_send_sendfile_no_path, file_send_single_calls, file_send_single_calls_no_path) with two functions that accept a send_path boolean parameter. Removes ~50 lines of duplicated code. The only behavioral difference between each pair was whether send_str() was called to transmit the file path, so a single send_path bool covers both cases cleanly.

TapTap added 1 commit 2026-07-18 16:25:34 +02:00

refactor: merge sendfile and single_calls _no_path variants with send_path bool 1924fd5157

Replace four send functions (file_send_sendfile, file_send_sendfile_no_path,
file_send_single_calls, file_send_single_calls_no_path) with two functions
that accept a send_path boolean parameter. Removes ~50 lines of duplicated code.

TapTap commented

2026-07-18 16:26:59 +02:00

PR Review

Verdict: PASS — Clean refactor, reduces code duplication

Files reviewed: 16 (includes incremental-sync changes + path variant merge)

Core Refactor

Merges _no_path variants into the main functions by adding a bool send_path parameter:

file_send_single_calls(file, fd, use_metadata, compression_level, send_path) — if send_path=false, skips sending the path (receiver already has it from STATUS_CHECK)
file_send_sendfile(file, fd, use_metadata, send_path) — same pattern
_no_path variants removed
Callers in client_send.c pass true (normal) or false (incremental) based on context

What looks good

Eliminates ~50 lines of duplicated code
send_path bool is clear and self-documenting
Compressed data cleanup is correct: data_to_send pointer + compressed_data for cleanup, no mutation of file->data
All error paths properly destroy compressed_data
Incremental sync changes (from PR #14) are included and correct:
- incremental_check() helper extracted
- STATUS_ERROR sends on all error paths in multiprocessing.c
- metadata_receive() ok parameter for I/O error detection
- Protocol version bumped to 1.1.0
- --incremental + -s rejection, auto -M

Minor observations (non-blocking)

The branch includes all incremental-sync commits (rebased). If PR #14 merges first, this branch will need a rebase. If this merges first, PR #14 becomes redundant.
STATUS_CHECK added to protocol enum and status_to_string() — correct.
server.c STATUS_CHECK handler duplicates multiprocessing.c handler logic (both do the same stat comparison + receive). This is inherent to the fork-based vs threaded architecture.

Safe to merge.

## PR Review **Verdict: PASS** — Clean refactor, reduces code duplication **Files reviewed:** 16 (includes incremental-sync changes + path variant merge) --- ### Core Refactor Merges `_no_path` variants into the main functions by adding a `bool send_path` parameter: - `file_send_single_calls(file, fd, use_metadata, compression_level, send_path)` — if `send_path=false`, skips sending the path (receiver already has it from `STATUS_CHECK`) - `file_send_sendfile(file, fd, use_metadata, send_path)` — same pattern - `_no_path` variants removed - Callers in `client_send.c` pass `true` (normal) or `false` (incremental) based on context ### What looks good - Eliminates ~50 lines of duplicated code - `send_path` bool is clear and self-documenting - Compressed data cleanup is correct: `data_to_send` pointer + `compressed_data` for cleanup, no mutation of `file->data` - All error paths properly destroy `compressed_data` - Incremental sync changes (from PR #14) are included and correct: - `incremental_check()` helper extracted - `STATUS_ERROR` sends on all error paths in `multiprocessing.c` - `metadata_receive()` `ok` parameter for I/O error detection - Protocol version bumped to 1.1.0 - `--incremental` + `-s` rejection, auto `-M` ### Minor observations (non-blocking) - The branch includes all incremental-sync commits (rebased). If PR #14 merges first, this branch will need a rebase. If this merges first, PR #14 becomes redundant. - `STATUS_CHECK` added to protocol enum and `status_to_string()` — correct. - `server.c` STATUS_CHECK handler duplicates `multiprocessing.c` handler logic (both do the same stat comparison + receive). This is inherent to the fork-based vs threaded architecture. Safe to merge.

TapTap merged commit 2dcfad0321 into incremental-sync

2026-07-18 16:29:23 +02:00

TapTap deleted branch refactor/merge-path-variants

2026-07-18 16:29:25 +02:00

TapTap referenced this pull request

2026-07-18 16:54:42 +02:00

Incremental sync: --incremental to skip unchanged files #14

TapTap referenced this pull request

2026-07-18 16:57:05 +02:00

Incremental sync: --incremental to skip unchanged files #14

TapTap referenced this pull request

2026-07-18 17:01:48 +02:00