[PATCH] bootconfig: Fix to terminate value search if it hits a newline

Masami Hiramatsu (Google) posted 1 patch 1 week, 1 day ago
tools/bootconfig/test-bootconfig.sh |   18 ++++++++++++++++++
1 file changed, 18 insertions(+)
[PATCH] bootconfig: Fix to terminate value search if it hits a newline
Posted by Masami Hiramatsu (Google) 1 week, 1 day ago
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Fix bootconfig to terminate the value search if it hits a newline
even if it is 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";

However, the Documentation/admin-guide/bootconfig.rst said that

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

Thus, it should be interpreted as::

  foo = "";
  bar = "value";

Note that if the line has a comment, it still keep searching the
value to the next line::

  foo = # comment
  value

This is interpreted as `foo = "value"`.
This also updates the test script so that it can check the above cases.

Fixes: 76db5a27a827 ("bootconfig: Add Extra Boot Config support")
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 tools/bootconfig/test-bootconfig.sh |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index 81f29c29f47b..94b55ec32444 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -563,7 +563,7 @@ static int __init __xbc_parse_value(char **__v, char **__n)
 	char *p, *v = *__v;
 	int c, quotes = 0;
 
-	v = skip_spaces(v);
+	v = skip_spaces_until_newline(v);
 	while (*v == '#') {
 		v = skip_comment(v);
 		v = skip_spaces(v);
diff --git a/tools/bootconfig/test-bootconfig.sh b/tools/bootconfig/test-bootconfig.sh
index 7594659af1e1..0873883182b0 100755
--- a/tools/bootconfig/test-bootconfig.sh
+++ b/tools/bootconfig/test-bootconfig.sh
@@ -171,6 +171,24 @@ $BOOTCONF $INITRD > $OUTFILE
 xfail grep -q 'val[[:space:]]' $OUTFILE
 xpass grep -q 'val2[[:space:]]' $OUTFILE
 
+echo "Empty value with ="
+cat > $TEMPCONF << EOF
+foo =
+bar
+EOF
+$BOOTCONF $TEMPCONF > $OUTFILE
+xfail grep 'foo[[:space:]]=[[:space:]]"bar"' $OUTFILE
+xpass grep 'foo[[:space:]]=[[:space:]]"";' $OUTFILE
+
+echo "Continue value after comment"
+cat > $TEMPCONF << EOF
+foo = # comment
+bar
+EOF
+$BOOTCONF $TEMPCONF > $OUTFILE
+xpass grep 'foo[[:space:]]=[[:space:]]"bar"' $OUTFILE
+xfail grep 'foo[[:space:]]=[[:space:]]"";' $OUTFILE
+
 echo "=== expected failure cases ==="
 for i in samples/bad-* ; do
   xfail $BOOTCONF -a $i $INITRD
Re: [PATCH] bootconfig: Fix to terminate value search if it hits a newline
Posted by Steven Rostedt 1 week ago
On Fri, 30 Jan 2026 13:41:45 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> Note that if the line has a comment, it still keep searching the
> value to the next line::
> 
>   foo = # comment
>   value

Hmm, so the above is the same as: foo = "value";

But what is:

    foo = value # comment
    bar

 ?

Would that be foo = value bar;

Or simply an error. Or would both be an error?

OK, this isn't related to the patch, but as comment processing was brought
to my attention, I'm wondering how it works for the above.

> 
> This is interpreted as `foo = "value"`.
> This also updates the test script so that it can check the above cases.
> 
> Fixes: 76db5a27a827 ("bootconfig: Add Extra Boot Config support")
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
>  tools/bootconfig/test-bootconfig.sh |   18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)

As for the patch itself:

Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>

-- Steve
Re: [PATCH] bootconfig: Fix to terminate value search if it hits a newline
Posted by Julius Werner 1 week ago
> > Note that if the line has a comment, it still keep searching the
> > value to the next line::
> >
> >   foo = # comment
> >   value

This seems inconsistent to me tbh. I think someone could reasonably
write a bootconfig like this:

foo =                  # We leave foo empty here because <reason>
bar = value

and expect it to work. There's nothing in the documentation that
suggests empty values would behave differently if there is a comment
behind them.

Also, I believe your patch might inadvertently break this case (which
I think(?) is supposed to work):

foo = first array value,
      second array value,
      third array value

Because __xbc_parse_value() returns from the first value with `&next`
pointing at the newline behind the comma, and that is passed to the
next iteration of __xbc_parse_value() which will now no longer skip
it. I think an already started array may need to be treated as a
special case here and you may want to add another
skip_spaces()/skip_comment() to the loop in xbc_parse_array().
Re: [PATCH] bootconfig: Fix to terminate value search if it hits a newline
Posted by Masami Hiramatsu (Google) 1 week ago
On Fri, 30 Jan 2026 15:26:46 -0800
Julius Werner <jwerner@google.com> wrote:

> > > Note that if the line has a comment, it still keep searching the
> > > value to the next line::
> > >
> > >   foo = # comment
> > >   value
> 
> This seems inconsistent to me tbh. I think someone could reasonably
> write a bootconfig like this:
> 
> foo =                  # We leave foo empty here because <reason>
> bar = value
> 
> and expect it to work. There's nothing in the documentation that
> suggests empty values would behave differently if there is a comment
> behind them.

But we already accept that example in the tree.

  tools/bootconfig/samples/good-array-space-comment.bconf

So it is hard to change this syntax. But without comment, the
document defines the value is terminated with a newline. Thus
it needs to be fixed.


> 
> Also, I believe your patch might inadvertently break this case (which
> I think(?) is supposed to work):
> 
> foo = first array value,
>       second array value,
>       third array value

Oops, good catch! I should add this as a good sample.

> 
> Because __xbc_parse_value() returns from the first value with `&next`
> pointing at the newline behind the comma, and that is passed to the
> next iteration of __xbc_parse_value() which will now no longer skip
> it. I think an already started array may need to be treated as a
> special case here and you may want to add another
> skip_spaces()/skip_comment() to the loop in xbc_parse_array().

OK, let me fix it.

Thanks,

-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>
Re: [PATCH] bootconfig: Fix to terminate value search if it hits a newline
Posted by Julius Werner 4 days, 15 hours ago
> > This seems inconsistent to me tbh. I think someone could reasonably
> > write a bootconfig like this:
> >
> > foo =                  # We leave foo empty here because <reason>
> > bar = value
> >
> > and expect it to work. There's nothing in the documentation that
> > suggests empty values would behave differently if there is a comment
> > behind them.
>
> But we already accept that example in the tree.
>
>   tools/bootconfig/samples/good-array-space-comment.bconf
>
> So it is hard to change this syntax. But without comment, the
> document defines the value is terminated with a newline. Thus
> it needs to be fixed.

Hmm... okay. I think it would be good to add an example like this
(without array) to admin-guide/bootconfig.rst. It isn't very clear to
me that the comment "negates" the following newline in the way it is
written right now.