[PATCH v2 0/4] bootconfig: Add mixed subkeys and value under the same key

[PATCH v2 0/4] bootconfig: Add mixed subkeys and value under the same key

[Masami Hiramatsu]

Hi,

Here is the 2nd version of the series which updates bootconfig to
support mixed subkeys and a value under the same key.
Since the kernel cmdline accepts options like 
"aaa.bbb=val1 aaa.bbb.ccc=val2", it is better that the bootconfig
also support it.

In this version, I fixed [1/4] to show /proc/bootconfig correctly
and update the description.

Changes in v2
 [1/4]:
      - Fix /proc/bootconfig accroding to Devin's report (Thanks!)
      - Update patch description to explain why, not what.
      - Use xbc_node_is_array() instead of checking field directly.
 [4/4]:
      - Update an exmaple code.


This series depends on the below bugfix.

 https://lore.kernel.org/lkml/162087519356.442660.11385099982318160180.stgit@devnote2/

With this series, sub-keys and a value can co-exist under a parent key.
For example, following config is allowed.

 foo = value1
 foo.bar = value2

Note, since there is no syntax to put a raw value directly under a
structured key, you have to define it outside of the brace. For example,

 foo {
     bar = value1
     bar {
         baz = value2
         qux = value3
     }
 }

Also, the order of the value node under a key is fixed. If there
are a value and subkeys, the value is always the first child node
of the key. Thus if user specifies subkeys first, e.g.

 foo.bar = value1
 foo = value2

In the program (and /proc/bootconfig), it will be shown as below

 foo = value2
 foo.bar = value1


Thank you,

---

Masami Hiramatsu (4):
      bootconfig: Change array value to use child node
      bootconfig: Support mixing a value and subkeys under a key
      tools/bootconfig: Support mixed value and subkey test cases
      docs: bootconfig: Update for mixing value and subkeys


 Documentation/admin-guide/bootconfig.rst           |   30 +++++++-
 fs/proc/bootconfig.c                               |    2 -
 include/linux/bootconfig.h                         |   38 +++++++++-
 lib/bootconfig.c                                   |   76 +++++++++++++++-----
 tools/bootconfig/main.c                            |   47 ++++++++++--
 tools/bootconfig/samples/bad-mixed-kv1.bconf       |    3 -
 tools/bootconfig/samples/bad-mixed-kv2.bconf       |    3 -
 tools/bootconfig/samples/bad-override.bconf        |    3 -
 tools/bootconfig/samples/bad-override2.bconf       |    3 -
 tools/bootconfig/samples/good-mixed-append.bconf   |    4 +
 tools/bootconfig/samples/good-mixed-kv1.bconf      |    3 +
 tools/bootconfig/samples/good-mixed-kv2.bconf      |    3 +
 tools/bootconfig/samples/good-mixed-kv3.bconf      |    6 ++
 tools/bootconfig/samples/good-mixed-override.bconf |    4 +
 14 files changed, 178 insertions(+), 47 deletions(-)
 delete mode 100644 tools/bootconfig/samples/bad-mixed-kv1.bconf
 delete mode 100644 tools/bootconfig/samples/bad-mixed-kv2.bconf
 delete mode 100644 tools/bootconfig/samples/bad-override.bconf
 delete mode 100644 tools/bootconfig/samples/bad-override2.bconf
 create mode 100644 tools/bootconfig/samples/good-mixed-append.bconf
 create mode 100644 tools/bootconfig/samples/good-mixed-kv1.bconf
 create mode 100644 tools/bootconfig/samples/good-mixed-kv2.bconf
 create mode 100644 tools/bootconfig/samples/good-mixed-kv3.bconf
 create mode 100644 tools/bootconfig/samples/good-mixed-override.bconf

--
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>

[PATCH v2 1/4] bootconfig: Change array value to use child node

[Masami Hiramatsu]

It is not possible to put an array value with subkeys under
a key node, because both of subkeys and the array elements
are using "next" field of the xbc_node.

Thus this changes the array values to use "child" field in
the array case. The reason why split this change is to
test it easily.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 Changes in v2:
  - Fix /proc/bootconfig accroding to Devin's report (Thanks!)
  - Update patch description to explain why, not what.
  - Use xbc_node_is_array() instead of checking field directly.
---
 fs/proc/bootconfig.c       |    2 +-
 include/linux/bootconfig.h |    6 +++---
 lib/bootconfig.c           |   23 +++++++++++++++++++----
 tools/bootconfig/main.c    |    2 +-
 4 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/fs/proc/bootconfig.c b/fs/proc/bootconfig.c
index ad31ec4ad627..6d8d4bf20837 100644
--- a/fs/proc/bootconfig.c
+++ b/fs/proc/bootconfig.c
@@ -49,7 +49,7 @@ static int __init copy_xbc_key_value_list(char *dst, size_t size)
 				else
 					q = '"';
 				ret = snprintf(dst, rest(dst, end), "%c%s%c%s",
-					q, val, q, vnode->next ? ", " : "\n");
+					q, val, q, xbc_node_is_array(vnode) ? ", " : "\n");
 				if (ret < 0)
 					goto out;
 				dst += ret;
diff --git a/include/linux/bootconfig.h b/include/linux/bootconfig.h
index 2696eb0fc149..3178a31fdabc 100644
--- a/include/linux/bootconfig.h
+++ b/include/linux/bootconfig.h
@@ -71,7 +71,7 @@ static inline __init bool xbc_node_is_key(struct xbc_node *node)
  */
 static inline __init bool xbc_node_is_array(struct xbc_node *node)
 {
-	return xbc_node_is_value(node) && node->next != 0;
+	return xbc_node_is_value(node) && node->child != 0;
 }
 
 /**
@@ -140,7 +140,7 @@ static inline struct xbc_node * __init xbc_find_node(const char *key)
  */
 #define xbc_array_for_each_value(anode, value)				\
 	for (value = xbc_node_get_data(anode); anode != NULL ;		\
-	     anode = xbc_node_get_next(anode),				\
+	     anode = xbc_node_get_child(anode),				\
 	     value = anode ? xbc_node_get_data(anode) : NULL)
 
 /**
@@ -171,7 +171,7 @@ static inline struct xbc_node * __init xbc_find_node(const char *key)
  */
 #define xbc_node_for_each_array_value(node, key, anode, value)		\
 	for (value = xbc_node_find_value(node, key, &anode); value != NULL; \
-	     anode = xbc_node_get_next(anode),				\
+	     anode = xbc_node_get_child(anode),				\
 	     value = anode ? xbc_node_get_data(anode) : NULL)
 
 /**
diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index 9f8c70a98fcf..44dcdcbd746a 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -367,6 +367,14 @@ static inline __init struct xbc_node *xbc_last_sibling(struct xbc_node *node)
 	return node;
 }
 
+static inline __init struct xbc_node *xbc_last_child(struct xbc_node *node)
+{
+	while (node->child)
+		node = xbc_node_get_child(node);
+
+	return node;
+}
+
 static struct xbc_node * __init xbc_add_sibling(char *data, u32 flag)
 {
 	struct xbc_node *sib, *node = xbc_add_node(data, flag);
@@ -517,17 +525,20 @@ static int __init xbc_parse_array(char **__v)
 	char *next;
 	int c = 0;
 
+	if (last_parent->child)
+		last_parent = xbc_node_get_child(last_parent);
+
 	do {
 		c = __xbc_parse_value(__v, &next);
 		if (c < 0)
 			return c;
 
-		node = xbc_add_sibling(*__v, XBC_VALUE);
+		node = xbc_add_child(*__v, XBC_VALUE);
 		if (!node)
 			return -ENOMEM;
 		*__v = next;
 	} while (c == ',');
-	node->next = 0;
+	node->child = 0;
 
 	return c;
 }
@@ -615,8 +626,12 @@ static int __init xbc_parse_kv(char **k, char *v, int op)
 
 	if (op == ':' && child) {
 		xbc_init_node(child, v, XBC_VALUE);
-	} else if (!xbc_add_sibling(v, XBC_VALUE))
-		return -ENOMEM;
+	} else {
+		if (op == '+' && child)
+			last_parent = xbc_last_child(child);
+		if (!xbc_add_sibling(v, XBC_VALUE))
+			return -ENOMEM;
+	}
 
 	if (c == ',') {	/* Array */
 		c = xbc_parse_array(&next);
diff --git a/tools/bootconfig/main.c b/tools/bootconfig/main.c
index 7362bef1a368..7a98756e594a 100644
--- a/tools/bootconfig/main.c
+++ b/tools/bootconfig/main.c
@@ -27,7 +27,7 @@ static int xbc_show_value(struct xbc_node *node, bool semicolon)
 			q = '\'';
 		else
 			q = '"';
-		printf("%c%s%c%s", q, val, q, node->next ? ", " : eol);
+		printf("%c%s%c%s", q, val, q, xbc_node_is_array(node) ? ", " : eol);
 		i++;
 	}
 	return i;

[PATCH v2 2/4] bootconfig: Support mixing a value and subkeys under a key

[Masami Hiramatsu]

Support mixing a value and subkeys under a key. Since kernel cmdline
options will support "aaa.bbb=value1 aaa.bbb.ccc=value2", it is
better that the bootconfig supports such configuration too.

Note that this does not change syntax itself but just accepts
mixed value and subkeys e.g.

key = value1
key.subkey = value2

But this is not accepted;

key {
 value1
 subkey = value2
}

That will make value1 as a subkey.

Also, the order of the value node under a key is fixed. If there
are a value and subkeys, the value is always the first child node
of the key. Thus if user specifies subkeys first, e.g.

key.subkey = value1
key = value2

In the program (and /proc/bootconfig), it will be shown as below

key = value2
key.subkey = value1

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 include/linux/bootconfig.h |   32 ++++++++++++++++++++++
 lib/bootconfig.c           |   65 ++++++++++++++++++++++++++++++--------------
 tools/bootconfig/main.c    |   45 +++++++++++++++++++++++++-----
 3 files changed, 114 insertions(+), 28 deletions(-)

diff --git a/include/linux/bootconfig.h b/include/linux/bootconfig.h
index 3178a31fdabc..e49043ac77c9 100644
--- a/include/linux/bootconfig.h
+++ b/include/linux/bootconfig.h
@@ -80,6 +80,8 @@ static inline __init bool xbc_node_is_array(struct xbc_node *node)
  *
  * Test the @node is a leaf key node which is a key node and has a value node
  * or no child. Returns true if it is a leaf node, or false if not.
+ * Note that the leaf node can have subkey nodes in addition to the
+ * value node.
  */
 static inline __init bool xbc_node_is_leaf(struct xbc_node *node)
 {
@@ -129,6 +131,23 @@ static inline struct xbc_node * __init xbc_find_node(const char *key)
 	return xbc_node_find_child(NULL, key);
 }
 
+/**
+ * xbc_node_get_subkey() - Return the first subkey node if exists
+ * @node: Parent node
+ *
+ * Return the first subkey node of the @node. If the @node has no child
+ * or only value node, this will return NULL.
+ */
+static inline struct xbc_node * __init xbc_node_get_subkey(struct xbc_node *node)
+{
+	struct xbc_node *child = xbc_node_get_child(node);
+
+	if (child && xbc_node_is_value(child))
+		return xbc_node_get_next(child);
+	else
+		return child;
+}
+
 /**
  * xbc_array_for_each_value() - Iterate value nodes on an array
  * @anode: An XBC arraied value node
@@ -149,11 +168,24 @@ static inline struct xbc_node * __init xbc_find_node(const char *key)
  * @child: Iterated XBC node.
  *
  * Iterate child nodes of @parent. Each child nodes are stored to @child.
+ * The @child can be mixture of a value node and subkey nodes.
  */
 #define xbc_node_for_each_child(parent, child)				\
 	for (child = xbc_node_get_child(parent); child != NULL ;	\
 	     child = xbc_node_get_next(child))
 
+/**
+ * xbc_node_for_each_subkey() - Iterate child subkey nodes
+ * @parent: An XBC node.
+ * @child: Iterated XBC node.
+ *
+ * Iterate subkey nodes of @parent. Each child nodes are stored to @child.
+ * The @child is only the subkey node.
+ */
+#define xbc_node_for_each_subkey(parent, child)				\
+	for (child = xbc_node_get_subkey(parent); child != NULL ;	\
+	     child = xbc_node_get_next(child))
+
 /**
  * xbc_node_for_each_array_value() - Iterate array entries of geven key
  * @node: An XBC node.
diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index 44dcdcbd746a..927017431fb6 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -156,7 +156,7 @@ xbc_node_find_child(struct xbc_node *parent, const char *key)
 	struct xbc_node *node;
 
 	if (parent)
-		node = xbc_node_get_child(parent);
+		node = xbc_node_get_subkey(parent);
 	else
 		node = xbc_root_node();
 
@@ -164,7 +164,7 @@ xbc_node_find_child(struct xbc_node *parent, const char *key)
 		if (!xbc_node_match_prefix(node, &key))
 			node = xbc_node_get_next(node);
 		else if (*key != '\0')
-			node = xbc_node_get_child(node);
+			node = xbc_node_get_subkey(node);
 		else
 			break;
 	}
@@ -274,6 +274,8 @@ int __init xbc_node_compose_key_after(struct xbc_node *root,
 struct xbc_node * __init xbc_node_find_next_leaf(struct xbc_node *root,
 						 struct xbc_node *node)
 {
+	struct xbc_node *next;
+
 	if (unlikely(!xbc_data))
 		return NULL;
 
@@ -282,6 +284,13 @@ struct xbc_node * __init xbc_node_find_next_leaf(struct xbc_node *root,
 		if (!node)
 			node = xbc_nodes;
 	} else {
+		/* Leaf node may have a subkey */
+		next = xbc_node_get_subkey(node);
+		if (next) {
+			node = next;
+			goto found;
+		}
+
 		if (node == root)	/* @root was a leaf, no child node. */
 			return NULL;
 
@@ -296,6 +305,7 @@ struct xbc_node * __init xbc_node_find_next_leaf(struct xbc_node *root,
 		node = xbc_node_get_next(node);
 	}
 
+found:
 	while (node && !xbc_node_is_leaf(node))
 		node = xbc_node_get_child(node);
 
@@ -375,18 +385,20 @@ static inline __init struct xbc_node *xbc_last_child(struct xbc_node *node)
 	return node;
 }
 
-static struct xbc_node * __init xbc_add_sibling(char *data, u32 flag)
+static struct xbc_node * __init __xbc_add_sibling(char *data, u32 flag, bool head)
 {
 	struct xbc_node *sib, *node = xbc_add_node(data, flag);
 
 	if (node) {
 		if (!last_parent) {
+			/* Ignore @head in this case */
 			node->parent = XBC_NODE_MAX;
 			sib = xbc_last_sibling(xbc_nodes);
 			sib->next = xbc_node_index(node);
 		} else {
 			node->parent = xbc_node_index(last_parent);
-			if (!last_parent->child) {
+			if (!last_parent->child || head) {
+				node->next = last_parent->child;
 				last_parent->child = xbc_node_index(node);
 			} else {
 				sib = xbc_node_get_child(last_parent);
@@ -400,6 +412,16 @@ static struct xbc_node * __init xbc_add_sibling(char *data, u32 flag)
 	return node;
 }
 
+static inline struct xbc_node * __init xbc_add_sibling(char *data, u32 flag)
+{
+	return __xbc_add_sibling(data, flag, false);
+}
+
+static inline struct xbc_node * __init xbc_add_head_sibling(char *data, u32 flag)
+{
+	return __xbc_add_sibling(data, flag, true);
+}
+
 static inline __init struct xbc_node *xbc_add_child(char *data, u32 flag)
 {
 	struct xbc_node *node = xbc_add_sibling(data, flag);
@@ -568,8 +590,9 @@ static int __init __xbc_add_key(char *k)
 		node = find_match_node(xbc_nodes, k);
 	else {
 		child = xbc_node_get_child(last_parent);
+		/* Since the value node is the first child, skip it. */
 		if (child && xbc_node_is_value(child))
-			return xbc_parse_error("Subkey is mixed with value", k);
+			child = xbc_node_get_next(child);
 		node = find_match_node(child, k);
 	}
 
@@ -612,27 +635,29 @@ static int __init xbc_parse_kv(char **k, char *v, int op)
 	if (ret)
 		return ret;
 
-	child = xbc_node_get_child(last_parent);
-	if (child) {
-		if (xbc_node_is_key(child))
-			return xbc_parse_error("Value is mixed with subkey", v);
-		else if (op == '=')
-			return xbc_parse_error("Value is redefined", v);
-	}
-
 	c = __xbc_parse_value(&v, &next);
 	if (c < 0)
 		return c;
 
-	if (op == ':' && child) {
-		xbc_init_node(child, v, XBC_VALUE);
-	} else {
-		if (op == '+' && child)
-			last_parent = xbc_last_child(child);
-		if (!xbc_add_sibling(v, XBC_VALUE))
-			return -ENOMEM;
+	child = xbc_node_get_child(last_parent);
+	if (child && xbc_node_is_value(child)) {
+		if (op == '=')
+			return xbc_parse_error("Value is redefined", v);
+		if (op == ':') {
+			unsigned short nidx = child->next;
+
+			xbc_init_node(child, v, XBC_VALUE);
+			child->next = nidx;	/* keep subkeys */
+			goto array;
+		}
+		/* op must be '+' */
+		last_parent = xbc_last_child(child);
 	}
+	/* The value node should always be the first child */
+	if (!xbc_add_head_sibling(v, XBC_VALUE))
+		return -ENOMEM;
 
+array:
 	if (c == ',') {	/* Array */
 		c = xbc_parse_array(&next);
 		if (c < 0)
diff --git a/tools/bootconfig/main.c b/tools/bootconfig/main.c
index 7a98756e594a..f5b564206428 100644
--- a/tools/bootconfig/main.c
+++ b/tools/bootconfig/main.c
@@ -35,30 +35,55 @@ static int xbc_show_value(struct xbc_node *node, bool semicolon)
 
 static void xbc_show_compact_tree(void)
 {
-	struct xbc_node *node, *cnode;
+	struct xbc_node *node, *cnode = NULL, *vnode;
 	int depth = 0, i;
 
 	node = xbc_root_node();
 	while (node && xbc_node_is_key(node)) {
 		for (i = 0; i < depth; i++)
 			printf("\t");
-		cnode = xbc_node_get_child(node);
+		if (!cnode)
+			cnode = xbc_node_get_child(node);
 		while (cnode && xbc_node_is_key(cnode) && !cnode->next) {
+			vnode = xbc_node_get_child(cnode);
+			/*
+			 * If @cnode has value and subkeys, this
+			 * should show it as below.
+			 *
+			 * key(@node) {
+			 *      key(@cnode) = value;
+			 *      key(@cnode) {
+			 *          subkeys;
+			 *      }
+			 * }
+			 */
+			if (vnode && xbc_node_is_value(vnode) && vnode->next)
+				break;
 			printf("%s.", xbc_node_get_data(node));
 			node = cnode;
-			cnode = xbc_node_get_child(node);
+			cnode = vnode;
 		}
 		if (cnode && xbc_node_is_key(cnode)) {
 			printf("%s {\n", xbc_node_get_data(node));
 			depth++;
 			node = cnode;
+			cnode = NULL;
 			continue;
 		} else if (cnode && xbc_node_is_value(cnode)) {
 			printf("%s = ", xbc_node_get_data(node));
 			xbc_show_value(cnode, true);
+			/*
+			 * If @node has value and subkeys, continue
+			 * looping on subkeys with same node.
+			 */
+			if (cnode->next) {
+				cnode = xbc_node_get_next(cnode);
+				continue;
+			}
 		} else {
 			printf("%s;\n", xbc_node_get_data(node));
 		}
+		cnode = NULL;
 
 		if (node->next) {
 			node = xbc_node_get_next(node);
@@ -70,10 +95,12 @@ static void xbc_show_compact_tree(void)
 				return;
 			if (!xbc_node_get_child(node)->next)
 				continue;
-			depth--;
-			for (i = 0; i < depth; i++)
-				printf("\t");
-			printf("}\n");
+			if (depth) {
+				depth--;
+				for (i = 0; i < depth; i++)
+					printf("\t");
+				printf("}\n");
+			}
 		}
 		node = xbc_node_get_next(node);
 	}
@@ -88,8 +115,10 @@ static void xbc_show_list(void)
 
 	xbc_for_each_key_value(leaf, val) {
 		ret = xbc_node_compose_key(leaf, key, XBC_KEYLEN_MAX);
-		if (ret < 0)
+		if (ret < 0) {
+			fprintf(stderr, "Failed to compose key %d\n", ret);
 			break;
+		}
 		printf("%s = ", key);
 		if (!val || val[0] == '\0') {
 			printf("\"\"\n");

[PATCH v2 3/4] tools/bootconfig: Support mixed value and subkey test cases

[Masami Hiramatsu]

Update test case to support mixed value and subkey on a key.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/bootconfig/samples/bad-mixed-kv1.bconf       |    3 ---
 tools/bootconfig/samples/bad-mixed-kv2.bconf       |    3 ---
 tools/bootconfig/samples/bad-override.bconf        |    3 ---
 tools/bootconfig/samples/bad-override2.bconf       |    3 ---
 tools/bootconfig/samples/good-mixed-append.bconf   |    4 ++++
 tools/bootconfig/samples/good-mixed-kv1.bconf      |    3 +++
 tools/bootconfig/samples/good-mixed-kv2.bconf      |    3 +++
 tools/bootconfig/samples/good-mixed-kv3.bconf      |    6 ++++++
 tools/bootconfig/samples/good-mixed-override.bconf |    4 ++++
 9 files changed, 20 insertions(+), 12 deletions(-)
 delete mode 100644 tools/bootconfig/samples/bad-mixed-kv1.bconf
 delete mode 100644 tools/bootconfig/samples/bad-mixed-kv2.bconf
 delete mode 100644 tools/bootconfig/samples/bad-override.bconf
 delete mode 100644 tools/bootconfig/samples/bad-override2.bconf
 create mode 100644 tools/bootconfig/samples/good-mixed-append.bconf
 create mode 100644 tools/bootconfig/samples/good-mixed-kv1.bconf
 create mode 100644 tools/bootconfig/samples/good-mixed-kv2.bconf
 create mode 100644 tools/bootconfig/samples/good-mixed-kv3.bconf
 create mode 100644 tools/bootconfig/samples/good-mixed-override.bconf

diff --git a/tools/bootconfig/samples/bad-mixed-kv1.bconf b/tools/bootconfig/samples/bad-mixed-kv1.bconf
deleted file mode 100644
index 1761547dd05c..000000000000
--- a/tools/bootconfig/samples/bad-mixed-kv1.bconf
+++ /dev/null
@@ -1,3 +0,0 @@
-# value -> subkey pattern
-key = value
-key.subkey = another-value
diff --git a/tools/bootconfig/samples/bad-mixed-kv2.bconf b/tools/bootconfig/samples/bad-mixed-kv2.bconf
deleted file mode 100644
index 6b32e0c3878c..000000000000
--- a/tools/bootconfig/samples/bad-mixed-kv2.bconf
+++ /dev/null
@@ -1,3 +0,0 @@
-# subkey -> value pattern
-key.subkey = value
-key = another-value
diff --git a/tools/bootconfig/samples/bad-override.bconf b/tools/bootconfig/samples/bad-override.bconf
deleted file mode 100644
index fde6c561512e..000000000000
--- a/tools/bootconfig/samples/bad-override.bconf
+++ /dev/null
@@ -1,3 +0,0 @@
-key.subkey = value
-# We can not override pre-defined subkeys with value
-key := value
diff --git a/tools/bootconfig/samples/bad-override2.bconf b/tools/bootconfig/samples/bad-override2.bconf
deleted file mode 100644
index 688587cb023c..000000000000
--- a/tools/bootconfig/samples/bad-override2.bconf
+++ /dev/null
@@ -1,3 +0,0 @@
-key = value
-# We can not override pre-defined value with subkey
-key.subkey := value
diff --git a/tools/bootconfig/samples/good-mixed-append.bconf b/tools/bootconfig/samples/good-mixed-append.bconf
new file mode 100644
index 000000000000..b99a089a05f5
--- /dev/null
+++ b/tools/bootconfig/samples/good-mixed-append.bconf
@@ -0,0 +1,4 @@
+key = foo
+keyx.subkey = value
+key += bar
+
diff --git a/tools/bootconfig/samples/good-mixed-kv1.bconf b/tools/bootconfig/samples/good-mixed-kv1.bconf
new file mode 100644
index 000000000000..1761547dd05c
--- /dev/null
+++ b/tools/bootconfig/samples/good-mixed-kv1.bconf
@@ -0,0 +1,3 @@
+# value -> subkey pattern
+key = value
+key.subkey = another-value
diff --git a/tools/bootconfig/samples/good-mixed-kv2.bconf b/tools/bootconfig/samples/good-mixed-kv2.bconf
new file mode 100644
index 000000000000..6b32e0c3878c
--- /dev/null
+++ b/tools/bootconfig/samples/good-mixed-kv2.bconf
@@ -0,0 +1,3 @@
+# subkey -> value pattern
+key.subkey = value
+key = another-value
diff --git a/tools/bootconfig/samples/good-mixed-kv3.bconf b/tools/bootconfig/samples/good-mixed-kv3.bconf
new file mode 100644
index 000000000000..2ce2b02224b8
--- /dev/null
+++ b/tools/bootconfig/samples/good-mixed-kv3.bconf
@@ -0,0 +1,6 @@
+# mixed key and subkeys with braces
+key = value
+key {
+	subkey1
+	subkey2 = foo
+}
diff --git a/tools/bootconfig/samples/good-mixed-override.bconf b/tools/bootconfig/samples/good-mixed-override.bconf
new file mode 100644
index 000000000000..18195b2873b6
--- /dev/null
+++ b/tools/bootconfig/samples/good-mixed-override.bconf
@@ -0,0 +1,4 @@
+key.foo = bar
+key = value
+# mixed key value can be overridden
+key := value2

[PATCH v2 4/4] docs: bootconfig: Update for mixing value and subkeys

[Masami Hiramatsu]

Update document for the mixing value and subkeys on a key.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 Changes in v2:
  - Update an example of mixing value and subkeys.
---
 Documentation/admin-guide/bootconfig.rst |   30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/Documentation/admin-guide/bootconfig.rst b/Documentation/admin-guide/bootconfig.rst
index 452b7dcd7f6b..6a79f2e59396 100644
--- a/Documentation/admin-guide/bootconfig.rst
+++ b/Documentation/admin-guide/bootconfig.rst
@@ -89,13 +89,35 @@ you can use ``+=`` operator. For example::
 
 In this case, the key ``foo`` has ``bar``, ``baz`` and ``qux``.
 
-However, a sub-key and a value can not co-exist under a parent key.
-For example, following config is NOT allowed.::
+Moreover, sub-keys and a value can coexist under a parent key.
+For example, following config is allowed.::
 
  foo = value1
- foo.bar = value2 # !ERROR! subkey "bar" and value "value1" can NOT co-exist
- foo.bar := value2 # !ERROR! even with the override operator, this is NOT allowed.
+ foo.bar = value2
+ foo := value3 # This will update foo's value.
+
+Note, since there is no syntax to put a raw value directly under a
+structured key, you have to define it outside of the brace. For example::
+
+ foo {
+     bar = value1
+     bar {
+         baz = value2
+         qux = value3
+     }
+ }
+
+Also, the order of the value node under a key is fixed. If there
+are a value and subkeys, the value is always the first child node
+of the key. Thus if user specifies subkeys first, e.g.::
+
+ foo.bar = value1
+ foo = value2
+
+In the program (and /proc/bootconfig), it will be shown as below::
 
+ foo = value2
+ foo.bar = value1
 
 Comments
 --------

Re: [PATCH v2 0/4] bootconfig: Add mixed subkeys and value under the same key

[Devin Moore]

On Thu, May 20, 2021 at 11:34 PM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> Hi,
>
> Here is the 2nd version of the series which updates bootconfig to
> support mixed subkeys and a value under the same key.
> Since the kernel cmdline accepts options like
> "aaa.bbb=val1 aaa.bbb.ccc=val2", it is better that the bootconfig
> also support it.
>
> In this version, I fixed [1/4] to show /proc/bootconfig correctly
> and update the description.
>
> Changes in v2
>  [1/4]:
>       - Fix /proc/bootconfig accroding to Devin's report (Thanks!)
>       - Update patch description to explain why, not what.
>       - Use xbc_node_is_array() instead of checking field directly.
>  [4/4]:
>       - Update an exmaple code.
>
>
> This series depends on the below bugfix.
>
>  https://lore.kernel.org/lkml/162087519356.442660.11385099982318160180.stgit@devnote2/
>
> With this series, sub-keys and a value can co-exist under a parent key.
> For example, following config is allowed.
>
>  foo = value1
>  foo.bar = value2
>
> Note, since there is no syntax to put a raw value directly under a
> structured key, you have to define it outside of the brace. For example,
>
>  foo {
>      bar = value1
>      bar {
>          baz = value2
>          qux = value3
>      }
>  }
>
> Also, the order of the value node under a key is fixed. If there
> are a value and subkeys, the value is always the first child node
> of the key. Thus if user specifies subkeys first, e.g.
>
>  foo.bar = value1
>  foo = value2
>
> In the program (and /proc/bootconfig), it will be shown as below
>
>  foo = value2
>  foo.bar = value1
>
>
> Thank you,
>
> ---
>
> Masami Hiramatsu (4):
>       bootconfig: Change array value to use child node
>       bootconfig: Support mixing a value and subkeys under a key
>       tools/bootconfig: Support mixed value and subkey test cases
>       docs: bootconfig: Update for mixing value and subkeys
>
>
>  Documentation/admin-guide/bootconfig.rst           |   30 +++++++-
>  fs/proc/bootconfig.c                               |    2 -
>  include/linux/bootconfig.h                         |   38 +++++++++-
>  lib/bootconfig.c                                   |   76 +++++++++++++++-----
>  tools/bootconfig/main.c                            |   47 ++++++++++--
>  tools/bootconfig/samples/bad-mixed-kv1.bconf       |    3 -
>  tools/bootconfig/samples/bad-mixed-kv2.bconf       |    3 -
>  tools/bootconfig/samples/bad-override.bconf        |    3 -
>  tools/bootconfig/samples/bad-override2.bconf       |    3 -
>  tools/bootconfig/samples/good-mixed-append.bconf   |    4 +
>  tools/bootconfig/samples/good-mixed-kv1.bconf      |    3 +
>  tools/bootconfig/samples/good-mixed-kv2.bconf      |    3 +
>  tools/bootconfig/samples/good-mixed-kv3.bconf      |    6 ++
>  tools/bootconfig/samples/good-mixed-override.bconf |    4 +
>  14 files changed, 178 insertions(+), 47 deletions(-)
>  delete mode 100644 tools/bootconfig/samples/bad-mixed-kv1.bconf
>  delete mode 100644 tools/bootconfig/samples/bad-mixed-kv2.bconf
>  delete mode 100644 tools/bootconfig/samples/bad-override.bconf
>  delete mode 100644 tools/bootconfig/samples/bad-override2.bconf
>  create mode 100644 tools/bootconfig/samples/good-mixed-append.bconf
>  create mode 100644 tools/bootconfig/samples/good-mixed-kv1.bconf
>  create mode 100644 tools/bootconfig/samples/good-mixed-kv2.bconf
>  create mode 100644 tools/bootconfig/samples/good-mixed-kv3.bconf
>  create mode 100644 tools/bootconfig/samples/good-mixed-override.bconf
>
> --
> Masami Hiramatsu (Linaro) <mhiramat@kernel.org>

Thanks a lot!
I tested this on an Android virtual device(Cuttlefish) with a variety of
parameters that included array values. I checked the output in /proc/bootconfig
before and after the changes.
I added a new parameter that failed before these changes and it worked great!
Added 'androidboot.hardware=cutf_cvm ' which worked with
'androidboot.hardware.gralloc = "minigbm"' that was already present.
So,

Tested-by: Devin Moore <devinmoore@google.com>

Re: [PATCH v2 0/4] bootconfig: Add mixed subkeys and value under the same key

[Masami Hiramatsu]

On Fri, 21 May 2021 15:06:22 -0700
Devin Moore <devinmoore@google.com> wrote:

> On Thu, May 20, 2021 at 11:34 PM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> >
> > Hi,
> >
> > Here is the 2nd version of the series which updates bootconfig to
> > support mixed subkeys and a value under the same key.
> > Since the kernel cmdline accepts options like
> > "aaa.bbb=val1 aaa.bbb.ccc=val2", it is better that the bootconfig
> > also support it.
> >
> > In this version, I fixed [1/4] to show /proc/bootconfig correctly
> > and update the description.
> >
> > Changes in v2
> >  [1/4]:
> >       - Fix /proc/bootconfig accroding to Devin's report (Thanks!)
> >       - Update patch description to explain why, not what.
> >       - Use xbc_node_is_array() instead of checking field directly.
> >  [4/4]:
> >       - Update an exmaple code.
> >
> >
> > This series depends on the below bugfix.
> >
> >  https://lore.kernel.org/lkml/162087519356.442660.11385099982318160180.stgit@devnote2/
> >
> > With this series, sub-keys and a value can co-exist under a parent key.
> > For example, following config is allowed.
> >
> >  foo = value1
> >  foo.bar = value2
> >
> > Note, since there is no syntax to put a raw value directly under a
> > structured key, you have to define it outside of the brace. For example,
> >
> >  foo {
> >      bar = value1
> >      bar {
> >          baz = value2
> >          qux = value3
> >      }
> >  }
> >
> > Also, the order of the value node under a key is fixed. If there
> > are a value and subkeys, the value is always the first child node
> > of the key. Thus if user specifies subkeys first, e.g.
> >
> >  foo.bar = value1
> >  foo = value2
> >
> > In the program (and /proc/bootconfig), it will be shown as below
> >
> >  foo = value2
> >  foo.bar = value1
> >
> >
> > Thank you,
> >
> > ---
> >
> > Masami Hiramatsu (4):
> >       bootconfig: Change array value to use child node
> >       bootconfig: Support mixing a value and subkeys under a key
> >       tools/bootconfig: Support mixed value and subkey test cases
> >       docs: bootconfig: Update for mixing value and subkeys
> >
> >
> >  Documentation/admin-guide/bootconfig.rst           |   30 +++++++-
> >  fs/proc/bootconfig.c                               |    2 -
> >  include/linux/bootconfig.h                         |   38 +++++++++-
> >  lib/bootconfig.c                                   |   76 +++++++++++++++-----
> >  tools/bootconfig/main.c                            |   47 ++++++++++--
> >  tools/bootconfig/samples/bad-mixed-kv1.bconf       |    3 -
> >  tools/bootconfig/samples/bad-mixed-kv2.bconf       |    3 -
> >  tools/bootconfig/samples/bad-override.bconf        |    3 -
> >  tools/bootconfig/samples/bad-override2.bconf       |    3 -
> >  tools/bootconfig/samples/good-mixed-append.bconf   |    4 +
> >  tools/bootconfig/samples/good-mixed-kv1.bconf      |    3 +
> >  tools/bootconfig/samples/good-mixed-kv2.bconf      |    3 +
> >  tools/bootconfig/samples/good-mixed-kv3.bconf      |    6 ++
> >  tools/bootconfig/samples/good-mixed-override.bconf |    4 +
> >  14 files changed, 178 insertions(+), 47 deletions(-)
> >  delete mode 100644 tools/bootconfig/samples/bad-mixed-kv1.bconf
> >  delete mode 100644 tools/bootconfig/samples/bad-mixed-kv2.bconf
> >  delete mode 100644 tools/bootconfig/samples/bad-override.bconf
> >  delete mode 100644 tools/bootconfig/samples/bad-override2.bconf
> >  create mode 100644 tools/bootconfig/samples/good-mixed-append.bconf
> >  create mode 100644 tools/bootconfig/samples/good-mixed-kv1.bconf
> >  create mode 100644 tools/bootconfig/samples/good-mixed-kv2.bconf
> >  create mode 100644 tools/bootconfig/samples/good-mixed-kv3.bconf
> >  create mode 100644 tools/bootconfig/samples/good-mixed-override.bconf
> >
> > --
> > Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
> 
> Thanks a lot!
> I tested this on an Android virtual device(Cuttlefish) with a variety of
> parameters that included array values. I checked the output in /proc/bootconfig
> before and after the changes.
> I added a new parameter that failed before these changes and it worked great!
> Added 'androidboot.hardware=cutf_cvm ' which worked with
> 'androidboot.hardware.gralloc = "minigbm"' that was already present.
> So,
> 
> Tested-by: Devin Moore <devinmoore@google.com>

Thanks Devin for testing it!


-- 
Masami Hiramatsu <mhiramat@kernel.org>