Should LYD_TREE_DFS_END use lyd_child_any() instead of lyd_child() ?

[rbresalier]

In xpath.c function get_node_pos() my tree root is from a netconf get request, so root node is "get" followed by "data", followed by the rest.

Inside get_node_pos() is a LYD_TREE_DFS_BEGIN()/LYD_TREE_DFS_END() pair.

The traversal using LYD_TREE_DFS_BEGIN()/LYD_TREE_DFS_END() is stopping at the get/data and is not going to children of the get/data and this causes my node (inside a leaf-list) not to be found. Not finding the node triggers a libyang internal error in get_node_pos().

I found that the reason the node is not found is because when the LYD_TREE_DFS_END() runs on the 'data' node of the netconf get, the lyd_child() inside of LYD_TREE_DFS_END() returns NULL because the "data" node is of type "LYS_ANYXML". This means it won't traverse the rest of my tree under the "data" node of type LYS_ANYXML.

I locally changed the LYD_TREE_DFS_END() to use lyd_child_any() instead of lyd_child() and the internal error goes away.

So should LYD_TREE_DFS_END() be changed to use lyd_child_any() instead of lyd_child()?

I found this issue by using a netconf get request with an xpath that points to a leaf-list in the netconf get reply. The leaf-list has multiple nodes inside it causing set_sort() to be called. set_sort() ends up calling (indirectly) get_node_pos() and this triggers the internal error when lyd_child() is used instead of lyd_child_any().

I observe this issue on the latest version of libyang as of today which is v5.4.9.

[rbresalier]

I'd like to point out that this issue started happening when I updated our libyang from 3.3.15 to 5.4.9.

In the 3.3.15 version the child of the get/data had a NULL parent, but now with 5.4.9 the child of get/data has a parent pointing to data.

So in 3.3.15 the LYD_TREE_DFS_BEGIN()/LYD_TREE_DFS_END() started with the child of the get/data instead of the get in get/data. So that is likely why we didn't see this issue with 3.3.15.

Is it correct in 5.4.9 that the child of the get/data should have its parent pointer pointing to the data in get/data? Or is the old behavior correct that the child of get/data should have a NULL parent pointer?