[PATCH 1/1] discover/grub2: Fix behavior of save_env -f

Sam Mendoza-Jonas sam at mendozajonas.com
Fri Feb 5 11:56:17 AEDT 2016


Hi Alan,

I was actually looking through my inbox yesterday and thought to myself
"Ahh this has been sitting here for weeks!".

I've been at a conference this week (LCA), but I should be able to look
at this at the start of next week.

Cheers,
Sam

On Thu, Feb 04, 2016 at 03:26:03PM -0800, Alan Dunn wrote:
> Hi folks,
> 
> It has been a few weeks since I posted my prior patch, so I thought I would
> check in and see whether you think it might get looked at in the near
> future.  Admittedly fixing "save_env -f" was just something I decided to do
> when thinking about making the initial changes that I suggested on-list
> (fixing the paths that "load_env -f" and "save_env -f" look at), and I
> haven't been able to submit a patch for that issue yet, but I think this
> patch is useful on its own.
> 
> Thanks,
> - Alan
> 
> On Tue, Jan 12, 2016 at 3:43 PM, Alan Dunn <amdunn at google.com> wrote:
> 
> > Currently, "save_env -f" in the GRUB2 parser only works with three
> > arguments, which means only commands of the form "save_env -f <path>"
> > that save *no* environment variables are allowed.
> >
> > Allow "save_env -f <path> [<var>]*", making "save_env -f" useful.
> >
> > Tested:
> >  Unit test test-grub2-save-env-dash-f tests this change, and the
> >  remaining unit tests still pass.
> >
> > Signed-off-by: Alan Dunn <amdunn at google.com>
> > ---
> >  discover/grub2/env.c                     | 17 ++++++++++++---
> >  test/parser/Makefile.am                  |  1 +
> >  test/parser/test-grub2-save-env-dash-f.c | 37
> > ++++++++++++++++++++++++++++++++
> >  3 files changed, 52 insertions(+), 3 deletions(-)
> >  create mode 100644 test/parser/test-grub2-save-env-dash-f.c
> >
> > diff --git a/discover/grub2/env.c b/discover/grub2/env.c
> > index 9de5e9f..a9ef2b9 100644
> > --- a/discover/grub2/env.c
> > +++ b/discover/grub2/env.c
> > @@ -214,6 +214,7 @@ int builtin_save_env(struct grub2_script *script,
> >         int i, rc, len, siglen;
> >         char *buf, *envpath;
> >         const char *envfile;
> > +       bool using_dash_f = false;
> >
> >         /* we only support local filesystems */
> >         if (!dev->mounted) {
> > @@ -222,9 +223,15 @@ int builtin_save_env(struct grub2_script *script,
> >                 return -1;
> >         }
> >
> > -       if (argc == 3 && !strcmp(argv[1], "-f"))
> > +       if (argc >= 2 && !strcmp(argv[1], "-f")) {
> > +               if (argc < 3) {
> > +                       pb_log("save_env: for -f, need argument\n");
> > +                       return -1;
> > +               }
> > +
> >                 envfile = argv[2];
> > -       else
> > +               using_dash_f = true;
> > +       } else
> >                 envfile = default_envfile;
> >
> >         envpath = talloc_asprintf(script, "%s/%s",
> > @@ -241,7 +248,11 @@ int builtin_save_env(struct grub2_script *script,
> >         if (rc || len < siglen || memcmp(buf, signature, siglen))
> >                 goto err;
> >
> > -       for (i = 1; i < argc; i++) {
> > +       /* For "-f", skip the "-f <file>" arguments in picking the
> > +        * variables to save. */
> > +       i = (using_dash_f ? 3 : 1);
> > +
> > +       for (; i < argc; i++) {
> >                 const char *name, *value;
> >
> >                 name = argv[i];
> > diff --git a/test/parser/Makefile.am b/test/parser/Makefile.am
> > index d69ca7f..dddb472 100644
> > --- a/test/parser/Makefile.am
> > +++ b/test/parser/Makefile.am
> > @@ -28,6 +28,7 @@ parser_TESTS = \
> >         test/parser/test-grub2-single-line-if \
> >         test/parser/test-grub2-load-env \
> >         test/parser/test-grub2-save-env \
> > +       test/parser/test-grub2-save-env-dash-f \
> >         test/parser/test-grub2-saved-default \
> >         test/parser/test-grub2-nondefault-prefix \
> >         test/parser/test-grub2-f18-ppc64 \
> > diff --git a/test/parser/test-grub2-save-env-dash-f.c
> > b/test/parser/test-grub2-save-env-dash-f.c
> > new file mode 100644
> > index 0000000..7d48f67
> > --- /dev/null
> > +++ b/test/parser/test-grub2-save-env-dash-f.c
> > @@ -0,0 +1,37 @@
> > +
> > +#include <string.h>
> > +
> > +#include <talloc/talloc.h>
> > +
> > +#include "parser-test.h"
> > +
> > +#if 0 /* PARSER_EMBEDDED_CONFIG */
> > +hello=world
> > +save_env -f env_file hello
> > +#endif
> > +
> > +static const char *envsig = "# GRUB Environment Block\n";
> > +
> > +void run_test(struct parser_test *test)
> > +{
> > +       const char *env_before, *env_after;
> > +
> > +       /* The environment file must be preallocated */
> > +
> > +       /* The padding at the end of the environment block is the length of
> > +        * "hello=world\n" */
> > +       env_before = talloc_asprintf(test, "%s%s", envsig,
> > +                                       "############");
> > +       test_add_file_data(test, test->ctx->device, "/boot/grub/env_file",
> > +                               env_before, strlen(env_before));
> > +
> > +       env_after = talloc_asprintf(test, "%s%s", envsig,
> > +                                       "hello=world\n");
> > +
> > +       test_read_conf_embedded(test, "/boot/grub/grub.cfg");
> > +
> > +       test_run_parser(test, "grub2");
> > +
> > +       check_file_contents(test, test->ctx->device, "/boot/grub/env_file",
> > +                               env_after, strlen(env_after));
> > +}
> > --
> > 2.6.0.rc2.230.g3dd15c0
> >
> >

> _______________________________________________
> Petitboot mailing list
> Petitboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/petitboot



More information about the Petitboot mailing list