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>
This commit is contained in:
Levi Neely 2026-03-11 07:43:50 +01:00
parent 78a517ebba
commit 054a2a97a3
3 changed files with 21 additions and 13 deletions

View File

@ -236,7 +236,7 @@ tests/test_plocks: $(UNIT_DIR)/test_plocks.c $(LIBDIR)/plocks.c $(LIBDIR)/pdbg.c
tests/test_pfsupload: $(UNIT_DIR)/test_pfsupload.c $(LIBDIR)/pfsupload_send.c $(LIBDIR)/pdbg.c $(LIBDIR)/pmem.c $(LIBDIR)/putil.c $(LIBDIR)/ppath.c tests/stubs/test_stubs.c
$(CC) $(TEST_CFLAGS) $(CFLAGS) -o $@ $^ \
-Wl,--wrap=papi_send
-Wl,--wrap=papi_send # redirect papi_send → __wrap_papi_send; GNU ld only (not macOS Apple ld)
tests/test_read_response: $(UNIT_DIR)/test_read_response.cpp rpcclient.cpp tests/stubs/test_stubs_cpp.c
$(CXX) $(TEST_CXXFLAGS) $(CXXFLAGS) -o $@ $^

View File

@ -80,10 +80,11 @@ static void reset_wrap(void) {
/* Find a string parameter by name; returns its value or NULL */
static const char *find_str_param(const char *name) {
if (!g_last_params) return NULL;
size_t nlen = strlen(name);
for (size_t i = 0; i < g_last_nparams; i++) {
if (g_last_params[i].paramtype == PARAM_STR &&
strcmp(g_last_params[i].paramnamelen == strlen(name) ?
g_last_params[i].paramname : "", name) == 0)
g_last_params[i].paramnamelen == nlen &&
strncmp(g_last_params[i].paramname, name, nlen) == 0)
return g_last_params[i].str;
}
return NULL;
@ -103,13 +104,13 @@ static int find_num_param(const char *name, uint64_t *out) {
return 0;
}
/* Build a minimal fake psock_t wrapping one end of a socketpair */
static psock_t *make_fake_api(int sv[2]) {
static psock_t fake;
/* Build a minimal fake psock_t wrapping one end of a socketpair.
* The caller provides a stack-allocated psock_t; no static storage is used
* so multiple calls within the same test function are safe. */
static void make_fake_api(psock_t *out, int sv[2]) {
socketpair(AF_UNIX, SOCK_STREAM, 0, sv);
memset(&fake, 0, sizeof(fake));
fake.sock = sv[1];
return &fake;
memset(out, 0, sizeof(*out));
out->sock = sv[1];
}
/* ------------------------------------------------------------------ */
@ -128,7 +129,7 @@ static void task_init(fsupload_task_t *t) {
static void test_send_mkdir_basic(void) {
reset_wrap();
int sv[2];
psock_t *api = make_fake_api(sv);
psock_t api_s; make_fake_api(&api_s, sv); psock_t *api = &api_s;
fsupload_task_t task;
task_init(&task);
@ -155,7 +156,7 @@ static void test_send_mkdir_basic(void) {
static void test_send_mkdir_encrypted(void) {
reset_wrap();
int sv[2];
psock_t *api = make_fake_api(sv);
psock_t api_s; make_fake_api(&api_s, sv); psock_t *api = &api_s;
fsupload_task_t task;
task_init(&task);
@ -183,7 +184,7 @@ static void test_send_mkdir_encrypted(void) {
static void test_send_rmdir(void) {
reset_wrap();
int sv[2];
psock_t *api = make_fake_api(sv);
psock_t api_s; make_fake_api(&api_s, sv); psock_t *api = &api_s;
fsupload_task_t task;
task_init(&task);
@ -210,7 +211,7 @@ static void test_api_error_path(void) {
reset_wrap();
g_wrap_rc = 0; /* simulate papi_send failure */
int sv[2];
psock_t *api = make_fake_api(sv);
psock_t api_s; make_fake_api(&api_s, sv); psock_t *api = &api_s;
fsupload_task_t task;
task_init(&task);

View File

@ -141,6 +141,13 @@ static void *upgrade_reader_thread(void *arg) {
return NULL;
}
/*
* test_upgrade_under_contention verifies that plocks_towrlock() completes
* (returns 0) and that the upgrading thread holds the write lock on return.
* Concurrent exclusivity — that no reader is simultaneously active once the
* write lock is granted — is covered by the counter-integrity check in
* test_stress().
*/
static void test_upgrade_under_contention(void) {
struct upgrade_state s;
plocks_init(&s.rw);