Re: [PATCH v2] grub-core/kern/corecmd: Quote variable values when displayed by the set command

[Glenn Washburn]

On Sat, 20 Aug 2022 01:07:35 +0200
Daniel Kiper <dkiper@net-space.pl> wrote:

> On Fri, Aug 19, 2022 at 05:38:24PM -0500, Glenn Washburn wrote:
> > Variable values may contain spaces at the end or newlines. However, when
> > displayed without quotes this is not obvious and can lead to confusion as
> > to the actual contents of variables. Also for some variables grub_env_get()
> > returns a NULL pointer instead of a pointer to an empty string and
> > previously would be printed as 'var=(null)'. Now such variables will be
> > displayed as 'var=""'.
> 
> Better but...
> 
> > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > ---
> >  grub-core/kern/corecmd.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/grub-core/kern/corecmd.c b/grub-core/kern/corecmd.c
> > index fc54f43f2..10b595d6b 100644
> > --- a/grub-core/kern/corecmd.c
> > +++ b/grub-core/kern/corecmd.c
> > @@ -40,7 +40,10 @@ grub_core_cmd_set (struct grub_command *cmd 
> > __attribute__ ((unused)),
> >      {
> >        struct grub_env_var *env;
> >        FOR_SORTED_ENV (env)
> > -   grub_printf ("%s=%s\n", env->name, grub_env_get (env->name));
> > +   {
> > +     val = grub_env_get (env->name);
> > +     grub_printf ("%s=\"%s\"\n", env->name, val ? val : "");
> 
> Maybe I am overzealous but what will happen if value contains "$"
> character, e.g. "$var", and somebody wants copy-pasta this value with
> "" around?  I think '' instead of "" would be better here. IIRC '' works
> in GRUB shell like it works in regular shell.

Sure, single quotes seem good.

> 
> Additionally, what if the value contains '"' characters? Should we
> escape them?

Yes, this would be ideal. I don't really want to be the one to implement
this though. The use case that I care about it seeing when variables
have whitespace at the end of the value. This is more for debugging
GRUB script that is _not_ created with the intention of making the
output of set trick the user. Also I don't have grub script in
variables (though this could be reasonable with someone using exec), so
its not an issue for me.

> 
> And a nit... I prefer s/val ?/(val != NULL) ?/.

Yep, of course.

Glenn