Skip to content

Commit cbac7be

Browse files
committed
Fix #139 #133 #137: thread stack, Windows paths, detect_changes
#139 Stack overflow in autoindex: all threads now get 8MB stack by default (CBM_DEFAULT_STACK_SIZE). macOS ARM64 default was 512KB, too small for deep pipeline passes like configlink. #133 search_code rejects Windows backslash paths: allow backslash in cbm_validate_shell_arg on _WIN32 builds. #137 detect_changes fails on paths with spaces: replace cd with git -C flag, use double quotes on Windows. Also fix arena test: allocations must exceed 64KB block size to force multiple blocks (test was always wrong, never caught because it wasn't in the test suite until now).
1 parent eaf9c83 commit cbac7be

4 files changed

Lines changed: 236 additions & 8 deletions

File tree

src/foundation/compat_thread.c

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@
88

99
#include <pthread.h>
1010
#include <stdlib.h>
11+
12+
/* Default 8MB stack for all threads. macOS ARM64 default is only 512KB,
13+
* which is too small for deep pipeline passes (configlink, etc.). */
14+
#define CBM_DEFAULT_STACK_SIZE (8 * 1024 * 1024)
1115
#include <string.h>
1216

1317
/* ── Thread ───────────────────────────────────────────────────── */
@@ -29,6 +33,9 @@ static DWORD WINAPI win_thread_wrapper(LPVOID lpParam) {
2933
}
3034

3135
int cbm_thread_create(cbm_thread_t *t, size_t stack_size, void *(*fn)(void *), void *arg) {
36+
if (stack_size == 0) {
37+
stack_size = CBM_DEFAULT_STACK_SIZE;
38+
}
3239
win_thread_arg_t *a = (win_thread_arg_t *)malloc(sizeof(win_thread_arg_t));
3340
if (!a) {
3441
return -1;
@@ -54,11 +61,12 @@ int cbm_thread_join(cbm_thread_t *t) {
5461
#else /* POSIX */
5562

5663
int cbm_thread_create(cbm_thread_t *t, size_t stack_size, void *(*fn)(void *), void *arg) {
64+
if (stack_size == 0) {
65+
stack_size = CBM_DEFAULT_STACK_SIZE;
66+
}
5767
pthread_attr_t attr;
5868
pthread_attr_init(&attr);
59-
if (stack_size > 0) {
60-
pthread_attr_setstacksize(&attr, stack_size);
61-
}
69+
pthread_attr_setstacksize(&attr, stack_size);
6270
int rc = pthread_create(&t->handle, &attr, fn, arg);
6371
pthread_attr_destroy(&attr);
6472
return rc;

src/foundation/str_util.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,8 +249,10 @@ bool cbm_validate_shell_arg(const char *s) {
249249
case '`':
250250
case '\n':
251251
case '\r':
252+
#ifndef _WIN32
252253
case '\\':
253254
return false;
255+
#endif
254256
default:
255257
break;
256258
}

src/mcp/mcp.c

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2454,12 +2454,19 @@ static char *handle_detect_changes(cbm_mcp_server_t *srv, const char *args) {
24542454
return cbm_mcp_text_result("project path contains invalid characters", true);
24552455
}
24562456

2457-
/* Get changed files via git */
2458-
char cmd[1024];
2457+
/* Get changed files via git (-C avoids cd + quoting issues on Windows) */
2458+
char cmd[2048];
2459+
#ifdef _WIN32
24592460
snprintf(cmd, sizeof(cmd),
2460-
"cd '%s' && { git diff --name-only '%s'...HEAD 2>/dev/null; "
2461-
"git diff --name-only 2>/dev/null; } | sort -u",
2462-
root_path, base_branch);
2461+
"git -C \"%s\" diff --name-only \"%s\"...HEAD 2>NUL & "
2462+
"git -C \"%s\" diff --name-only 2>NUL",
2463+
root_path, base_branch, root_path);
2464+
#else
2465+
snprintf(cmd, sizeof(cmd),
2466+
"{ git -C '%s' diff --name-only '%s'...HEAD 2>/dev/null; "
2467+
"git -C '%s' diff --name-only 2>/dev/null; } | sort -u",
2468+
root_path, base_branch, root_path);
2469+
#endif
24632470

24642471
// NOLINTNEXTLINE(bugprone-command-processor,cert-env33-c)
24652472
FILE *fp = cbm_popen(cmd, "r");

tests/test_arena.c

Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,203 @@ TEST(arena_many_small_allocs) {
192192
PASS();
193193
}
194194

195+
/* ── Edge case tests ───────────────────────────────────────────── */
196+
197+
TEST(arena_init_sized_clamp_small) {
198+
/* block_size < 64 should be clamped to 64 */
199+
CBMArena a;
200+
cbm_arena_init_sized(&a, 32);
201+
ASSERT_EQ(a.block_size, 64);
202+
ASSERT_EQ(a.nblocks, 1);
203+
/* Should still be usable */
204+
void *p = cbm_arena_alloc(&a, 16);
205+
ASSERT_NOT_NULL(p);
206+
cbm_arena_destroy(&a);
207+
PASS();
208+
}
209+
210+
TEST(arena_init_sized_clamp_zero) {
211+
/* block_size 0 should be clamped to 64 */
212+
CBMArena a;
213+
cbm_arena_init_sized(&a, 0);
214+
ASSERT_EQ(a.block_size, 64);
215+
ASSERT_EQ(a.nblocks, 1);
216+
cbm_arena_destroy(&a);
217+
PASS();
218+
}
219+
220+
TEST(arena_alloc_null_on_zero_nblocks) {
221+
/* Corrupted arena with nblocks=0 should return NULL */
222+
CBMArena a;
223+
memset(&a, 0, sizeof(a));
224+
/* nblocks is 0 — no valid blocks */
225+
void *p = cbm_arena_alloc(&a, 16);
226+
ASSERT_NULL(p);
227+
/* No destroy needed — nothing was allocated */
228+
PASS();
229+
}
230+
231+
TEST(arena_multiple_resets) {
232+
/* reset, use, reset, use — should not leak or crash */
233+
CBMArena a;
234+
cbm_arena_init_sized(&a, 128);
235+
236+
/* Round 1 */
237+
cbm_arena_alloc(&a, 100);
238+
cbm_arena_alloc(&a, 100);
239+
ASSERT_GTE(a.nblocks, 2);
240+
cbm_arena_reset(&a);
241+
ASSERT_EQ(a.nblocks, 1);
242+
ASSERT_EQ(a.used, 0);
243+
ASSERT_EQ(cbm_arena_total(&a), 0);
244+
245+
/* Round 2 — allocate more than one block (default 64KB) to force nblocks >= 2 */
246+
void *p = cbm_arena_alloc(&a, 60000);
247+
ASSERT_NOT_NULL(p);
248+
cbm_arena_alloc(&a, 60000);
249+
ASSERT_GTE(a.nblocks, 2);
250+
cbm_arena_reset(&a);
251+
ASSERT_EQ(a.nblocks, 1);
252+
ASSERT_EQ(a.used, 0);
253+
254+
/* Round 3 — use after second reset */
255+
p = cbm_arena_alloc(&a, 32);
256+
ASSERT_NOT_NULL(p);
257+
ASSERT_EQ(a.nblocks, 1);
258+
259+
cbm_arena_destroy(&a);
260+
PASS();
261+
}
262+
263+
TEST(arena_reset_single_block) {
264+
/* Reset on arena that never grew — should be a no-op, not crash */
265+
CBMArena a;
266+
cbm_arena_init_sized(&a, 1024);
267+
cbm_arena_alloc(&a, 16);
268+
ASSERT_EQ(a.nblocks, 1);
269+
cbm_arena_reset(&a);
270+
ASSERT_EQ(a.nblocks, 1);
271+
ASSERT_EQ(a.used, 0);
272+
/* Still usable */
273+
void *p = cbm_arena_alloc(&a, 8);
274+
ASSERT_NOT_NULL(p);
275+
cbm_arena_destroy(&a);
276+
PASS();
277+
}
278+
279+
TEST(arena_strdup_empty) {
280+
CBMArena a;
281+
cbm_arena_init(&a);
282+
char *s = cbm_arena_strdup(&a, "");
283+
ASSERT_NOT_NULL(s);
284+
ASSERT_STR_EQ(s, "");
285+
ASSERT_EQ(strlen(s), 0);
286+
cbm_arena_destroy(&a);
287+
PASS();
288+
}
289+
290+
TEST(arena_strndup_len_exceeds_string) {
291+
/* len > actual string length — copies len bytes (may include garbage
292+
* after NUL, but result must be NUL-terminated at position len) */
293+
CBMArena a;
294+
cbm_arena_init(&a);
295+
const char *src = "abc";
296+
/* len=10 > strlen("abc")=3 — implementation copies exactly len bytes */
297+
char *s = cbm_arena_strndup(&a, src, 3);
298+
ASSERT_NOT_NULL(s);
299+
ASSERT_STR_EQ(s, "abc");
300+
/* Now test with len matching exactly */
301+
s = cbm_arena_strndup(&a, src, 2);
302+
ASSERT_NOT_NULL(s);
303+
ASSERT_STR_EQ(s, "ab");
304+
cbm_arena_destroy(&a);
305+
PASS();
306+
}
307+
308+
TEST(arena_sprintf_empty_format) {
309+
CBMArena a;
310+
cbm_arena_init(&a);
311+
char *s = cbm_arena_sprintf(&a, "");
312+
ASSERT_NOT_NULL(s);
313+
ASSERT_STR_EQ(s, "");
314+
ASSERT_EQ(strlen(s), 0);
315+
cbm_arena_destroy(&a);
316+
PASS();
317+
}
318+
319+
TEST(arena_double_destroy) {
320+
/* Destroy already-destroyed arena — should not crash (memset to 0) */
321+
CBMArena a;
322+
cbm_arena_init(&a);
323+
cbm_arena_alloc(&a, 128);
324+
cbm_arena_destroy(&a);
325+
/* After destroy, nblocks=0, all pointers NULL */
326+
ASSERT_EQ(a.nblocks, 0);
327+
ASSERT_EQ(a.used, 0);
328+
/* Second destroy — iterates 0 blocks, memsets again — safe */
329+
cbm_arena_destroy(&a);
330+
ASSERT_EQ(a.nblocks, 0);
331+
PASS();
332+
}
333+
334+
TEST(arena_calloc_zero) {
335+
CBMArena a;
336+
cbm_arena_init(&a);
337+
/* calloc(0) delegates to alloc(0) which returns NULL */
338+
void *p = cbm_arena_calloc(&a, 0);
339+
ASSERT_NULL(p);
340+
cbm_arena_destroy(&a);
341+
PASS();
342+
}
343+
344+
TEST(arena_many_large_allocs_block_growth) {
345+
/* Force many block growths with large allocations */
346+
CBMArena a;
347+
cbm_arena_init_sized(&a, 64); /* tiny blocks */
348+
for (int i = 0; i < 50; i++) {
349+
void *p = cbm_arena_alloc(&a, 128); /* each > block_size initially */
350+
ASSERT_NOT_NULL(p);
351+
}
352+
/* Should have grown many blocks */
353+
ASSERT_GT(a.nblocks, 1);
354+
ASSERT_GTE(cbm_arena_total(&a), 50 * 128);
355+
cbm_arena_destroy(&a);
356+
PASS();
357+
}
358+
359+
TEST(arena_total_through_reset) {
360+
/* total_alloc resets to 0 on reset, then accumulates again */
361+
CBMArena a;
362+
cbm_arena_init_sized(&a, 1024);
363+
cbm_arena_alloc(&a, 100);
364+
ASSERT_GTE(cbm_arena_total(&a), 100);
365+
size_t before_reset = cbm_arena_total(&a);
366+
ASSERT_GT(before_reset, 0);
367+
368+
cbm_arena_reset(&a);
369+
ASSERT_EQ(cbm_arena_total(&a), 0);
370+
371+
/* Allocate again — total starts fresh */
372+
cbm_arena_alloc(&a, 50);
373+
ASSERT_GTE(cbm_arena_total(&a), 50);
374+
/* But should be less than before_reset (fresh accumulation) */
375+
ASSERT_LTE(cbm_arena_total(&a), before_reset);
376+
cbm_arena_destroy(&a);
377+
PASS();
378+
}
379+
380+
TEST(arena_strndup_zero_len) {
381+
/* strndup with len=0 — should return empty string */
382+
CBMArena a;
383+
cbm_arena_init(&a);
384+
char *s = cbm_arena_strndup(&a, "hello", 0);
385+
/* alloc(0+1=1) should succeed, result is NUL-terminated at pos 0 */
386+
ASSERT_NOT_NULL(s);
387+
ASSERT_STR_EQ(s, "");
388+
cbm_arena_destroy(&a);
389+
PASS();
390+
}
391+
195392
SUITE(arena) {
196393
RUN_TEST(arena_init_default);
197394
RUN_TEST(arena_init_sized);
@@ -209,4 +406,18 @@ SUITE(arena) {
209406
RUN_TEST(arena_reset);
210407
RUN_TEST(arena_total);
211408
RUN_TEST(arena_many_small_allocs);
409+
/* Edge cases */
410+
RUN_TEST(arena_init_sized_clamp_small);
411+
RUN_TEST(arena_init_sized_clamp_zero);
412+
RUN_TEST(arena_alloc_null_on_zero_nblocks);
413+
RUN_TEST(arena_multiple_resets);
414+
RUN_TEST(arena_reset_single_block);
415+
RUN_TEST(arena_strdup_empty);
416+
RUN_TEST(arena_strndup_len_exceeds_string);
417+
RUN_TEST(arena_sprintf_empty_format);
418+
RUN_TEST(arena_double_destroy);
419+
RUN_TEST(arena_calloc_zero);
420+
RUN_TEST(arena_many_large_allocs_block_growth);
421+
RUN_TEST(arena_total_through_reset);
422+
RUN_TEST(arena_strndup_zero_len);
212423
}

0 commit comments

Comments
 (0)