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

[Alan Dunn]

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