* Fix bad-free heap corruption in ppathstatus and psyncer (#391)
Both ppathstatus.c and psyncer.c used bare free() via
ptree_for_each_element_call_safe to bulk-free tree nodes that were
allocated with pmem_malloc. pmem_malloc prepends a pmem_header_t to
every allocation, so the returned pointer is an interior pointer to the
underlying glibc chunk. Passing it to free() makes glibc read a garbage
size field from the pmem header and abort with "free(): invalid size".
This is the same class of bug fixed earlier in pintervaltree.c and
pfscrypto.c. The crash manifests reliably with larger files because
more sync-queue and path-status churn occurs, increasing the likelihood
that one of these bulk-free paths is hit while a non-empty tree exists.
Fix: add free_folder_tasks_node() (ppathstatus.c) and
free_synced_down_folder() (psyncer.c) helpers that call pmem_free, and
use them as the ptree_for_each_element_call_safe callback in all three
affected call sites.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Fix bad-free in pfs.c and ppagecache.c (pcl-26j)
Two more call sites passed bare free() as a callback to tree/list
traversal macros, but the nodes were allocated via pmem_malloc which
prepends a pmem_header_t. Freeing the data pointer directly skips the
header and corrupts the heap.
- pfs.c: ptree_for_each_element_call_safe on sectorsinlog used free()
on psync_sector_inlog_t; replaced with free_sector_inlog_node() that
calls pmem_free(PMEM_SUBSYS_OTHER, e).
- ppagecache.c: psync_list_for_each_element_call on request->ranges
used free() on psync_request_range_t; replaced with
free_request_range() that calls pmem_free(PMEM_SUBSYS_CACHE, range).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Add unit and smoke tests for pcl-26j bad-free (ppagecache, pfs, psyncer)
Unit test (test_pcl26j_free): exercises all four fixed call sites —
psync_request_range_t, synced_down_folder, folder_tasks_t, and
psync_sector_inlog_t — using --wrap=malloc/free to verify that
pmem_free() passes the header pointer to free(), not the data pointer.
Includes a harness self-check that confirms bare free(data_ptr) is
detected. Would have failed against pre-fix code.
Smoke test (smoke-test-large-read.sh): builds pcloudcc with ASAN,
starts the daemon, streams a large file (>=50 MB) from the FUSE mount
to /dev/null to force multi-range psync_request_range_t allocation and
teardown via psync_pagecache_free_request, then scans the ASAN log for
any bad-free reports.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* Add unit test for psync_task_free refcount fix (#377)
Adds tests/unit-tests/test_ptask_free.c to verify all code paths of the
psync_task_free fix from #377: single-owner free, last-ref destroy,
non-last-ref decrement, READY task signaling, and lock-before-refcnt
ordering. All 5 tests pass. Also adds compiled binary to .gitignore.
* Refactor test_ptask_free to link production code via --wrap
Extract psync_task_free + psync_task_destroy (and their static helpers
psync_task_dec_refcnt, psync_task_entry) from ptask.c into a new
separately-compilable unit pclsync/ptask_free.c. Add
pclsync/ptask_free_internal.h to expose the internal struct layout
(struct psync_task_manager_t_ / struct psync_task_t_) for test use
without pulling in ptask.c's heavyweight transitive dependencies.
Rewrite tests/unit-tests/test_ptask_free.c to:
- Include ptask_free_internal.h instead of duplicating structs inline
- Call the real psync_task_free() rather than a local replica
- Intercept pthread_mutex_lock/unlock and pmem_free via --wrap linker
flags to observe lock discipline and detect destroy invocations
Update the Makefile test_ptask_free target to link pclsync/ptask_free.c
and pass the required --wrap flags. Production build unchanged: ptask_free.o
is picked up automatically by the existing wildcard COBJ rule.
* Implement Tasks #3, #4, #5: tree tests, pfstasks tree layer, DB harness
Task #3 — Unit tests for ptree and pintervaltree
tests/unit-tests/test_ptree.c: 8 tests covering single-node insert,
in-order traversal after arbitrary and reverse inserts, BST lookup,
leaf/root/all-node deletion, and ptree_for_each visitation.
tests/unit-tests/test_pintervaltree.c: 18 tests covering single add,
non-overlapping, overlapping/adjacent/contained/spanning merges,
chain merge, remove middle split, remove exact/left/right/spanning,
cut_end, first_interval_containing_or_after, and free(NULL).
Task #4 — Extract pfstasks tree layer
pclsync/pfstasks_tree.h + pclsync/pfstasks_tree.c: pure tree layer
(zero psql calls) extracted from pfstasks.c — pfs_task_search_tree,
pfs_task_walk_tree (static helpers), pfs_task_insert_into_tree,
pfs_task_find_mkdir/rmdir/creat/unlink,
pfs_task_find_mkdir_by_folderid, pfs_task_find_creat_by_fileid.
pclsync/pfstasks.c: #includes pfstasks_tree.h; all moved functions
removed; all callers unchanged.
tests/unit-tests/test_pfstasks_tree.c: 13 tests using direct tree
construction (no DB) to verify find-by-name, taskid discrimination,
find-by-numeric-id, and empty-folder edge cases.
Task #5 — psql in-memory harness + pfstasks DB tests
tests/helpers/psql_test_helpers.h + .c: lightweight harness that
opens :memory: via sqlite3_open, enables PRAGMA foreign_keys=ON, and
applies the full PSYNC_DATABASE_STRUCTURE schema. Exposes
psql_test_db(), psql_test_exec(), psql_test_insert_fstask(),
psql_test_count_fstask/fstaskdepend(). No dependency on psql.c.
tests/unit-tests/test_pfstasks_db.c: 10 tests verifying schema
creation, fstask insertion/query, fstaskdepend insertion, CASCADE
DELETE propagation, FK enforcement, rmdir-blocking SQL pattern,
creat-after-unlink sequencing, and open/close idempotence.
All 11 new tests pass; production build clean.
* Fix P1 review findings in pfstasks_db test and helpers
1. Check psql_test_exec() return values in test_cascade_delete() and
test_creat_after_unlink() consistently with test_fstaskdepend_insert().
2. Remove dead dep_cnt variable and (void)dep_cnt suppressor from
test_creat_after_unlink().
3. Change SQLITE_STATIC → SQLITE_TRANSIENT for text1 binding in
psql_test_insert_fstask() to avoid dangling-pointer footgun on
future reuse.
* Fix ASAN/LSAN failures: ppath_home stack-use-after-scope + intentional leak
pclsync/ppath.c: Move buff[4096] to function scope in ppath_home() so
the pointer stored in dir via result->pw_dir remains live through the
putil_strdup(dir) call. Previously buff went out of scope at the if-block
close, causing a stack-use-after-scope ASAN report on every call that fell
through the getpwuid_r path.
tests/unit-tests/test_ptools_errptr.c: run_unfixed() intentionally leaks
errPtr to demonstrate the pre-fix bug. Wrap the allocation with
LSAN_DISABLE() / LSAN_ENABLE() so LSAN does not abort the process at exit
before stdio flushes, which was causing a non-zero exit code. The guard
uses nested #ifdef/__has_feature to remain compatible with both GCC
(__SANITIZE_ADDRESS__) and Clang (__has_feature(address_sanitizer))
without triggering "missing binary operator" errors on GCC.
* Implement Tasks #11 and #12: plocks stress test + pfsupload send tests
Task #11 — plocks.c stress test (test_plocks.c)
7 tests: basic rdlock/wrlock round-trip, recursive TLS counting (same
thread acquires rdlock N times; unlock only releases on final decrement),
upgrade under contention (N readers + towrlock; barrier-synchronized),
writer starvation prevention (sustained reader load; writer acquires
within 500ms), N-reader + M-writer counter-integrity stress test (ASAN),
and try-variant contention (trywrlock fails when another thread holds
rdlock). TSAN note documented: custom lock internals require ASAN-only
when ThreadSanitizer annotations are absent.
Task #12 — pfsupload send-function tests (pfsupload_send.c/h + test_pfsupload.c)
Extract psync_send_task_mkdir and psync_send_task_rmdir from pfsupload.c
into pclsync/pfsupload_send.c as non-static pfsupload_send_mkdir/rmdir.
Expose fsupload_task_t struct via pclsync/pfsupload_send.h. Add
__attribute__((weak)) get_urls() as an injectable URL seam for large-
upload paths. pfsupload.c updated to include pfsupload_send.h and use
the renamed functions in its dispatch table; pfsupload_send.o is
automatically picked up by the production wildcard build.
5 tests: mkdir (non-encrypted) command + folderid param, mkdir
(encrypted) key param present, rmdir command + sfolderid, API error path
(papi_send failure → -1), get_urls() weak override. Uses
--wrap=papi_send to intercept API calls and socketpair() to provide a
valid psock_t without real network I/O.
* P2 cleanup: comments, make_fake_api stack alloc, find_str_param fix
1. test_plocks.c: add comment to test_upgrade_under_contention clarifying
it verifies towrlock completion and holding_wrlock; notes that concurrent
exclusivity is covered by test_stress().
2. test_pfsupload.c / make_fake_api: replace static-local psock_t with
caller-supplied stack allocation (out parameter) to eliminate the
multiple-calls-per-test footgun.
3. test_pfsupload.c / find_str_param: replace ternary
`paramnamelen == strlen ? paramname : ""` with explicit length check +
strncmp, matching the cleaner pattern used in find_num_param.
4. Makefile: add comment next to -Wl,--wrap=papi_send noting it redirects
papi_send to __wrap_papi_send and is GNU ld only (not macOS Apple ld).
* Implement Tasks #19 and #20: ppagecache + pfs helper extraction
Task #19 — ppagecache.c decomposition (ppagecache_helpers.c/h)
ppagecache_compute_page_priority(usecnt): pure function returning the
LRU eviction tier (0–4) that matches the five pagecache_entry_cmp_*
sort comparators in ppagecache.c (thresholds 2/4/8/16).
ppagecache_verify_crc(data, size, stored_crc): wraps pcrc32c_compute
and compares; returns 0 on match, -1 on mismatch.
ppagecache_get_download_urls(fileid, hash, nout): __attribute__((weak))
URL-injection seam; default returns NULL (falls through to real API).
ppagecache.c updated to include ppagecache_helpers.h.
test_ppagecache.c: 10 tests covering tier boundary conditions (0/1/2/3/4
including UINT32_MAX), CRC match, single-bit flip, wrong stored CRC,
zero-length buffer, and weak URL override.
Task #20 — pfs.c helper extraction (pfs_helpers.c/h)
pfs_row_to_folder_stat(row, stbuf): converts psql folder row → struct
stat; uses pfs_task_get_folder_tasks_rdlocked for in-memory mtime.
pfs_row_to_file_stat(row, stbuf, flags): converts psql file row → stat;
encrypted path calls pfs_crpt_plain_size.
pfs_mkdir_to_folder_stat(mk, stbuf): converts in-memory mkdir task →
stat (no SQL).
pfs_apply_task_overlay(stbuf, folder, name, flags): applies pending
mkdir/rmdir/unlink/creat-new overlays from the in-memory task queue;
returns 1/2/-1/0.
pfs_stat_uid/gid: exported globals; pfs.c syncs them from myuid/mygid.
pfs.c updated to #include pfs_helpers.h and sync the uid/gid globals.
pfsfolder.c: pfs_fldr_resolve_path decorated __attribute__((weak)) so
tests can inject fake path resolution without a FUSE mount or psql.
test_pfs_helpers.c: 7 tests covering folder/file stat field correctness,
overlay NULL/mkdir/rmdir/no-match cases, and weak path override; uses
--wrap for pfs_task_get_folder_tasks_rdlocked, pfs_crpt_plain_size, and
ptimer_time to stay SQL/crypto/timer-free.
Production build clean; make check exits 0.
---------
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* Add unit test for psync_task_free refcount fix (#377)
Adds tests/unit-tests/test_ptask_free.c to verify all code paths of the
psync_task_free fix from #377: single-owner free, last-ref destroy,
non-last-ref decrement, READY task signaling, and lock-before-refcnt
ordering. All 5 tests pass. Also adds compiled binary to .gitignore.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Refactor test_ptask_free to link production code via --wrap
Extract psync_task_free + psync_task_destroy (and their static helpers
psync_task_dec_refcnt, psync_task_entry) from ptask.c into a new
separately-compilable unit pclsync/ptask_free.c. Add
pclsync/ptask_free_internal.h to expose the internal struct layout
(struct psync_task_manager_t_ / struct psync_task_t_) for test use
without pulling in ptask.c's heavyweight transitive dependencies.
Rewrite tests/unit-tests/test_ptask_free.c to:
- Include ptask_free_internal.h instead of duplicating structs inline
- Call the real psync_task_free() rather than a local replica
- Intercept pthread_mutex_lock/unlock and pmem_free via --wrap linker
flags to observe lock discipline and detect destroy invocations
Update the Makefile test_ptask_free target to link pclsync/ptask_free.c
and pass the required --wrap flags. Production build unchanged: ptask_free.o
is picked up automatically by the existing wildcard COBJ rule.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Implement Tasks #3, #4, #5: tree tests, pfstasks tree layer, DB harness
Task #3 — Unit tests for ptree and pintervaltree
tests/unit-tests/test_ptree.c: 8 tests covering single-node insert,
in-order traversal after arbitrary and reverse inserts, BST lookup,
leaf/root/all-node deletion, and ptree_for_each visitation.
tests/unit-tests/test_pintervaltree.c: 18 tests covering single add,
non-overlapping, overlapping/adjacent/contained/spanning merges,
chain merge, remove middle split, remove exact/left/right/spanning,
cut_end, first_interval_containing_or_after, and free(NULL).
Task #4 — Extract pfstasks tree layer
pclsync/pfstasks_tree.h + pclsync/pfstasks_tree.c: pure tree layer
(zero psql calls) extracted from pfstasks.c — pfs_task_search_tree,
pfs_task_walk_tree (static helpers), pfs_task_insert_into_tree,
pfs_task_find_mkdir/rmdir/creat/unlink,
pfs_task_find_mkdir_by_folderid, pfs_task_find_creat_by_fileid.
pclsync/pfstasks.c: #includes pfstasks_tree.h; all moved functions
removed; all callers unchanged.
tests/unit-tests/test_pfstasks_tree.c: 13 tests using direct tree
construction (no DB) to verify find-by-name, taskid discrimination,
find-by-numeric-id, and empty-folder edge cases.
Task #5 — psql in-memory harness + pfstasks DB tests
tests/helpers/psql_test_helpers.h + .c: lightweight harness that
opens :memory: via sqlite3_open, enables PRAGMA foreign_keys=ON, and
applies the full PSYNC_DATABASE_STRUCTURE schema. Exposes
psql_test_db(), psql_test_exec(), psql_test_insert_fstask(),
psql_test_count_fstask/fstaskdepend(). No dependency on psql.c.
tests/unit-tests/test_pfstasks_db.c: 10 tests verifying schema
creation, fstask insertion/query, fstaskdepend insertion, CASCADE
DELETE propagation, FK enforcement, rmdir-blocking SQL pattern,
creat-after-unlink sequencing, and open/close idempotence.
All 11 new tests pass; production build clean.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Fix P1 review findings in pfstasks_db test and helpers
1. Check psql_test_exec() return values in test_cascade_delete() and
test_creat_after_unlink() consistently with test_fstaskdepend_insert().
2. Remove dead dep_cnt variable and (void)dep_cnt suppressor from
test_creat_after_unlink().
3. Change SQLITE_STATIC → SQLITE_TRANSIENT for text1 binding in
psql_test_insert_fstask() to avoid dangling-pointer footgun on
future reuse.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Fix ASAN/LSAN failures: ppath_home stack-use-after-scope + intentional leak
pclsync/ppath.c: Move buff[4096] to function scope in ppath_home() so
the pointer stored in dir via result->pw_dir remains live through the
putil_strdup(dir) call. Previously buff went out of scope at the if-block
close, causing a stack-use-after-scope ASAN report on every call that fell
through the getpwuid_r path.
tests/unit-tests/test_ptools_errptr.c: run_unfixed() intentionally leaks
errPtr to demonstrate the pre-fix bug. Wrap the allocation with
LSAN_DISABLE() / LSAN_ENABLE() so LSAN does not abort the process at exit
before stdio flushes, which was causing a non-zero exit code. The guard
uses nested #ifdef/__has_feature to remain compatible with both GCC
(__SANITIZE_ADDRESS__) and Clang (__has_feature(address_sanitizer))
without triggering "missing binary operator" errors on GCC.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Implement Tasks #11 and #12: plocks stress test + pfsupload send tests
Task #11 — plocks.c stress test (test_plocks.c)
7 tests: basic rdlock/wrlock round-trip, recursive TLS counting (same
thread acquires rdlock N times; unlock only releases on final decrement),
upgrade under contention (N readers + towrlock; barrier-synchronized),
writer starvation prevention (sustained reader load; writer acquires
within 500ms), N-reader + M-writer counter-integrity stress test (ASAN),
and try-variant contention (trywrlock fails when another thread holds
rdlock). TSAN note documented: custom lock internals require ASAN-only
when ThreadSanitizer annotations are absent.
Task #12 — pfsupload send-function tests (pfsupload_send.c/h + test_pfsupload.c)
Extract psync_send_task_mkdir and psync_send_task_rmdir from pfsupload.c
into pclsync/pfsupload_send.c as non-static pfsupload_send_mkdir/rmdir.
Expose fsupload_task_t struct via pclsync/pfsupload_send.h. Add
__attribute__((weak)) get_urls() as an injectable URL seam for large-
upload paths. pfsupload.c updated to include pfsupload_send.h and use
the renamed functions in its dispatch table; pfsupload_send.o is
automatically picked up by the production wildcard build.
5 tests: mkdir (non-encrypted) command + folderid param, mkdir
(encrypted) key param present, rmdir command + sfolderid, API error path
(papi_send failure → -1), get_urls() weak override. Uses
--wrap=papi_send to intercept API calls and socketpair() to provide a
valid psock_t without real network I/O.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* P2 cleanup: comments, make_fake_api stack alloc, find_str_param fix
1. test_plocks.c: add comment to test_upgrade_under_contention clarifying
it verifies towrlock completion and holding_wrlock; notes that concurrent
exclusivity is covered by test_stress().
2. test_pfsupload.c / make_fake_api: replace static-local psock_t with
caller-supplied stack allocation (out parameter) to eliminate the
multiple-calls-per-test footgun.
3. test_pfsupload.c / find_str_param: replace ternary
`paramnamelen == strlen ? paramname : ""` with explicit length check +
strncmp, matching the cleaner pattern used in find_num_param.
4. Makefile: add comment next to -Wl,--wrap=papi_send noting it redirects
papi_send to __wrap_papi_send and is GNU ld only (not macOS Apple ld).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Implement Tasks #19 and #20: ppagecache + pfs helper extraction
Task #19 — ppagecache.c decomposition (ppagecache_helpers.c/h)
ppagecache_compute_page_priority(usecnt): pure function returning the
LRU eviction tier (0–4) that matches the five pagecache_entry_cmp_*
sort comparators in ppagecache.c (thresholds 2/4/8/16).
ppagecache_verify_crc(data, size, stored_crc): wraps pcrc32c_compute
and compares; returns 0 on match, -1 on mismatch.
ppagecache_get_download_urls(fileid, hash, nout): __attribute__((weak))
URL-injection seam; default returns NULL (falls through to real API).
ppagecache.c updated to include ppagecache_helpers.h.
test_ppagecache.c: 10 tests covering tier boundary conditions (0/1/2/3/4
including UINT32_MAX), CRC match, single-bit flip, wrong stored CRC,
zero-length buffer, and weak URL override.
Task #20 — pfs.c helper extraction (pfs_helpers.c/h)
pfs_row_to_folder_stat(row, stbuf): converts psql folder row → struct
stat; uses pfs_task_get_folder_tasks_rdlocked for in-memory mtime.
pfs_row_to_file_stat(row, stbuf, flags): converts psql file row → stat;
encrypted path calls pfs_crpt_plain_size.
pfs_mkdir_to_folder_stat(mk, stbuf): converts in-memory mkdir task →
stat (no SQL).
pfs_apply_task_overlay(stbuf, folder, name, flags): applies pending
mkdir/rmdir/unlink/creat-new overlays from the in-memory task queue;
returns 1/2/-1/0.
pfs_stat_uid/gid: exported globals; pfs.c syncs them from myuid/mygid.
pfs.c updated to #include pfs_helpers.h and sync the uid/gid globals.
pfsfolder.c: pfs_fldr_resolve_path decorated __attribute__((weak)) so
tests can inject fake path resolution without a FUSE mount or psql.
test_pfs_helpers.c: 7 tests covering folder/file stat field correctness,
overlay NULL/mkdir/rmdir/no-match cases, and weak path override; uses
--wrap for pfs_task_get_folder_tasks_rdlocked, pfs_crpt_plain_size, and
ptimer_time to stay SQL/crypto/timer-free.
Production build clean; make check exits 0.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* Add unit test for psync_task_free refcount fix (#377)
Adds tests/unit-tests/test_ptask_free.c to verify all code paths of the
psync_task_free fix from #377: single-owner free, last-ref destroy,
non-last-ref decrement, READY task signaling, and lock-before-refcnt
ordering. All 5 tests pass. Also adds compiled binary to .gitignore.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Refactor test_ptask_free to link production code via --wrap
Extract psync_task_free + psync_task_destroy (and their static helpers
psync_task_dec_refcnt, psync_task_entry) from ptask.c into a new
separately-compilable unit pclsync/ptask_free.c. Add
pclsync/ptask_free_internal.h to expose the internal struct layout
(struct psync_task_manager_t_ / struct psync_task_t_) for test use
without pulling in ptask.c's heavyweight transitive dependencies.
Rewrite tests/unit-tests/test_ptask_free.c to:
- Include ptask_free_internal.h instead of duplicating structs inline
- Call the real psync_task_free() rather than a local replica
- Intercept pthread_mutex_lock/unlock and pmem_free via --wrap linker
flags to observe lock discipline and detect destroy invocations
Update the Makefile test_ptask_free target to link pclsync/ptask_free.c
and pass the required --wrap flags. Production build unchanged: ptask_free.o
is picked up automatically by the existing wildcard COBJ rule.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Implement Tasks #3, #4, #5: tree tests, pfstasks tree layer, DB harness
Task #3 — Unit tests for ptree and pintervaltree
tests/unit-tests/test_ptree.c: 8 tests covering single-node insert,
in-order traversal after arbitrary and reverse inserts, BST lookup,
leaf/root/all-node deletion, and ptree_for_each visitation.
tests/unit-tests/test_pintervaltree.c: 18 tests covering single add,
non-overlapping, overlapping/adjacent/contained/spanning merges,
chain merge, remove middle split, remove exact/left/right/spanning,
cut_end, first_interval_containing_or_after, and free(NULL).
Task #4 — Extract pfstasks tree layer
pclsync/pfstasks_tree.h + pclsync/pfstasks_tree.c: pure tree layer
(zero psql calls) extracted from pfstasks.c — pfs_task_search_tree,
pfs_task_walk_tree (static helpers), pfs_task_insert_into_tree,
pfs_task_find_mkdir/rmdir/creat/unlink,
pfs_task_find_mkdir_by_folderid, pfs_task_find_creat_by_fileid.
pclsync/pfstasks.c: #includes pfstasks_tree.h; all moved functions
removed; all callers unchanged.
tests/unit-tests/test_pfstasks_tree.c: 13 tests using direct tree
construction (no DB) to verify find-by-name, taskid discrimination,
find-by-numeric-id, and empty-folder edge cases.
Task #5 — psql in-memory harness + pfstasks DB tests
tests/helpers/psql_test_helpers.h + .c: lightweight harness that
opens :memory: via sqlite3_open, enables PRAGMA foreign_keys=ON, and
applies the full PSYNC_DATABASE_STRUCTURE schema. Exposes
psql_test_db(), psql_test_exec(), psql_test_insert_fstask(),
psql_test_count_fstask/fstaskdepend(). No dependency on psql.c.
tests/unit-tests/test_pfstasks_db.c: 10 tests verifying schema
creation, fstask insertion/query, fstaskdepend insertion, CASCADE
DELETE propagation, FK enforcement, rmdir-blocking SQL pattern,
creat-after-unlink sequencing, and open/close idempotence.
All 11 new tests pass; production build clean.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Fix P1 review findings in pfstasks_db test and helpers
1. Check psql_test_exec() return values in test_cascade_delete() and
test_creat_after_unlink() consistently with test_fstaskdepend_insert().
2. Remove dead dep_cnt variable and (void)dep_cnt suppressor from
test_creat_after_unlink().
3. Change SQLITE_STATIC → SQLITE_TRANSIENT for text1 binding in
psql_test_insert_fstask() to avoid dangling-pointer footgun on
future reuse.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Fix ASAN/LSAN failures: ppath_home stack-use-after-scope + intentional leak
pclsync/ppath.c: Move buff[4096] to function scope in ppath_home() so
the pointer stored in dir via result->pw_dir remains live through the
putil_strdup(dir) call. Previously buff went out of scope at the if-block
close, causing a stack-use-after-scope ASAN report on every call that fell
through the getpwuid_r path.
tests/unit-tests/test_ptools_errptr.c: run_unfixed() intentionally leaks
errPtr to demonstrate the pre-fix bug. Wrap the allocation with
LSAN_DISABLE() / LSAN_ENABLE() so LSAN does not abort the process at exit
before stdio flushes, which was causing a non-zero exit code. The guard
uses nested #ifdef/__has_feature to remain compatible with both GCC
(__SANITIZE_ADDRESS__) and Clang (__has_feature(address_sanitizer))
without triggering "missing binary operator" errors on GCC.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Implement Tasks #11 and #12: plocks stress test + pfsupload send tests
Task #11 — plocks.c stress test (test_plocks.c)
7 tests: basic rdlock/wrlock round-trip, recursive TLS counting (same
thread acquires rdlock N times; unlock only releases on final decrement),
upgrade under contention (N readers + towrlock; barrier-synchronized),
writer starvation prevention (sustained reader load; writer acquires
within 500ms), N-reader + M-writer counter-integrity stress test (ASAN),
and try-variant contention (trywrlock fails when another thread holds
rdlock). TSAN note documented: custom lock internals require ASAN-only
when ThreadSanitizer annotations are absent.
Task #12 — pfsupload send-function tests (pfsupload_send.c/h + test_pfsupload.c)
Extract psync_send_task_mkdir and psync_send_task_rmdir from pfsupload.c
into pclsync/pfsupload_send.c as non-static pfsupload_send_mkdir/rmdir.
Expose fsupload_task_t struct via pclsync/pfsupload_send.h. Add
__attribute__((weak)) get_urls() as an injectable URL seam for large-
upload paths. pfsupload.c updated to include pfsupload_send.h and use
the renamed functions in its dispatch table; pfsupload_send.o is
automatically picked up by the production wildcard build.
5 tests: mkdir (non-encrypted) command + folderid param, mkdir
(encrypted) key param present, rmdir command + sfolderid, API error path
(papi_send failure → -1), get_urls() weak override. Uses
--wrap=papi_send to intercept API calls and socketpair() to provide a
valid psock_t without real network I/O.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* P2 cleanup: comments, make_fake_api stack alloc, find_str_param fix
1. test_plocks.c: add comment to test_upgrade_under_contention clarifying
it verifies towrlock completion and holding_wrlock; notes that concurrent
exclusivity is covered by test_stress().
2. test_pfsupload.c / make_fake_api: replace static-local psock_t with
caller-supplied stack allocation (out parameter) to eliminate the
multiple-calls-per-test footgun.
3. test_pfsupload.c / find_str_param: replace ternary
`paramnamelen == strlen ? paramname : ""` with explicit length check +
strncmp, matching the cleaner pattern used in find_num_param.
4. Makefile: add comment next to -Wl,--wrap=papi_send noting it redirects
papi_send to __wrap_papi_send and is GNU ld only (not macOS Apple ld).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* Add unit test for psync_task_free refcount fix (#377)
Adds tests/unit-tests/test_ptask_free.c to verify all code paths of the
psync_task_free fix from #377: single-owner free, last-ref destroy,
non-last-ref decrement, READY task signaling, and lock-before-refcnt
ordering. All 5 tests pass. Also adds compiled binary to .gitignore.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Add GitHub Actions CI workflow for unit tests
Triggers on push/PR to automated-testing branch. Installs cmake and
build-essential, builds all test targets via cmake, and runs ctest
--output-on-failure. Fails workflow on any test failure.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Replace cmake CI with make tests/check targets
Adds tests and check targets to Makefile — no cmake required.
Each test binary is built with the correct flags (pthread, -lrt,
--wrap linker flags for prun/ptools_errptr). CI workflow installs
only build-essential, runs make tests then make check; exits non-zero
on any failure. All 8 test suites pass locally.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Fix CI: install libfuse3-dev so Makefile parses on Ubuntu
detect_fuse.sh runs at Makefile parse time; without fuse headers the
$(error) fires before any target runs. Adding libfuse3-dev unblocks
make tests (test binaries themselves don't link fuse).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix makefile
* Add automated testing infrastructure
- Update CI workflow to run unit tests and build verification
- Add Makefile targets for test compilation and execution
- Implement unit tests for pdbg_path, prun, and read_response
- Add test stubs for pCloud API mocking
- Add test binaries for pfs_lock_ordering and signal_safety verification
* Add missing dependencies to CI workflow
Install libfuse-dev and libssl-dev required for build
* Add test job to c-cpp.yml workflow
Include unit test execution in C/C++ workflow
* Add missing stubs to test_stubs.c
Complete stub implementations for all required pCloud API functions
* Fix stub signatures to match headers
Correct function signatures for pCloud API stubs
* Fix psql_* stub signatures
Correct all psql function signatures to match headers
* Fix stub implementations and Makefile
Update stub functions and build configuration
* Link real utility files instead of stubbing
Update Makefile to use actual implementation files for utilities
* Complete test framework with all 41 tests passing
- Makefile: Add test rules with real dependencies
- tests/stubs/test_stubs.c: Minimal stubs for external APIs
- tests/stubs/test_stubs_cpp.c: Stubs for C++ test
- pclsync/putil.c: Add null check in putil_strdup
- pclsync/pdbg.c: Add recursion guard in pdbg_printf
* Remove duplicate ci.yml workflow
Consolidate CI configuration into c-cpp.yml
* Remove compiled test binaries from git
- Remove test_pfs_lock_ordering and test_signal_safety binaries
- Add tests/test_* to .gitignore to prevent future commits
---------
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* Fix pcl-dls.1: free errPtr between calls in ptools_set_backend_file_dates()
char *errPtr was already used instead of char msgErr[1024], but was
not freed between the two ptools_backend_call() invocations. If the
first call allocated errPtr, the second would overwrite the pointer
without freeing it. Add free(errPtr); errPtr = NULL; between the two
calls to eliminate the leak.
Ref GH #194.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Add ptools_set_backend_file_dates() errPtr lifecycle tests (pcl-dls)
6 test cases using malloc/free wrapping covering both-succeed,
call1-error, call2-error, both-errors, pre-fix leak demonstration,
and no-double-free after mid-free NULLing.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* Fix pcl-aex.1: validate PCLOUD_LOG_PATH before use in psync_debug_path()
Add pdbg_path_is_safe() helper that rejects PCLOUD_LOG_PATH values
that are not absolute, contain '..' path components, or do not resolve
under the user HOME directory or /tmp. On rejection, fall back to the
default ~/.pcloud/debug.log path and emit a warning to stderr.
Also fix a pre-existing memory leak: ppath_home() return was not freed
in the default-path branch.
Ref GH #291.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Add PCLOUD_LOG_PATH path-safety tests (pcl-aex)
28 test cases covering relative paths, '..' traversal, paths outside
HOME and /tmp, valid accepted paths, and psync_debug_path() env
fallback behaviour.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* Fix pcl-aqb.1: eliminate function-pointer cast UB in start_thread() via union
Replace the single void* run field in thread_data with a union
{ thread0_run run0; thread1_run run1 } so each function pointer is
stored and retrieved at its correct type, eliminating the
thread0_run <-> thread1_run cast chain UB. Split start_thread() into
prun_thread() and prun_thread1() that each populate the appropriate
union member, backed by a shared start_thread_common() helper. Add
malloc NULL check with error log in both public functions.
pthread_create failure handling (log + free) was already present.
Ref GH #199.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Add prun start_thread() resource-leak and UB tests (pcl-aqb)
8 test cases using linker interposition covering pthread_create
failure data-free, attr_destroy on failure, malloc failure graceful
return, union fn storage without cast, and success path accounting.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* Fix pcl-a1j.1: replace sprintf with snprintf and add strcat length check in ptools_create_backend_event()
Add paramname length check (> 254 bytes → skip with warning) before
snprintf into charBuff[i][258], and validate the snprintf return
value. Add explicit length check before strcat into keyParams to
prevent overflow when paramname exceeds remaining buffer space.
Eliminates buffer overflow from long paramname.
Ref GH #195.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Fix pcl-a1j.1: clamp pCnt to PTOOLS_MAX_PARAMS to prevent charBuff stack overflow
charBuff[30][258] is a fixed-size stack array but pCnt was unbounded,
allowing any caller with params->paramCnt > 30 to overflow the stack
via charBuff[i] access. Add PTOOLS_MAX_PARAMS (30) define, use it to
size charBuff, and clamp pCnt to PTOOLS_MAX_PARAMS with a warning log
before the loop.
Ref GH #195.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Add ptools_create_backend_event() validation tests (pcl-a1j)
11 test cases covering pCnt clamping, paramname length guards,
snprintf boundary, keyParams overflow check, and comma-prefix
for subsequent params.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* Fix pcl-6nb.1: readResponse buffer over-read in rpcclient.cpp
Replace malloc'd receive buffer with stack array, fix the broken
validation (msg->length compared against POVERLAY_BUFSIZE rather
than max_value_size which excluded the header), fix the payload-read
loop target (msg->length already includes the header, so the old
`header_size + msg->length` limit over-read), add the missing
`msg->length > total_read` guard before memcpy, and add EINTR
handling in the read loop. Eliminates heap over-read when the daemon
sends msg->length > POVERLAY_BUFSIZE.
Ref GH #184.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Add readResponse validation tests (pcl-6nb)
Six socketpair-based test cases covering oversized msg->length,
total_read underrun, header underflow, truncated header, valid
message, and exact-boundary acceptance.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Levi Neely