<p>laforge <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/libosmocore/+/16140">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">logging/vty: fix: actually ignore deprecated logging commands<br><br>We shall not prevent programs from starting if their configuration<br>files contain deprecated 'logging level ...' commands. Just print<br>a warning and return CMD_SUCCESS instead of CMD_WARNING.<br><br>While writing a unit test, another funny bug has been uncovered.<br>Parsing of a deprecated command indeed triggers a deprecation<br>warning, originated from libosmovty's log_deprecated_func().<br>This function simply calls vty_out(), but...<br><br>Since the invocation of the vty_out() happens _before_ the VTY<br>is initialized, the process is actually writing that warning<br>to its own stdin! Most likely, because we use talloc_zero()<br>to allocate a new instance of struct 'vty'.<br><br>As a side effect, the evil warning magically appears in the output<br>of 'make check', breaking the test statistics. Let's work around<br>this bug for now by redirecting stdin to /dev/null.<br><br>Change-Id: Ia934581410cd41594791d4e14ee74c16abe1009a<br>Fixes: Ic9c1b566ec4a459f03e6319cf369691903cf9d00<br>---<br>M src/vty/logging_vty.c<br>M tests/Makefile.am<br>M tests/testsuite.at<br>A tests/vty/ok_deprecated_logging.cfg<br>M tests/vty/vty_test.c<br>M tests/vty/vty_test.ok<br>6 files changed, 15 insertions(+), 2 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/vty/logging_vty.c b/src/vty/logging_vty.c</span><br><span>index b4c3776..6d908d9 100644</span><br><span>--- a/src/vty/logging_vty.c</span><br><span>+++ b/src/vty/logging_vty.c</span><br><span>@@ -998,7 +998,7 @@</span><br><span> static int log_deprecated_func(struct cmd_element *cmd, struct vty *vty, int argc, const char *argv[])</span><br><span> {</span><br><span> vty_out(vty, "%% Ignoring deprecated '%s'%s", cmd->string, VTY_NEWLINE);</span><br><span style="color: hsl(0, 100%, 40%);">- return CMD_WARNING;</span><br><span style="color: hsl(120, 100%, 40%);">+ return CMD_SUCCESS; /* Otherwise the process would terminate */</span><br><span> }</span><br><span> </span><br><span> void logging_vty_add_deprecated_subsys(void *ctx, const char *name)</span><br><span>diff --git a/tests/Makefile.am b/tests/Makefile.am</span><br><span>index 7624996..e8e4dee 100644</span><br><span>--- a/tests/Makefile.am</span><br><span>+++ b/tests/Makefile.am</span><br><span>@@ -306,6 +306,7 @@</span><br><span> vty/ok_more_spaces.cfg \</span><br><span> vty/ok_tabs_and_spaces.cfg \</span><br><span> vty/ok_tabs.cfg \</span><br><span style="color: hsl(120, 100%, 40%);">+ vty/ok_deprecated_logging.cfg \</span><br><span> comp128/comp128_test.ok bits/bitfield_test.ok \</span><br><span> utils/utils_test.ok utils/utils_test.err stats/stats_test.ok \</span><br><span> bitvec/bitvec_test.ok msgb/msgb_test.ok bits/bitcomp_test.ok \</span><br><span>diff --git a/tests/testsuite.at b/tests/testsuite.at</span><br><span>index c231b96..5865140 100644</span><br><span>--- a/tests/testsuite.at</span><br><span>+++ b/tests/testsuite.at</span><br><span>@@ -199,7 +199,10 @@</span><br><span> AT_KEYWORDS([vty])</span><br><span> cat $abs_srcdir/vty/vty_test.ok > expout</span><br><span> cp $abs_srcdir/vty/*.cfg .</span><br><span style="color: hsl(0, 100%, 40%);">-AT_CHECK([$abs_top_builddir/tests/vty/vty_test], [0], [expout], [ignore])</span><br><span style="color: hsl(120, 100%, 40%);">+# FIXME: calling vty_out() during initialization of the VTY interface would cause</span><br><span style="color: hsl(120, 100%, 40%);">+# the process write to its own *stdin*! This breaks the output of 'make check'.</span><br><span style="color: hsl(120, 100%, 40%);">+# Let's work this around untill the bug in libosmovty is fixed.</span><br><span style="color: hsl(120, 100%, 40%);">+AT_CHECK([$abs_top_builddir/tests/vty/vty_test 0>/dev/null], [0], [expout], [ignore])</span><br><span> AT_CLEANUP</span><br><span> </span><br><span> AT_SETUP([gprs-bssgp])</span><br><span>diff --git a/tests/vty/ok_deprecated_logging.cfg b/tests/vty/ok_deprecated_logging.cfg</span><br><span>new file mode 100644</span><br><span>index 0000000..2699719</span><br><span>--- /dev/null</span><br><span>+++ b/tests/vty/ok_deprecated_logging.cfg</span><br><span>@@ -0,0 +1,3 @@</span><br><span style="color: hsl(120, 100%, 40%);">+log stderr</span><br><span style="color: hsl(120, 100%, 40%);">+ logging filter all 1</span><br><span style="color: hsl(120, 100%, 40%);">+ logging level depr debug</span><br><span>diff --git a/tests/vty/vty_test.c b/tests/vty/vty_test.c</span><br><span>index 0d68a6c..1139638 100644</span><br><span>--- a/tests/vty/vty_test.c</span><br><span>+++ b/tests/vty/vty_test.c</span><br><span>@@ -29,6 +29,7 @@</span><br><span> </span><br><span> #include <osmocom/core/application.h></span><br><span> #include <osmocom/core/talloc.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <osmocom/core/logging_internal.h></span><br><span> #include <osmocom/core/logging.h></span><br><span> #include <osmocom/core/stats.h></span><br><span> #include <osmocom/core/utils.h></span><br><span>@@ -442,6 +443,8 @@</span><br><span> install_element(CONFIG_NODE, &cfg_ret_warning_cmd);</span><br><span> install_element(CONFIG_NODE, &cfg_ret_success_cmd);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ logging_vty_add_deprecated_subsys(tall_log_ctx, "depr");</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> install_element(CONFIG_NODE, &cfg_level1_cmd);</span><br><span> install_node(&level1_node, NULL);</span><br><span> install_element(LEVEL1_NODE, &cfg_level1_child_cmd);</span><br><span>@@ -544,6 +547,7 @@</span><br><span> test_exit_by_indent("ok_indented_root.cfg", 0);</span><br><span> test_exit_by_indent("ok_empty_parent.cfg", 0);</span><br><span> test_exit_by_indent("fail_cmd_ret_warning.cfg", -EINVAL);</span><br><span style="color: hsl(120, 100%, 40%);">+ test_exit_by_indent("ok_deprecated_logging.cfg", 0);</span><br><span> </span><br><span> test_is_cmd_ambiguous();</span><br><span> </span><br><span>diff --git a/tests/vty/vty_test.ok b/tests/vty/vty_test.ok</span><br><span>index 0b5ac9c..d2c9611 100644</span><br><span>--- a/tests/vty/vty_test.ok</span><br><span>+++ b/tests/vty/vty_test.ok</span><br><span>@@ -290,6 +290,8 @@</span><br><span> Called: 'return-success'</span><br><span> Called: 'return-warning'</span><br><span> got rc=-22</span><br><span style="color: hsl(120, 100%, 40%);">+reading file ok_deprecated_logging.cfg, expecting rc=0</span><br><span style="color: hsl(120, 100%, 40%);">+got rc=0</span><br><span> Going to test is_cmd_ambiguous()</span><br><span> Going to execute 'ambiguous_nr'</span><br><span> Called: 'ambiguous_nr [<0-23>]' (argc=0)</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/libosmocore/+/16140">change 16140</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/libosmocore/+/16140"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: libosmocore </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Ia934581410cd41594791d4e14ee74c16abe1009a </div>
<div style="display:none"> Gerrit-Change-Number: 16140 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: fixeria <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: fixeria <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>