Skip to content

Fix parsing for notification after leaf#70

Merged
michalvasko merged 1 commit intoCESNET:develfrom
ADTRAN:notification-after-leaf
Aug 13, 2018
Merged

Fix parsing for notification after leaf#70
michalvasko merged 1 commit intoCESNET:develfrom
ADTRAN:notification-after-leaf

Conversation

@alangefe
Copy link
Copy Markdown
Contributor

This commit fixes parsing of notification messages for the case where the notification follows a leaf. Prior to this change, if the notification followed a leaf, the notification would be missed. In the LYS_LEAF case, elem would be advanced to elem->next (the notification), but the for loop condition to detect the notification would not be re-evaluated due to the goto next_node jump. By the time the loop condition was re-evaluated, elem would have been advanced to the notification's child.

We've written integration tests that demonstrate the problem. This commit, together with ADTRAN/sysrepo@acac046, will allow those failing test cases to pass. (See CI results, lines 7282-7284, or test source code.) These test cases cover several different variations of notifications tied to data nodes.

This commit fixes parsing of notification messages for the case where the
notification follows a leaf. Prior to this change, if the notification
followed a leaf, the notification would be missed. In the `LYS_LEAF` case,
`elem` would be advanced to `elem->next` (the notification), but the `for`
loop condition to detect the notification would not be re-evaluated due to the
`goto next_node` jump. By the time the loop condition was re-evaluated, `elem`
would have been advanced to the notification's child.

We've written integration tests that demonstrate the problem. This commit,
together with a forthcoming commit to sysrepo, will allow those failing test
cases to pass. (See [CI results], lines 7282-7284, or [test source code].)
These test cases cover several different variations of notifications tied to
data nodes.

[CI results]: https://travis-ci.org/ADTRAN/netopeer2-integration-tests/builds/414144146#L7282
[test source code]: https://github.com/ADTRAN/netopeer2-integration-tests/blob/master/tests/test_notif.py#L229
@michalvasko
Copy link
Copy Markdown
Member

Hi,
nice, thanks!

Regards,
Michal

@michalvasko michalvasko merged commit fcea27b into CESNET:devel Aug 13, 2018
@alangefe alangefe deleted the notification-after-leaf branch August 29, 2018 01:38
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.

2 participants