Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix libyang error when setting confirmed commit #283

Closed
wants to merge 1 commit into from

Conversation

syyyr
Copy link
Contributor

@syyyr syyyr commented Jan 13, 2021

When creating a commit RPC, if the ietf-netconf module doesn't have the
"confirmed-commit" feature enabled, it always prints a libyang error. This
patch adds a check for that.

Reproducer program:

#include <libnetconf2/session_client.h>

int main(int argc, char* argv[])
{
    auto sess = nc_connect_unix("/opt/cesnet/Netopeer2/netopeer2-server.sock", nullptr);
    auto rpc = nc_rpc_commit(1, 0, nullptr, nullptr, NC_PARAMTYPE_CONST);
    uint64_t id;
    nc_send_rpc(sess, rpc, 0, &id);
}

When run, it prints:

$ ./a.out 
libyang[0]: Failed to find "confirmed" as a sibling to "ietf-netconf:confirmed".

When creating a commit RPC, if the ietf-netconf module doesn't have the
"confirmed-commit" feature enabled, it always prints a libyang error.
This patch adds a check for that.
@michalvasko
Copy link
Member

So you want to silently ignore these parameters? I definitely consider printing an error a better solution, it is intentional.

@syyyr
Copy link
Contributor Author

syyyr commented Jan 13, 2021

Yeah, you are right, this isn't a solution, my bad. So, what if, when the server doesn't support confirmed-commit, the RPC fails to send and libnetconf2 prints a custom error message, something like "Confirmed commit is enabled on the remote server"? When I first used nc_rpc_commit I didn't realize what "confirmed" means, so I just put in "1" (which is my mistake, I admit). Right now, if you put in "1" and the server doesn't support it, libnetconf2 still ignores it and prints a libyang error, but, to be honest, the libyang error doesn't help too much...

@michalvasko
Copy link
Member

Right, the return value is not checked for these leaves but it is hard to decide whether it was intentional or not. In any case, I have added the checks. As for the error, I agree it is not really clear, but there is not much to be done about it because it is specified this way (although I am not able to find this right now). Also, libyang version 2 is not even able to determine whether the node does not exist or has only false if-feature so it will not be changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants