Re: [PATCH] grub: add grub variable update functionality

[Prarit Bhargava]

Just re-pinging on this ...

P.

On 1/4/19 7:53 AM, Prarit Bhargava wrote:
> Please be aware I am NOT subscribed to grub-devel.
> 
> P.
> 
> ---8<---
> 
> Customers and users of the kernel are commenting that there is no way to 
> update
> a grub variable without copy and pasting the existing data.
> 
> For example,
> 
> [10:57 AM address@hidden grub-2.02]# ./grub-editenv list
> saved_entry=0
> next_entry=
> kernelopts=root=/dev/mapper/rhel_intel--wildcatpass--07-root ro 
> crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap 
> rd.lvm.lv=rhel_intel-wildcatpass-07/root 
> rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81  
> ignore_loglevel
> [10:57 AM address@hidden grub-2.02]# ./grub-editenv - set 
> kernelopts="root=/dev/mapper/rhel_intel--wildcatpass--07-root ro 
> crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap 
> rd.lvm.lv=rhel_intel-wildcatpass-07/root 
> rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81  
> ignore_loglevel newarg"
> [10:57 AM address@hidden grub-2.02]# ./grub-editenv list
> saved_entry=0
> next_entry=
> kernelopts=root=/dev/mapper/rhel_intel--wildcatpass--07-root ro 
> crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap 
> rd.lvm.lv=rhel_intel-wildcatpass-07/root 
> rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81  
> ignore_loglevel newarg
> 
> which is cumbersome.
> 
> Add functionality to add to an existing variable.  For example,
> 
> [10:58 AM address@hidden grub-2.02]# ./grub-editenv list
> saved_entry=0
> next_entry=
> kernelopts=root=/dev/mapper/rhel_intel--wildcatpass--07-root ro 
> crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap 
> rd.lvm.lv=rhel_intel-wildcatpass-07/root 
> rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81  
> ignore_loglevel
> [10:58 AM address@hidden grub-2.02]# ./grub-editenv - set kernelopts+="newarg"
> [10:59 AM address@hidden grub-2.02]# ./grub-editenv list
> saved_entry=0
> next_entry=
> kernelopts=root=/dev/mapper/rhel_intel--wildcatpass--07-root ro 
> crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap 
> rd.lvm.lv=rhel_intel-wildcatpass-07/root 
> rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81  
> ignore_loglevel newarg
> 
> Signed-off-by: Prarit Bhargava <address@hidden>
> Cc: address@hidden
> Cc: address@hidden
> Cc: address@hidden
> Cc: address@hidden
> ---
>  grub-core/lib/envblk.c    | 61 +++++++++++++++++++++++++++++++++++++++
>  include/grub/lib/envblk.h |  1 +
>  util/grub-editenv.c       | 25 +++++++++++++++-
>  3 files changed, 86 insertions(+), 1 deletion(-)
> 
> diff --git a/grub-core/lib/envblk.c b/grub-core/lib/envblk.c
> index 230e0e9d9abe..8ddbe2e8267e 100644
> --- a/grub-core/lib/envblk.c
> +++ b/grub-core/lib/envblk.c
> @@ -295,3 +295,64 @@ grub_envblk_iterate (grub_envblk_t envblk,
>        p = find_next_line (p, pend);
>      }
>  }
> +
> +int
> +grub_envblk_add (grub_envblk_t envblk, const char *name, const char *add)
> +{
> +  char *current, *new, *ostart, *pstart, *pend;
> +  int newsize, ret = 1;
> +
> +  /* get a copy of the existing entry */
> +  pstart = envblk->buf + sizeof (GRUB_ENVBLK_SIGNATURE) - 1;
> +  pstart = grub_strstr (pstart, name);
> +  if (!pstart)
> +  {
> +     ret = -1; /* existing entry not found */
> +     goto out;
> +  }
> +  pend = grub_strchr (pstart, '\n');
> +  if (!pend || pend > (envblk->buf + envblk->size))
> +  {
> +     ret = -1; /* existing entry not found */
> +     goto out;
> +  }
> +
> +  current = grub_zalloc (pend - pstart + 1);
> +  if (!current)
> +  {
> +     ret = -2; /* out of memory */
> +     goto out;
> +  }
> +  grub_strncpy (current, pstart, (int)(pend - pstart));
> +
> +  ostart = grub_strchr (current, '=');
> +  ostart++;
> +
> +  /* create a buffer for the updated entry. */
> +  newsize = grub_strlen (ostart) + grub_strlen (add) + 2;
> +  new = grub_zalloc (newsize);
> +  if (!new)
> +    {
> +      return -2; /* out of memory */
> +      goto out;
> +    }
> +
> +  grub_strcpy (new, ostart);
> +  grub_memcpy (new + grub_strlen (new), " ", 1);
> +  grub_strcpy (new + grub_strlen (new), add);
> +
> +  /* erase the current entry */
> +  grub_envblk_delete (envblk, name);
> +  /* add the updated entry */
> +  if (!grub_envblk_set (envblk, name, new))
> +    {
> +      /* restore the original entry.  This should always work */
> +      grub_envblk_set (envblk, name, ostart);
> +      ret = 0;
> +    }
> +
> +out:
> +  grub_free(new);
> +  grub_free(current);
> +  return ret;
> +}
> diff --git a/include/grub/lib/envblk.h b/include/grub/lib/envblk.h
> index c3e655921709..2a0f09e3435b 100644
> --- a/include/grub/lib/envblk.h
> +++ b/include/grub/lib/envblk.h
> @@ -37,6 +37,7 @@ void grub_envblk_delete (grub_envblk_t envblk, const char 
> *name);
>  void grub_envblk_iterate (grub_envblk_t envblk,
>                            void *hook_data,
>                            int hook (const char *name, const char *value, 
> void *hook_data));
> +int grub_envblk_add(grub_envblk_t envblk, const char *name, const char *add);
>  void grub_envblk_close (grub_envblk_t envblk);
>  
>  static inline char *
> diff --git a/util/grub-editenv.c b/util/grub-editenv.c
> index 118e89fe57fe..2df81a20b6bc 100644
> --- a/util/grub-editenv.c
> +++ b/util/grub-editenv.c
> @@ -210,15 +210,38 @@ set_variables (const char *name, int argc, char *argv[])
>    while (argc)
>      {
>        char *p;
> +      int add = 0;
>  
>        p = strchr (argv[0], '=');
>        if (! p)
>          grub_util_error (_("invalid parameter %s"), argv[0]);
>  
> +      if ( *(p - 1) == '+')
> +        {
> +          add = 1;
> +          *(p - 1) = 0;
> +        }
>        *(p++) = 0;
>  
> -      if (! grub_envblk_set (envblk, argv[0], p))
> +      if (!add && ! grub_envblk_set (envblk, argv[0], p))
>          grub_util_error ("%s", _("environment block too small"));
> +      else if (add) {
> +        int ret;
> +        ret = grub_envblk_add (envblk, argv[0], p);
> +        switch (ret) {
> +        case (0):
> +          grub_util_error ("%s", _("environment block too small"));
> +          break;
> +        case (-1):
> +          grub_util_error("%s", _("existing entry not found"));
> +          break;
> +        case (-2):
> +          grub_util_error("%s", _("out of memory error"));
> +          break;
> +        default:
> +          break;
> +        }
> +      }
>  
>        argc--;
>        argv++;
>