Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

    https://gerrit.osmocom.org/3992

to look at the new patch set (#2).

fix vty regression: empty parent node

The recent exit-by-indent patch breaks a VTY case where a node is entered but
directly followed by a sibling or ancestor without listing any child nodes.
Regression introduced by I24cbb3f6de111f2d31110c3c484c066f1153aac9.

An example is a common usage in osmo-bts, where 'phy N' / 'instance N' is a
parent node that is commonly left empty:

        phy 0
         instance 0
        bts 0
         band 1800

Before this patch, this case produces the error:

        There is no such command.
        Error occurred during reading the below line:
        bts 0

Fix indentation parsing logic in command.c to accomodate this case.

Add a unit test for empty parent node.

Change-Id: Ia0880a17ae55accb092ae8585cc3a1bec9986891
---
M src/vty/command.c
M tests/Makefile.am
A tests/vty/ok_empty_parent.cfg
M tests/vty/vty_test.c
M tests/vty/vty_test.ok
5 files changed, 37 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/92/3992/2

diff --git a/src/vty/command.c b/src/vty/command.c
index a65b4de..f389928 100644
--- a/src/vty/command.c
+++ b/src/vty/command.c
@@ -2374,10 +2374,37 @@
                        continue;
                }
 
-               /* We have a nonempty line. This might be the first on a deeper 
indenting level, so let's
-                * remember this indent if we don't have one yet. */
-               if (!vty->indent)
-                       vty->indent = talloc_strdup(vty, indent);
+               /* We have a nonempty line. */
+               if (!vty->indent) {
+                       /* We have just entered a node and expecting the first 
child to come up; but we
+                        * may also skip right back to a parent or ancestor 
level. */
+                       parent = vty_parent(vty);
+                       if (parent) {
+                               cmp = indent_cmp(indent, parent->indent);
+
+                               if (cmp == EINVAL)
+                                       goto return_invalid_indent;
+
+                               if (cmp <= 0) {
+                                       /* We have gone right back to the 
parent level or higher, we are
+                                        * skipping this child node level 
entirely. Pop the parent to
+                                        * reinstate the vty->indent, and enter 
below loop to find a
+                                        * matching parent indent. */
+                                       vty_go_parent(vty);
+                                       /* (The next indent_cmp() below is just 
a repetition, but would
+                                        * make the code complex to skip it.) */
+                               }
+                       } else {
+                               /* If there is no parent, record any 
indentation we encounter: */
+                               cmp = 1;
+                       }
+
+                       if (cmp > 0) {
+                               /* The indent is deeper than the just entered 
parent, record the new
+                                * indentation characters. */
+                               vty->indent = talloc_strdup(vty, indent);
+                       }
+               }
 
                cmp = indent_cmp(indent, vty->indent);
                if (cmp == EINVAL)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 8935bf7..b138717 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -222,6 +222,7 @@
             vty/fail_tabs_and_spaces.cfg \
             vty/fail_too_much_indent.cfg \
             vty/ok.cfg \
+            vty/ok_empty_parent.cfg \
             vty/ok_ignore_blank.cfg \
             vty/ok_ignore_comment.cfg \
             vty/ok_indented_root.cfg \
diff --git a/tests/vty/ok_empty_parent.cfg b/tests/vty/ok_empty_parent.cfg
new file mode 100644
index 0000000..fe04fcf
--- /dev/null
+++ b/tests/vty/ok_empty_parent.cfg
@@ -0,0 +1,2 @@
+line vty
+log stderr
diff --git a/tests/vty/vty_test.c b/tests/vty/vty_test.c
index eba9995..d9af6ae 100644
--- a/tests/vty/vty_test.c
+++ b/tests/vty/vty_test.c
@@ -342,6 +342,7 @@
        test_exit_by_indent("fail_too_much_indent.cfg", -EINVAL);
        test_exit_by_indent("fail_tabs_and_spaces.cfg", -EINVAL);
        test_exit_by_indent("ok_indented_root.cfg", 0);
+       test_exit_by_indent("ok_empty_parent.cfg", 0);
 
        /* Leak check */
        OSMO_ASSERT(talloc_total_blocks(stats_ctx) == 1);
diff --git a/tests/vty/vty_test.ok b/tests/vty/vty_test.ok
index b2df1a1..f9fea34 100644
--- a/tests/vty/vty_test.ok
+++ b/tests/vty/vty_test.ok
@@ -128,4 +128,6 @@
 got rc=-22
 reading file ok_indented_root.cfg, expecting rc=0
 got rc=0
+reading file ok_empty_parent.cfg, expecting rc=0
+got rc=0
 All tests passed

-- 
To view, visit https://gerrit.osmocom.org/3992
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia0880a17ae55accb092ae8585cc3a1bec9986891
Gerrit-PatchSet: 2
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <[email protected]>

Reply via email to