[PATCH] libosmocore[master]: ctrl_type_vals: fix range check, explicitly terminate

[Neels Hofmeyr]

Hello Jenkins Builder,

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

    https://gerrit.osmocom.org/1962

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

ctrl_type_vals: fix range check, explicitly terminate

In ctrl_cmd_parse(), fix missing check for not parseable ctrl type.

Don't use CTRL_TYPE_UNKNOWN as value_string[] terminator, use an explicit, more
obvious { 0, NULL } termination. Set an explicit string for CTRL_TYPE_UNKNOWN.
No other value_string[]s to date have such a "hidden" terminator.

BTW, a { 0, "string" } item is not a terminator, only { 0, NULL } is, so we can
set a string for CTRL_TYPE_UNKNOWN == 0.

Also, having a string value for CTRL_TYPE_UNKNOWN is not harmful because all
code paths explicitly check for the CTRL_TYPE_*s that are valid.

Adjust the test expectation.

>From the ctrl_type_vals enum, remove the = 0, because it is implicitly 0
anyway.

One motivation to press this fixup: I am trying to add a script that checks
whether all value_string[]s are terminated to our jenkins jobs, and to find
that this one is terminated, it would need to interpret the CTRL_TYPE_UNKNOWN
constant, which would make things far more complex. At this point, all of the
value_string[]s have an explicit termination, and I would like to enforce this
from now on -- for readable code and to not spend more time on the validator.

The patch adding ctrl_type_vals (Icd4e96dd9f00876cb70b43cfcf42ab4f10311b28) was
accepted by another reviewer before I could reconfirm my -1, so this is a fixup
to enable the termination checking script patches.

Related: I2bc93ab4781487e7685cfb63091a489cd126b1a8 (adds script to libosmocore)
         I7fe3678b524d602fc6aa14bc0ed06308df809a3e (uses in jenkins.sh)
	 Icd4e96dd9f00876cb70b43cfcf42ab4f10311b28 (adds ctrl_type_vals)

Change-Id: Ia99f37464c7b36b587da2cc78f52c82725f02cbc
---
M include/osmocom/ctrl/control_cmd.h
M src/ctrl/control_cmd.c
M tests/ctrl/ctrl_test.ok
3 files changed, 5 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/62/1962/2

diff --git a/include/osmocom/ctrl/control_cmd.h b/include/osmocom/ctrl/control_cmd.h
index d82c9af..d9092f3 100644
--- a/include/osmocom/ctrl/control_cmd.h
+++ b/include/osmocom/ctrl/control_cmd.h
@@ -22,7 +22,7 @@
 };
 
 enum ctrl_type {
-	CTRL_TYPE_UNKNOWN = 0,
+	CTRL_TYPE_UNKNOWN,
 	CTRL_TYPE_GET,
 	CTRL_TYPE_SET,
 	CTRL_TYPE_GET_REPLY,
diff --git a/src/ctrl/control_cmd.c b/src/ctrl/control_cmd.c
index 1cc8247..d706995 100644
--- a/src/ctrl/control_cmd.c
+++ b/src/ctrl/control_cmd.c
@@ -40,13 +40,14 @@
 extern vector ctrl_node_vec;
 
 const struct value_string ctrl_type_vals[] = {
+	{ CTRL_TYPE_UNKNOWN,	"(unknown)" },
 	{ CTRL_TYPE_GET,	"GET" },
 	{ CTRL_TYPE_SET,	"SET" },
 	{ CTRL_TYPE_GET_REPLY,	"GET_REPLY" },
 	{ CTRL_TYPE_SET_REPLY,	"SET_REPLY" },
 	{ CTRL_TYPE_TRAP,	"TRAP" },
 	{ CTRL_TYPE_ERROR,	"ERROR" },
-	{ CTRL_TYPE_UNKNOWN,	NULL }
+	{ 0, NULL }
 };
 
 /* Functions from libosmocom */
@@ -289,7 +290,7 @@
 	}
 
 	cmd->type = get_string_value(ctrl_type_vals, tmp);
-	if (cmd->type == CTRL_TYPE_UNKNOWN) {
+	if (cmd->type < 0 || cmd->type == CTRL_TYPE_UNKNOWN) {
 		cmd->type = CTRL_TYPE_ERROR;
 		cmd->id = "err";
 		cmd->reply = "Request type unknown";
diff --git a/tests/ctrl/ctrl_test.ok b/tests/ctrl/ctrl_test.ok
index c4b05e9..8f97a27 100644
--- a/tests/ctrl/ctrl_test.ok
+++ b/tests/ctrl/ctrl_test.ok
@@ -1,5 +1,5 @@
 Checking ctrl types...
-ctrl type 0 is unknown 0x0 [PARSE FAILED]
+ctrl type 0 is (unknown) -> 0 OK
 ctrl type 1 is GET -> 1 OK
 ctrl type 2 is SET -> 2 OK
 ctrl type 3 is GET_REPLY -> 3 OK

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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia99f37464c7b36b587da2cc78f52c82725f02cbc
Gerrit-PatchSet: 2
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder