Unprocessed return value in DatNode::unlink()

[mbohal]

Problem

When I try to call:

libyang-cpp/src/DataNode.cpp

Line 647 in 17da45e

void DataNode::unlink()

on a list key libyang::lyd_unlink_tree() returns error value. However this return value is ignored and the function unlink() nor returns error nor throws.

In other places the return value is handled correctly:

libyang-cpp/src/DataNode.cpp

Line 917 in 17da45e

DataNodeTerm::ValueChange DataNodeTerm::changeValue(const std::string value)

Proposal

I think the

libyang-cpp/src/DataNode.cpp

Line 541 in 17da45e

void handleLyTreeOperation(DataNode* affectedNode, Operation operation, OperationScope scope, std::shared_ptr<internal_refcount> newRefs)

method should be modified to throw if operation returns an error value.

Essentially changing this:

operation();

to this:

auto ret = operation();
throwIfError(ret, "libyang::unlink");

This would effect other methods, that also use handleLyTreeOperation(), but it seems to me, that is the desired state anyway.

This change, however, would change the public API of the library. How should it be handled?
Would you accept a PR in gerrit?

[jktjkt]

Nice discovery, error conditions reported by the C library should not be silently ignored. Moreover, the current behavior (with no error handling) can lead to memory corruption due to a heap-use-after-free.

Patch is here: https://gerrit.cesnet.cz/c/CzechLight/libyang-cpp/+/9134