On Wed, Aug 06, 2025 at 04:30:32PM +0200, Toon Claes wrote:
> Patrick Steinhardt <ps@xxxxxx> writes:
> > 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
