[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] grub-core/kern/corecmd: Quote variable values when displa
From: |
Glenn Washburn |
Subject: |
Re: [PATCH v2] grub-core/kern/corecmd: Quote variable values when displayed by the set command |
Date: |
Mon, 22 Aug 2022 15:48:16 -0500 |
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