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.
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.
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-sync2026-07-18 16:29:23 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
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.
PR Review
Verdict: PASS — Clean refactor, reduces code duplication
Files reviewed: 16 (includes incremental-sync changes + path variant merge)
Core Refactor
Merges
_no_pathvariants into the main functions by adding abool send_pathparameter:file_send_single_calls(file, fd, use_metadata, compression_level, send_path)— ifsend_path=false, skips sending the path (receiver already has it fromSTATUS_CHECK)file_send_sendfile(file, fd, use_metadata, send_path)— same pattern_no_pathvariants removedclient_send.cpasstrue(normal) orfalse(incremental) based on contextWhat looks good
send_pathbool is clear and self-documentingdata_to_sendpointer +compressed_datafor cleanup, no mutation offile->datacompressed_dataincremental_check()helper extractedSTATUS_ERRORsends on all error paths inmultiprocessing.cmetadata_receive()okparameter for I/O error detection--incremental+-srejection, auto-MMinor observations (non-blocking)
STATUS_CHECKadded to protocol enum andstatus_to_string()— correct.server.cSTATUS_CHECK handler duplicatesmultiprocessing.chandler logic (both do the same stat comparison + receive). This is inherent to the fork-based vs threaded architecture.Safe to merge.