[PATCH v3 0/2] bootconfig: Handle an empty value

[PATCH v3 0/2] bootconfig: Handle an empty value

[Masami Hiramatsu (Google)]

This is the 3rd version of the update of the bootconfig parser to
make it handle an empty value which is terminated by a newline or
a comment. The previous version is here:

https://lore.kernel.org/all/177019401833.80694.554894321526842218.stgit@devnote2/

This version fixes typos and add 2 bad examples which were
previously good[1/2].

Thank you,

---

Masami Hiramatsu (Google) (2):
      bootconfig: Terminate value search if it hits a newline
      bootconfig: Check the parsed output of the good examples


 .../samples/bad-array-after-comment.bconf          |    4 ++++
 .../samples/bad-array-in-next-line.bconf           |    4 ++++
 .../samples/exp-good-array-space-comment.bconf     |    1 +
 .../samples/exp-good-comment-after-value.bconf     |    1 +
 .../bootconfig/samples/exp-good-mixed-append.bconf |    2 ++
 tools/bootconfig/samples/exp-good-mixed-kv1.bconf  |    2 ++
 tools/bootconfig/samples/exp-good-mixed-kv2.bconf  |    2 ++
 tools/bootconfig/samples/exp-good-mixed-kv3.bconf  |    5 +++++
 .../samples/exp-good-mixed-override.bconf          |    2 ++
 tools/bootconfig/samples/exp-good-override.bconf   |    4 ++++
 tools/bootconfig/samples/exp-good-printables.bconf |    2 ++
 tools/bootconfig/samples/exp-good-simple.bconf     |    8 ++++++++
 tools/bootconfig/samples/exp-good-single.bconf     |    3 +++
 .../samples/exp-good-space-after-value.bconf       |    1 +
 tools/bootconfig/samples/exp-good-tree.bconf       |    8 ++++++++
 .../samples/good-array-space-comment.bconf         |    3 +--
 tools/bootconfig/test-bootconfig.sh                |    3 +++
 17 files changed, 53 insertions(+), 2 deletions(-)
 create mode 100644 tools/bootconfig/samples/bad-array-after-comment.bconf
 create mode 100644 tools/bootconfig/samples/bad-array-in-next-line.bconf
 create mode 100644 tools/bootconfig/samples/exp-good-array-space-comment.bconf
 create mode 100644 tools/bootconfig/samples/exp-good-comment-after-value.bconf
 create mode 100644 tools/bootconfig/samples/exp-good-mixed-append.bconf
 create mode 100644 tools/bootconfig/samples/exp-good-mixed-kv1.bconf
 create mode 100644 tools/bootconfig/samples/exp-good-mixed-kv2.bconf
 create mode 100644 tools/bootconfig/samples/exp-good-mixed-kv3.bconf
 create mode 100644 tools/bootconfig/samples/exp-good-mixed-override.bconf
 create mode 100644 tools/bootconfig/samples/exp-good-override.bconf
 create mode 100644 tools/bootconfig/samples/exp-good-printables.bconf
 create mode 100644 tools/bootconfig/samples/exp-good-simple.bconf
 create mode 100644 tools/bootconfig/samples/exp-good-single.bconf
 create mode 100644 tools/bootconfig/samples/exp-good-space-after-value.bconf
 create mode 100644 tools/bootconfig/samples/exp-good-tree.bconf

--
Masami Hiramatsu (Google) <mhiramat@google.com>

[PATCH v3 1/2] bootconfig: Terminate value search if it hits a newline

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Terminate the value search for a key if it hits a newline and make
the value empty.

When we pass a bootconfig with an empty value terminated by the
newline, like below::

  foo =
  bar = value

Current bootconfig interprets it as a single entry::

  foo = "bar = value";

The Documentation/admin-guide/bootconfig.rst defines the value
itself is terminated by newline:

  The value has to be terminated by semi-colon (``;``) or newline (``\n``).

but it does not define when the value search is terminated.
This changes the behavior to be more line-oriented, so that it is
clearer in how it works.

- The value search of key-value pair will be terminated by a comment
  or newline.
- The value search of an array will continue beyond comments and
  newlines.

Thus, with this update, the above example is interpreted as::

  foo = "";
  bar = "value";

And the below example will cause a syntax error because "bar" is expected
as a key but it has ','.

  foo =
    bar, buz

According to this change, one wrong example config is updated.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
---
 Changes in v3:
  - Fix typos.
  - Add bad examples (which was good previously).
 Changes in v2:
  - Fix to handle multi-line array case correctly.
  - Make this as a spec update, not fix.
---
 .../samples/bad-array-after-comment.bconf          |    4 ++++
 .../samples/bad-array-in-next-line.bconf           |    4 ++++
 .../samples/good-array-space-comment.bconf         |    3 +--
 3 files changed, 9 insertions(+), 2 deletions(-)
 create mode 100644 tools/bootconfig/samples/bad-array-after-comment.bconf
 create mode 100644 tools/bootconfig/samples/bad-array-in-next-line.bconf

diff --git a/Documentation/admin-guide/bootconfig.rst b/Documentation/admin-guide/bootconfig.rst
index 7a86042c9b6d..f712758472d5 100644
--- a/Documentation/admin-guide/bootconfig.rst
+++ b/Documentation/admin-guide/bootconfig.rst
@@ -20,18 +20,26 @@ Config File Syntax
 
 The boot config syntax is a simple structured key-value. Each key consists
 of dot-connected-words, and key and value are connected by ``=``. The value
-has to be terminated by semi-colon (``;``) or newline (``\n``).
-For array value, array entries are separated by comma (``,``). ::
-
-  KEY[.WORD[...]] = VALUE[, VALUE2[...]][;]
-
-Unlike the kernel command line syntax, spaces are OK around the comma and ``=``.
+string has to be terminated by the following delimiters described below.
 
 Each key word must contain only alphabets, numbers, dash (``-``) or underscore
 (``_``). And each value only contains printable characters or spaces except
 for delimiters such as semi-colon (``;``), new-line (``\n``), comma (``,``),
 hash (``#``) and closing brace (``}``).
 
+If the ``=`` is followed by whitespace up to one of these delimiters, the
+key is assigned an empty value.
+
+For arrays, the array values are comma (``,``) separated, and comments and
+line breaks with newline (``\n``) are allowed between array values for
+readability. Thus the first entry of the array must be on the same line as
+the key.::
+
+  KEY[.WORD[...]] = VALUE[, VALUE2[...]][;]
+
+Unlike the kernel command line syntax, white spaces (including tabs) are
+ignored around the comma and ``=``.
+
 If you want to use those delimiters in a value, you can use either double-
 quotes (``"VALUE"``) or single-quotes (``'VALUE'``) to quote it. Note that
 you can not escape these quotes.
@@ -138,8 +146,8 @@ This is parsed as below::
  foo = value
  bar = 1, 2, 3
 
-Note that you can not put a comment between value and delimiter(``,`` or
-``;``). This means following config has a syntax error ::
+Note that you can NOT put a comment or a newline between value and delimiter
+(``,`` or ``;``). This means following config has a syntax error ::
 
  key = 1 # comment
        ,2
diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index 81f29c29f47b..449369a60846 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -557,17 +557,13 @@ static int __init __xbc_close_brace(char *p)
 /*
  * Return delimiter or error, no node added. As same as lib/cmdline.c,
  * you can use " around spaces, but can't escape " for value.
+ * *@__v must point real value string. (not including spaces before value.)
  */
 static int __init __xbc_parse_value(char **__v, char **__n)
 {
 	char *p, *v = *__v;
 	int c, quotes = 0;
 
-	v = skip_spaces(v);
-	while (*v == '#') {
-		v = skip_comment(v);
-		v = skip_spaces(v);
-	}
 	if (*v == '"' || *v == '\'') {
 		quotes = *v;
 		v++;
@@ -617,6 +613,13 @@ static int __init xbc_parse_array(char **__v)
 		last_parent = xbc_node_get_child(last_parent);
 
 	do {
+		/* Search the next array value beyond comments and empty lines */
+		next = skip_spaces(*__v);
+		while (*next == '#') {
+			next = skip_comment(next);
+			next = skip_spaces(next);
+		}
+		*__v = next;
 		c = __xbc_parse_value(__v, &next);
 		if (c < 0)
 			return c;
@@ -701,9 +704,17 @@ static int __init xbc_parse_kv(char **k, char *v, int op)
 	if (ret)
 		return ret;
 
-	c = __xbc_parse_value(&v, &next);
-	if (c < 0)
-		return c;
+	v = skip_spaces_until_newline(v);
+	/* If there is a comment, this has an empty value. */
+	if (*v == '#') {
+		next = skip_comment(v);
+		*v = '\0';
+		c = '\n';
+	} else {
+		c = __xbc_parse_value(&v, &next);
+		if (c < 0)
+			return c;
+	}
 
 	child = xbc_node_get_child(last_parent);
 	if (child && xbc_node_is_value(child)) {
diff --git a/tools/bootconfig/samples/bad-array-after-comment.bconf b/tools/bootconfig/samples/bad-array-after-comment.bconf
new file mode 100644
index 000000000000..fdb6d4e04447
--- /dev/null
+++ b/tools/bootconfig/samples/bad-array-after-comment.bconf
@@ -0,0 +1,4 @@
+# the first array value must be on the same line as the key
+key = # comment
+ value1,
+ value2
diff --git a/tools/bootconfig/samples/bad-array-in-next-line.bconf b/tools/bootconfig/samples/bad-array-in-next-line.bconf
new file mode 100644
index 000000000000..95a99a3bde8c
--- /dev/null
+++ b/tools/bootconfig/samples/bad-array-in-next-line.bconf
@@ -0,0 +1,4 @@
+# the first array value must be on the same line as the key
+key =
+  value1,
+  value2
diff --git a/tools/bootconfig/samples/good-array-space-comment.bconf b/tools/bootconfig/samples/good-array-space-comment.bconf
index 45b938dc0695..416fa2ed4109 100644
--- a/tools/bootconfig/samples/good-array-space-comment.bconf
+++ b/tools/bootconfig/samples/good-array-space-comment.bconf
@@ -1,4 +1,3 @@
-key =	# comment
-	"value1",	  # comment1
+key = "value1",	  # comment1
 	"value2"	 , # comment2
 	"value3"

[PATCH v3 2/2] bootconfig: Check the parsed output of the good examples

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Check whether the parsed output of the good example configs are
the same as expected.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Tested-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 .../samples/exp-good-array-space-comment.bconf     |    1 +
 .../samples/exp-good-comment-after-value.bconf     |    1 +
 .../bootconfig/samples/exp-good-mixed-append.bconf |    2 ++
 tools/bootconfig/samples/exp-good-mixed-kv1.bconf  |    2 ++
 tools/bootconfig/samples/exp-good-mixed-kv2.bconf  |    2 ++
 tools/bootconfig/samples/exp-good-mixed-kv3.bconf  |    5 +++++
 .../samples/exp-good-mixed-override.bconf          |    2 ++
 tools/bootconfig/samples/exp-good-override.bconf   |    4 ++++
 tools/bootconfig/samples/exp-good-printables.bconf |    2 ++
 tools/bootconfig/samples/exp-good-simple.bconf     |    8 ++++++++
 tools/bootconfig/samples/exp-good-single.bconf     |    3 +++
 .../samples/exp-good-space-after-value.bconf       |    1 +
 tools/bootconfig/samples/exp-good-tree.bconf       |    8 ++++++++
 tools/bootconfig/test-bootconfig.sh                |    3 +++
 14 files changed, 44 insertions(+)
 create mode 100644 tools/bootconfig/samples/exp-good-array-space-comment.bconf
 create mode 100644 tools/bootconfig/samples/exp-good-comment-after-value.bconf
 create mode 100644 tools/bootconfig/samples/exp-good-mixed-append.bconf
 create mode 100644 tools/bootconfig/samples/exp-good-mixed-kv1.bconf
 create mode 100644 tools/bootconfig/samples/exp-good-mixed-kv2.bconf
 create mode 100644 tools/bootconfig/samples/exp-good-mixed-kv3.bconf
 create mode 100644 tools/bootconfig/samples/exp-good-mixed-override.bconf
 create mode 100644 tools/bootconfig/samples/exp-good-override.bconf
 create mode 100644 tools/bootconfig/samples/exp-good-printables.bconf
 create mode 100644 tools/bootconfig/samples/exp-good-simple.bconf
 create mode 100644 tools/bootconfig/samples/exp-good-single.bconf
 create mode 100644 tools/bootconfig/samples/exp-good-space-after-value.bconf
 create mode 100644 tools/bootconfig/samples/exp-good-tree.bconf

diff --git a/tools/bootconfig/samples/exp-good-array-space-comment.bconf b/tools/bootconfig/samples/exp-good-array-space-comment.bconf
new file mode 100644
index 000000000000..8d3278fa6af5
--- /dev/null
+++ b/tools/bootconfig/samples/exp-good-array-space-comment.bconf
@@ -0,0 +1 @@
+key = "value1", "value2", "value3";
diff --git a/tools/bootconfig/samples/exp-good-comment-after-value.bconf b/tools/bootconfig/samples/exp-good-comment-after-value.bconf
new file mode 100644
index 000000000000..a8e8450db3c0
--- /dev/null
+++ b/tools/bootconfig/samples/exp-good-comment-after-value.bconf
@@ -0,0 +1 @@
+key = "value";
diff --git a/tools/bootconfig/samples/exp-good-mixed-append.bconf b/tools/bootconfig/samples/exp-good-mixed-append.bconf
new file mode 100644
index 000000000000..c2b407901ddd
--- /dev/null
+++ b/tools/bootconfig/samples/exp-good-mixed-append.bconf
@@ -0,0 +1,2 @@
+key = "foo", "bar";
+keyx.subkey = "value";
diff --git a/tools/bootconfig/samples/exp-good-mixed-kv1.bconf b/tools/bootconfig/samples/exp-good-mixed-kv1.bconf
new file mode 100644
index 000000000000..8346287d9251
--- /dev/null
+++ b/tools/bootconfig/samples/exp-good-mixed-kv1.bconf
@@ -0,0 +1,2 @@
+key = "value";
+key.subkey = "another-value";
diff --git a/tools/bootconfig/samples/exp-good-mixed-kv2.bconf b/tools/bootconfig/samples/exp-good-mixed-kv2.bconf
new file mode 100644
index 000000000000..40c6232c7cdd
--- /dev/null
+++ b/tools/bootconfig/samples/exp-good-mixed-kv2.bconf
@@ -0,0 +1,2 @@
+key = "another-value";
+key.subkey = "value";
diff --git a/tools/bootconfig/samples/exp-good-mixed-kv3.bconf b/tools/bootconfig/samples/exp-good-mixed-kv3.bconf
new file mode 100644
index 000000000000..8368a7bef60a
--- /dev/null
+++ b/tools/bootconfig/samples/exp-good-mixed-kv3.bconf
@@ -0,0 +1,5 @@
+key = "value";
+key {
+	subkey1;
+	subkey2 = "foo";
+}
diff --git a/tools/bootconfig/samples/exp-good-mixed-override.bconf b/tools/bootconfig/samples/exp-good-mixed-override.bconf
new file mode 100644
index 000000000000..58757712ca45
--- /dev/null
+++ b/tools/bootconfig/samples/exp-good-mixed-override.bconf
@@ -0,0 +1,2 @@
+key = "value2";
+key.foo = "bar";
diff --git a/tools/bootconfig/samples/exp-good-override.bconf b/tools/bootconfig/samples/exp-good-override.bconf
new file mode 100644
index 000000000000..00bbd30e99ae
--- /dev/null
+++ b/tools/bootconfig/samples/exp-good-override.bconf
@@ -0,0 +1,4 @@
+key {
+	word = "2", "3";
+	new.word = "new";
+}
diff --git a/tools/bootconfig/samples/exp-good-printables.bconf b/tools/bootconfig/samples/exp-good-printables.bconf
new file mode 100644
index 000000000000..5981d304eacb
--- /dev/null
+++ b/tools/bootconfig/samples/exp-good-printables.bconf
@@ -0,0 +1,2 @@
+key = "	
+\v\f !#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwxyz{|}~";
diff --git a/tools/bootconfig/samples/exp-good-simple.bconf b/tools/bootconfig/samples/exp-good-simple.bconf
new file mode 100644
index 000000000000..d17f39421c86
--- /dev/null
+++ b/tools/bootconfig/samples/exp-good-simple.bconf
@@ -0,0 +1,8 @@
+key {
+	word1 = "1";
+	word2 = "2";
+	word3 = "3";
+	word4 = "4";
+	word5 = "5";
+	word6 = "6";
+}
diff --git a/tools/bootconfig/samples/exp-good-single.bconf b/tools/bootconfig/samples/exp-good-single.bconf
new file mode 100644
index 000000000000..01196910d7f4
--- /dev/null
+++ b/tools/bootconfig/samples/exp-good-single.bconf
@@ -0,0 +1,3 @@
+key = "1";
+key2 = "2";
+key3 = "alpha", "beta";
diff --git a/tools/bootconfig/samples/exp-good-space-after-value.bconf b/tools/bootconfig/samples/exp-good-space-after-value.bconf
new file mode 100644
index 000000000000..a8e8450db3c0
--- /dev/null
+++ b/tools/bootconfig/samples/exp-good-space-after-value.bconf
@@ -0,0 +1 @@
+key = "value";
diff --git a/tools/bootconfig/samples/exp-good-tree.bconf b/tools/bootconfig/samples/exp-good-tree.bconf
new file mode 100644
index 000000000000..b711d38d86fd
--- /dev/null
+++ b/tools/bootconfig/samples/exp-good-tree.bconf
@@ -0,0 +1,8 @@
+key {
+	word.tree.value = "0";
+	word2.tree.value = "1", "2";
+}
+other.tree {
+	value = "2";
+	value2 = "3";
+}
diff --git a/tools/bootconfig/test-bootconfig.sh b/tools/bootconfig/test-bootconfig.sh
index 7594659af1e1..be9bd18b1d56 100755
--- a/tools/bootconfig/test-bootconfig.sh
+++ b/tools/bootconfig/test-bootconfig.sh
@@ -179,6 +179,9 @@ done
 echo "=== expected success cases ==="
 for i in samples/good-* ; do
   xpass $BOOTCONF -a $i $INITRD
+  x="samples/exp-"`basename $i`
+  $BOOTCONF $i > $TEMPCONF
+  xpass diff $x $TEMPCONF
 done