Change in libosmocore[master]: logging/vty: fix: actually ignore deprecated logging commands

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

laforge gerrit-no-reply at lists.osmocom.org
Thu Nov 21 06:26:42 UTC 2019


laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/16140 )

Change subject: logging/vty: fix: actually ignore deprecated logging commands
......................................................................

logging/vty: fix: actually ignore deprecated logging commands

We shall not prevent programs from starting if their configuration
files contain deprecated 'logging level ...' commands. Just print
a warning and return CMD_SUCCESS instead of CMD_WARNING.

While writing a unit test, another funny bug has been uncovered.
Parsing of a deprecated command indeed triggers a deprecation
warning, originated from libosmovty's log_deprecated_func().
This function simply calls vty_out(), but...

Since the invocation of the vty_out() happens _before_ the VTY
is initialized, the process is actually writing that warning
to its own stdin! Most likely, because we use talloc_zero()
to allocate a new instance of struct 'vty'.

As a side effect, the evil warning magically appears in the output
of 'make check', breaking the test statistics. Let's work around
this bug for now by redirecting stdin to /dev/null.

Change-Id: Ia934581410cd41594791d4e14ee74c16abe1009a
Fixes: Ic9c1b566ec4a459f03e6319cf369691903cf9d00
---
M src/vty/logging_vty.c
M tests/Makefile.am
M tests/testsuite.at
A tests/vty/ok_deprecated_logging.cfg
M tests/vty/vty_test.c
M tests/vty/vty_test.ok
6 files changed, 15 insertions(+), 2 deletions(-)

Approvals:
  Jenkins Builder: Verified
  laforge: Looks good to me, approved



diff --git a/src/vty/logging_vty.c b/src/vty/logging_vty.c
index b4c3776..6d908d9 100644
--- a/src/vty/logging_vty.c
+++ b/src/vty/logging_vty.c
@@ -998,7 +998,7 @@
 static int log_deprecated_func(struct cmd_element *cmd, struct vty *vty, int argc, const char *argv[])
 {
 	vty_out(vty, "%% Ignoring deprecated '%s'%s", cmd->string, VTY_NEWLINE);
-	return CMD_WARNING;
+	return CMD_SUCCESS; /* Otherwise the process would terminate */
 }
 
 void logging_vty_add_deprecated_subsys(void *ctx, const char *name)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 7624996..e8e4dee 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -306,6 +306,7 @@
 	     vty/ok_more_spaces.cfg \
 	     vty/ok_tabs_and_spaces.cfg \
 	     vty/ok_tabs.cfg \
+	     vty/ok_deprecated_logging.cfg \
 	     comp128/comp128_test.ok bits/bitfield_test.ok		\
 	     utils/utils_test.ok utils/utils_test.err stats/stats_test.ok \
 	     bitvec/bitvec_test.ok msgb/msgb_test.ok bits/bitcomp_test.ok \
diff --git a/tests/testsuite.at b/tests/testsuite.at
index c231b96..5865140 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -199,7 +199,10 @@
 AT_KEYWORDS([vty])
 cat $abs_srcdir/vty/vty_test.ok > expout
 cp $abs_srcdir/vty/*.cfg .
-AT_CHECK([$abs_top_builddir/tests/vty/vty_test], [0], [expout], [ignore])
+# FIXME: calling vty_out() during initialization of the VTY interface would cause
+# the process write to its own *stdin*! This breaks the output of 'make check'.
+# Let's work this around untill the bug in libosmovty is fixed.
+AT_CHECK([$abs_top_builddir/tests/vty/vty_test 0>/dev/null], [0], [expout], [ignore])
 AT_CLEANUP
 
 AT_SETUP([gprs-bssgp])
diff --git a/tests/vty/ok_deprecated_logging.cfg b/tests/vty/ok_deprecated_logging.cfg
new file mode 100644
index 0000000..2699719
--- /dev/null
+++ b/tests/vty/ok_deprecated_logging.cfg
@@ -0,0 +1,3 @@
+log stderr
+ logging filter all 1
+ logging level depr debug
diff --git a/tests/vty/vty_test.c b/tests/vty/vty_test.c
index 0d68a6c..1139638 100644
--- a/tests/vty/vty_test.c
+++ b/tests/vty/vty_test.c
@@ -29,6 +29,7 @@
 
 #include <osmocom/core/application.h>
 #include <osmocom/core/talloc.h>
+#include <osmocom/core/logging_internal.h>
 #include <osmocom/core/logging.h>
 #include <osmocom/core/stats.h>
 #include <osmocom/core/utils.h>
@@ -442,6 +443,8 @@
 	install_element(CONFIG_NODE, &cfg_ret_warning_cmd);
 	install_element(CONFIG_NODE, &cfg_ret_success_cmd);
 
+	logging_vty_add_deprecated_subsys(tall_log_ctx, "depr");
+
 	install_element(CONFIG_NODE, &cfg_level1_cmd);
 	install_node(&level1_node, NULL);
 	install_element(LEVEL1_NODE, &cfg_level1_child_cmd);
@@ -544,6 +547,7 @@
 	test_exit_by_indent("ok_indented_root.cfg", 0);
 	test_exit_by_indent("ok_empty_parent.cfg", 0);
 	test_exit_by_indent("fail_cmd_ret_warning.cfg", -EINVAL);
+	test_exit_by_indent("ok_deprecated_logging.cfg", 0);
 
 	test_is_cmd_ambiguous();
 
diff --git a/tests/vty/vty_test.ok b/tests/vty/vty_test.ok
index 0b5ac9c..d2c9611 100644
--- a/tests/vty/vty_test.ok
+++ b/tests/vty/vty_test.ok
@@ -290,6 +290,8 @@
 Called: 'return-success'
 Called: 'return-warning'
 got rc=-22
+reading file ok_deprecated_logging.cfg, expecting rc=0
+got rc=0
 Going to test is_cmd_ambiguous()
 Going to execute 'ambiguous_nr'
 Called: 'ambiguous_nr [<0-23>]' (argc=0)

-- 
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/16140
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ia934581410cd41594791d4e14ee74c16abe1009a
Gerrit-Change-Number: 16140
Gerrit-PatchSet: 3
Gerrit-Owner: fixeria <axilirator at gmail.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <axilirator at gmail.com>
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20191121/227256fd/attachment.htm>


More information about the gerrit-log mailing list