Re: [PATCH 2/3] within_depth: fix return for empty path
From: Patrick Steinhardt <hidden>
Date: 2025-08-07 06:15:45
On Wed, Aug 06, 2025 at 04:30:32PM +0200, Toon Claes wrote:
Patrick Steinhardt [off-list ref] writes:quoted
Just for my own understanding: the only difference is when we don't have even a single matching slash, as we don't verify `depth > max_depth` in that case. So in theory, we could modify the function to the following equivalent: int within_depth(const char *name, int namelen, int depth, int max_depth) { const char *cp = name, *cpe = name + namelen; if (depth > max_depth) return 0; while (cp < cpe) { if (*cp++ != '/') continue; depth++; if (depth > max_depth) return 0; } return 1; } (Not saying we should, I'm just double checking my understanding).To be honest, I wasn't sure no more. I decided to see if I can write a unit test. This is what I came up with: void test_dir__within_depth(void) { struct test_case { const char *path; int depth; int max_depth; int expect; } test_cases[] = { /* depth = 0; max_depth = 0 */ { "", 0, 0, 1 }, { "file", 0, 0, 1 }, { "a", 0, 0, 1 }, { "a/file", 0, 0, 0 }, { "a/b", 0, 0, 0 }, { "a/b/file", 0, 0, 0 }, /* depth = 0; max_depth = 1 */ { "", 0, 1, 1 }, { "file", 0, 1, 1 }, { "a", 0, 1, 1 }, { "a/file", 0, 1, 1 }, { "a/b", 0, 1, 1 }, { "a/b/file", 0, 1, 0 }, /* depth = 1; max_depth = 1 */ { "", 1, 1, 1 }, { "file", 1, 1, 1 }, { "a", 1, 1, 1 }, { "a/file", 1, 1, 0 }, { "a/b", 1, 1, 0 }, { "a/b/file", 1, 1, 0 }, /* depth = 1; max_depth = 0 */ { "", 1, 0, 0 }, { "file", 1, 0, 0 }, { "a", 1, 0, 0 }, { "a/file", 1, 0, 0 }, { "a/b", 1, 0, 0 }, { "a/b/file", 1, 0, 0 }, }; for (size_t i = 0; i < ARRAY_SIZE(test_cases); i++) { int result = within_depth(test_cases[i].path, strlen(test_cases[i].path), test_cases[i].depth, test_cases[i].max_depth); cl_assert_equal_b(result, test_cases[i].expect); } } The change in this patch affect the last batch of tests (the batch with 'depth = 1; max_depth = 0'). Running the test on both this patch and your suggestion gives the same results, so yes your suggestion has the same output. Do you think it's worth to add such unit test?
Sure, if we can easily test this via such a unit test I think it would be a good addition. Patrick