Re: [PATCH] Fix get_node_by_path string equality check

[David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 4340 bytes --]

On Tue, Apr 18, 2017 at 03:18:55AM +0000, Tim Montague wrote:
> When determining if to recurse into a node, get_node_by_path does not
> check if the length of each node name is equal.  If searching for
> /foo/baz, this can result in recursing into /foobar because
> strneq("foo", "foobar", 3) is true.
> 
> This can result in a reference to /foo/baz to be incorrectly set to
> /foobar/baz.  A test for this was added.
> 
> Signed-off-by: Tim Montague <tmontague-5j0BnJlmnLM@public.gmane.org>

Ah.. it looks like your mailer has mangled the patch, I'm afraid:

$ git am -s ~/foobox/
Applying: Fix get_node_by_path string equality check
.git/rebase-apply/patch:16: trailing whitespace.
		if (p && (strlen(child->name) == p-path) &&
.git/rebase-apply/patch:17: trailing whitespace.
				strneq(path, child->name, p-path))
.git/rebase-apply/patch:30: trailing whitespace.
	int n1, n2, n3, n4;
.git/rebase-apply/patch:38: trailing whitespace.
	/* Check reference to nested nodes with common prefix */
.git/rebase-apply/patch:39: trailing whitespace.
	n3 = fdt_path_offset(fdt, "/foo/baz");
error: patch failed: livetree.c:478
error: livetree.c: patch does not apply
error: patch failed: tests/path-references.c:66
error: tests/path-references.c: patch does not apply
error: patch failed: tests/path-references.dts:12
error: tests/path-references.dts: patch does not apply
Patch failed at 0001 Fix get_node_by_path string equality check
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Can you check your mailer settings.  If it's not clear how to stop it
doing that, it's probably safe to attach the patch instead.

> ---
>  livetree.c                |  3 ++-
>  tests/path-references.c   | 12 +++++++++++-
>  tests/path-references.dts | 13 +++++++++++++
>  3 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/livetree.c b/livetree.c
> index 3673de0..aecd278 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -478,7 +478,8 @@ struct node *get_node_by_path(struct node *tree, const char *path)
>  	p = strchr(path, '/');
>  
>  	for_each_child(tree, child) {
> -		if (p && strneq(path, child->name, p-path))
> +		if (p && (strlen(child->name) == p-path) &&
> +				strneq(path, child->name, p-path))
>  			return get_node_by_path(child, p+1);
>  		else if (!p && streq(path, child->name))
>  			return child;
> diff --git a/tests/path-references.c b/tests/path-references.c
> index c8d25fb..5e332e8 100644
> --- a/tests/path-references.c
> +++ b/tests/path-references.c
> @@ -66,7 +66,7 @@ int main(int argc, char *argv[])
>  	void *fdt;
>  	const char *p;
>  	int len, multilen;
> -	int n1, n2;
> +	int n1, n2, n3, n4;
>  
>  	test_init(argc, argv);
>  	fdt = load_blob_arg(argc, argv);
> @@ -92,6 +92,16 @@ int main(int argc, char *argv[])
>  	if ((!streq(p, "/node1") || !streq(p + strlen("/node1") + 1, "/node2")))
>  		FAIL("multiref has wrong value");
>  
> +	/* Check reference to nested nodes with common prefix */
> +	n3 = fdt_path_offset(fdt, "/foo/baz");
> +	if (n3 < 0)
> +		FAIL("fdt_path_offset(/foo/baz): %s", fdt_strerror(n3));
> +	n4 = fdt_path_offset(fdt, "/foobar/baz");
> +	if (n4 < 0)
> +		FAIL("fdt_path_offset(/foobar/baz): %s", fdt_strerror(n4));
> +	check_ref(fdt, n3, "/foobar/baz");
> +	check_ref(fdt, n4, "/foo/baz");
> +
>  	check_rref(fdt);
>  
>  	PASS();
> diff --git a/tests/path-references.dts b/tests/path-references.dts
> index b00fd79..3808aa6 100644
> --- a/tests/path-references.dts
> +++ b/tests/path-references.dts
> @@ -12,4 +12,17 @@
>  		ref = &{/node1}; /* reference after target */
>  		lref = &n1;
>  	};
> +	/* Check references to nested nodes with common prefix */
> +	foobar {
> +		n3: baz {
> +		    ref = &{/foo/baz};
> +		    lref = &n4;
> +		};
> +	};
> +	foo {
> +		n4: baz {
> +		    ref = &{/foobar/baz};
> +		    lref = &n3;
> +		};
> +	};
>  };

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]