RE: [PATCH] env: Allow string CONFIG options in the text environment

From: Holger Brunck <holger.brunck@hitachienergy.com>
To: Rasmus Villemoes <rasmus.villemoes@prevas.dk>,
	Simon Glass <sjg@chromium.org>
Cc: "U-Boot Mailing List" <u-boot@lists.denx.de>,
	"Tom Rini" <trini@konsulko.com>,
	"Heinrich Schuchardt" <xypron.glpk@gmx.de>,
	"Joe Hershberger" <joe.hershberger@ni.com>,
	"Marek Behún" <kabel@kernel.org>, "Wolfgang Denk" <wd@denx.de>
Subject: RE: [PATCH] env: Allow string CONFIG options in the text environment
Date: Mon, 7 Nov 2022 12:54:54 +0000	[thread overview]
Message-ID: <DB6PR0601MB24854E59773B7687EAF3ED0BF73C9@DB6PR0601MB2485.eurprd06.prod.outlook.com> (raw)
In-Reply-To: <98221ad8-3db3-7f66-bf02-26f4a4bd4e2a@prevas.dk>

> > On Fri, 4 Nov 2022 at 08:20, Holger Brunck
> > <holger.brunck@hitachienergy.com> wrote:
> >>
> >> Hi Simon,
> >> I got no time to try it yet but I have a general comment.
> >>
> >>>
> >>> Sometimes it is useful to include a CONFIG option that contains a string.
> >>> This is hard to do in general, since in many cases it is useful to
> >>> have the quotes around the string so that, for example:
> >>>
> >>
> >> wouldn't it be cleaner to always convert a Kconfig option which is
> >> defined as a string to a string without the double quotes? If someone
> >> needs them he could explicitly add them with
> >>
> >> bootcmd=run "CONFIG_BOARD_CMD"
> >>
> >> Because  in my case I have some options I use them to build together
> >> the kernel command line I pass to the kernel.  Ok I could store them
> >> before in an own variable and them use them with ${variable} in the
> >> command line. But I think it would be cleaner to always convert a
> >> string defined in Kconfig in a string without the quotes. What do you think?
> >
> > Yes I would prefer that to. I'm not sure how to implement it though.
> > Any thoughts?
> 
> I agree that special-casing the RHS containing a single qouted string is a bad
> idea, it's really hard to understand and explain what the rules are.
> 
> Unfortunately, I don't think we can just create a separate version of the config.h
> header where the quotes are removed and then as Holger suggests rely on
> including the double quotes when needed, because then the C preprocessor
> would see "CONFIG_BOARD_CMD" as a string literal, inside which macro
> expansion is not done.
> 
> What we really want is to separate the two uses of the config values:
> "control" and "data". One use is on conditional lines
> 
> #if IS_ENABLED(CONFIG_FOO)
> 
> and another is the case of substituting values into RHS values.
> 
> It is really convenient to use the C preprocessor for the former. But for the
> latter, it's kind of overkill, and we could probably just as well implement a simple
> perl script (or python or whatnot) that would do what we want, including
> stripping of double quotes from string items before substitution. But adding that
> as a pre-pre-processor step (only doing substitution on lines not beginning with a
> #) would break down if anybody uses #include directives, and it's also an
> annoying extra step and extra script to maintain.
> 
> tl;dr: no, I don't have any good ideas.
> 

ok if I I understand it correctly that at the time we process the environment.txt files
the CONFIG_* options are already replaced by some generic process and therefore
at the later stage we cannot remove the double quotes as we don't know if they are
intended or due to a Kconfig string.

In this case the only thing that comes in my mind would be to have an additional
tag we can use in the environment.txt file to explicitly tell the parser to remove them.

Something like:
netdev=__plain_string(CONFIG_KM_NETEDEV)

this we could parse later on and remove the double quotes between the braces.
In this case we know we always have them per default, but users would be able
to remove them if wanted. This would at least be consistent.

Best regards
Holger