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

[Sam Mendoza-Jonas]

Tested-by: Sam Mendoza-Jonas <sam at mendozajonas.com>

Thanks for the fix (and test!), merged as 01e104 here:
https://github.com/open-power/petitboot/commit/01e104dfb79a09bc28f8c5fb65bfe44596c25161

On Tue, Jan 12, 2016 at 03:43:02PM -0800, Alan Dunn 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