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

[Alan Dunn]

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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/petitboot/attachments/20160204/33d7c1c4/attachment.html>