Thread (14 messages) 14 messages, 2 authors, 2025-08-07

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help