xpath.c call to ncx_find_any_object yields weird runtime errors

[APCBoston]

In xpath.c, if an XPath does not include a namespace prefix, it calls ncx_find_any_object, which returns the first such object found. This issue reflects my spending an entire day tracking down a runtime error caused by this default with unhelpful error messages. Resolution might include changing the logic or enhancing error reporting.

In this particular case, I was debugging a SIL created from a yangdump stub. The YANG for the device in question includes a container called system. The auto-generated SIL code calls agt_cb_register_callback() with /system/... for that container and its child nodes, with no prefix specified. However, this results in ncx_find_any_object returning the ietf-system module, which it finds before mine, and which also contains a container called "system." Unfortunately, the result of all this is that agt_cb_register_callback() fails with ERR_NCX_DEFSEG_NOT_FOUND. And of course then I see on stderr that netconfd could not find the definition for /system/foo and look at my YANG and say "well why not? It's right there!" I ultimately had to compile libyuma with debugging symbols and run it in gdb to isolate the root cause of the error, which is not ideal in terms of the helpfulness of error messages.

A quick fix could include simply adding a log_debug3 or log_debug4 call at https://github.com/vlvassilev/yuma123/blob/d8d38672b9c92bcffc3383fc5c410b6dfe4a13de/netconf/src/ncx/xpath.c#L789C18-L789C33, indicating that in the absence of a prefix, the library is searching all modules and that spurious ERR_NCX_DEFSEG_NOT_FOUND results might be resolved by adding a prefix. It seems to me that agt_cb_register_callback could be made smart enough to add the relevant prefix to the XPath if none were supplied, but I haven't looked deeply at where that might happen.

[vlvassilev]

I see 2 scenarios that have an issue:

1. The function failing to find /system/foo in the wrong module namespace (ietf-system) and returning error should ideally continue searching for a match in the rest of the modules. (that is your case)

2. Another case not currently handled is finding a match but in the wrong namespace. e.g.if ietf-system:system/foo actually existed. That would have been an even tougher problem to debug. And here the right behavior should be to actually report there are more then one matches and the ambiguity should be resolved by adding namespaces.

That said I do not think yangdump generated wrapper code is useful for anything more than a demonstration. I do not think the *_edit callback model is worth using. A single commit_complete callback per module is the only realistic way to handle configuration changes not the *_edit callbacks mechanism which only works for unrealistically simple models.

[APCBoston]

In case 1, searching for a match in the rest of the modules would be great, but a more-informative error would also work and would be a significant improvement. Case #2 is a good catch.

As far as the utility of the yangdump code is concerned, that's a useful note (and would be useful to include in the yangdump docs!) . In this particular case I happen to be working on an unrealistically simple model (a trivial standalone netconfd-based simulation of a device), so it gets me further than it otherwise might :-)