On Thu, Mar 26, 2026 at 11:30:42PM +0900, Masami Hiramatsu wrote:
> On Wed, 25 Mar 2026 23:22:04 +0900
> Masami Hiramatsu (Google) <[email protected]> wrote:
>
> > > + /*
> > > +  * Keys that do not match any early_param() handler are silently
> > > +  * ignored — do_early_param() always returns 0.
> > > +  */
> > > + xbc_node_for_each_key_value(root, knode, val) {
> >
> > [sashiko comment]
> > | Does this loop handle array values correctly?
> > | xbc_node_for_each_key_value() only assigns the first value of an array to
> > | the val pointer before advancing to the next key. It does not iterate over
> > | the child nodes of the array.
> > | If the bootconfig contains a multi-value key like
> > | kernel.console = "ttyS0", "tty0", will the subsequent values in the array
> > | be silently dropped instead of passed to the early_param handlers?
> >
> > Also, good catch :) we need to use xbc_node_for_each_array_value()
> > for inner loop.
>
> FYI, xbc_snprint_cmdline() translates the arraied parameter as
> multiple parameters. For example,
>
> foo = bar, buz;
>
> will be converted to
>
> foo=bar foo=buz
>
> Thus, I think we should do the same thing below;
>
> >
> > > +         if (xbc_node_compose_key_after(root, knode, xbc_namebuf, 
> > > XBC_KEYLEN_MAX) < 0)
> > > +                 continue;
> > > +
> > > +         /*
> > > +          * We need to copy const char *val to a char pointer,
> > > +          * which is what do_early_param() need, given it might
> > > +          * call strsep(), strtok() later.
> > > +          */
> > > +         ret = strscpy(val_buf, val, sizeof(val_buf));
> > > +         if (ret < 0) {
> > > +                 pr_warn("ignoring bootconfig value '%s', too long\n",
> > > +                         xbc_namebuf);
> > > +                 continue;
> > > +         }
> > > +         do_early_param(xbc_namebuf, val_buf, NULL, NULL);
>
> So instead of this;
>
> xbc_array_for_each_value(vnode, val) {
>       do_early_param(xbc_namebuf, val, NULL, NULL);
> }
>
> Maybe it is a good timing to recondier unifying kernel cmdline and bootconfig
> from API viewpoint.

I'm not familiar with the history on this topic. Has unifying the APIs been
previously considered and set aside?

Given all the feedback on this series, I see three types of issues to address:

1) Minor patch improvements
2) Architecture-specific super early parameters being parsed before bootconfig
   is available
3) Unifying kernel cmdline and bootconfig interfaces

Which of these areas would you recommend I prioritize?

Thanks for the guidance,
--breno

Reply via email to