refactor: quality cleanup — table-driven CLI, file.c split, scanner helpers, unified error reporting #216

Merged
TapTap merged 7 commits from refactor/quality-cleanup into dev 2026-09-01 20:39:31 +02:00
Owner

Quality refactor following a codebase-wide scan (follow-up to #212).

Changes

  • Dead API removal: public array_list_extend -> static; legacy 16-param parallel_scanner_create deleted (test migrated to parallel_scanner_create_with_options)
  • Naming: to_disk -> file_write_to_disk, is_remote_dest -> config_is_remote_dest
  • client_cli.c: table-driven option parsing (OPTION_TABLE + offsetof dispatch); extracted parse_ull_arg() (replaces 5 duplicated strtoull blocks) and config_add_pattern()
  • scanner.c: parallel_scanner_create_with_options split into parallel_scanner_init, batch_files, scan_root_directory, spawn_parallel_workers (232 -> ~40 lines)
  • file.c split: lifecycle-only file.c + file_send.c (client send) + file_receive.c (server receive/save/manifest); new file_types.h/file_send.h/file_receive.h, file.h remains umbrella header
  • send_files(): shared print_transfer_progress() for ST/MT paths; single cleanup exit path; fixes manifest leak on success without --delete
  • Error reporting: new log_perror(); all bare perror() and client fprintf(stderr, "Error:...") converted to the log module (routed to --log-file too)
  • Fixed double-newline perror calls; clang-format 18 applied

Verification

  • Unit: 25/25 pass (also under ASan); integration pytest: 39 passed, 10 skipped, 1 xpassed
  • STRICT_WARNINGS build clean; ASan build + tests clean
  • clang-format 18 clean; cppcheck findings pre-existing on dev (version diff)
Quality refactor following a codebase-wide scan (follow-up to #212). ## Changes - **Dead API removal**: public `array_list_extend` -> static; legacy 16-param `parallel_scanner_create` deleted (test migrated to `parallel_scanner_create_with_options`) - **Naming**: `to_disk` -> `file_write_to_disk`, `is_remote_dest` -> `config_is_remote_dest` - **client_cli.c**: table-driven option parsing (`OPTION_TABLE` + `offsetof` dispatch); extracted `parse_ull_arg()` (replaces 5 duplicated strtoull blocks) and `config_add_pattern()` - **scanner.c**: `parallel_scanner_create_with_options` split into `parallel_scanner_init`, `batch_files`, `scan_root_directory`, `spawn_parallel_workers` (232 -> ~40 lines) - **file.c split**: lifecycle-only `file.c` + `file_send.c` (client send) + `file_receive.c` (server receive/save/manifest); new `file_types.h`/`file_send.h`/`file_receive.h`, `file.h` remains umbrella header - **send_files()**: shared `print_transfer_progress()` for ST/MT paths; single cleanup exit path; fixes manifest leak on success without --delete - **Error reporting**: new `log_perror()`; all bare `perror()` and client `fprintf(stderr, "Error:...")` converted to the log module (routed to --log-file too) - Fixed double-newline `perror` calls; clang-format 18 applied ## Verification - Unit: 25/25 pass (also under ASan); integration pytest: 39 passed, 10 skipped, 1 xpassed - STRICT_WARNINGS build clean; ASan build + tests clean - clang-format 18 clean; cppcheck findings pre-existing on dev (version diff)
TapTap added 7 commits 2026-09-01 20:37:10 +02:00
- Delete unused public array_list_extend (made static)
- Delete legacy 16-parameter parallel_scanner_create wrapper; migrate test to parallel_scanner_create_with_options
- Rename to_disk -> file_write_to_disk and is_remote_dest -> config_is_remote_dest for module_action naming convention
- Remove stray newlines in perror calls (perror already appends one)
- Add OPTION_TABLE for options that map directly to Config fields
  (flag/string/pos-int/nonneg-int/ull kinds)
- Extract parse_ull_arg() replacing 5 duplicated strtoull blocks
- Extract config_add_pattern() replacing duplicated --exclude/--include
  append logic, also reused by read_patterns_from_file()
- parse_args() reduced from ~275 to ~160 lines
- parallel_scanner_init(): result queue + sync primitive setup with unwinding
- batch_files(): root-file chunk batching, reusable by other scan paths
- scan_root_directory()/scan_root_entry(): root-dir scanning
- spawn_parallel_workers(): worker thread creation with per-thread arg setup
Main function reduced from ~230 to ~40 lines
- file.c: File/FileMetadata lifecycle and local disk helpers (~110 lines)
- file_send.c: client-side send path (file_send_single_calls, file_send_sendfile)
- file_receive.c: server-side receive/save path (file_receive, receive_incremental_check,
  receive_manifest, file_save_to_disk)
- file_types.h holds shared struct definitions; file.h remains an umbrella header
  so existing includes are unaffected
Completes the transfer/protocol separation started in PR #212
- Extract print_transfer_progress() shared by single-threaded loop and
  the multithreaded progress thread
- Route all send_files() exits through a single send_fail cleanup path
- Fix pre-existing manifest leak on success without --delete
- Add log_perror() helper (context + strerror(errno)) to the log module
- Replace all bare perror() calls with log_perror() so errors are routed
  through the unified logger (stderr sink + optional --log-file sink)
- Convert fprintf(stderr, "Error:/Warning: ...") in client code to
  log_message(); raw fprintf kept only for progress/stats output
style: apply clang-format 18
CI / lint (pull_request) Successful in 23s
CI / sanitizers (address) (pull_request) Successful in 54s
CI / sanitizers (undefined) (pull_request) Successful in 54s
CI / fuzz-build (pull_request) Successful in 14s
CI / build-and-test (pull_request) Successful in 1m17s
CI / coverage (pull_request) Successful in 31s
CI / valgrind (pull_request) Successful in 33s
047a9a1906
TapTap merged commit 265f0c0c09 into dev 2026-09-01 20:39:31 +02:00
Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: TapTap/FastSync#216