<div dir="ltr">Hi folks,<div><br></div><div>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.</div><div><br></div><div>Thanks,</div><div>- Alan</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jan 12, 2016 at 3:43 PM, Alan Dunn <span dir="ltr"><<a href="mailto:amdunn@google.com" target="_blank">amdunn@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Currently, "save_env -f" in the GRUB2 parser only works with three<br>
arguments, which means only commands of the form "save_env -f <path>"<br>
that save *no* environment variables are allowed.<br>
<br>
Allow "save_env -f <path> [<var>]*", making "save_env -f" useful.<br>
<br>
Tested:<br>
Unit test test-grub2-save-env-dash-f tests this change, and the<br>
remaining unit tests still pass.<br>
<br>
Signed-off-by: Alan Dunn <<a href="mailto:amdunn@google.com">amdunn@google.com</a>><br>
---<br>
discover/grub2/env.c | 17 ++++++++++++---<br>
test/parser/Makefile.am | 1 +<br>
test/parser/test-grub2-save-env-dash-f.c | 37 ++++++++++++++++++++++++++++++++<br>
3 files changed, 52 insertions(+), 3 deletions(-)<br>
create mode 100644 test/parser/test-grub2-save-env-dash-f.c<br>
<br>
diff --git a/discover/grub2/env.c b/discover/grub2/env.c<br>
index 9de5e9f..a9ef2b9 100644<br>
--- a/discover/grub2/env.c<br>
+++ b/discover/grub2/env.c<br>
@@ -214,6 +214,7 @@ int builtin_save_env(struct grub2_script *script,<br>
int i, rc, len, siglen;<br>
char *buf, *envpath;<br>
const char *envfile;<br>
+ bool using_dash_f = false;<br>
<br>
/* we only support local filesystems */<br>
if (!dev->mounted) {<br>
@@ -222,9 +223,15 @@ int builtin_save_env(struct grub2_script *script,<br>
return -1;<br>
}<br>
<br>
- if (argc == 3 && !strcmp(argv[1], "-f"))<br>
+ if (argc >= 2 && !strcmp(argv[1], "-f")) {<br>
+ if (argc < 3) {<br>
+ pb_log("save_env: for -f, need argument\n");<br>
+ return -1;<br>
+ }<br>
+<br>
envfile = argv[2];<br>
- else<br>
+ using_dash_f = true;<br>
+ } else<br>
envfile = default_envfile;<br>
<br>
envpath = talloc_asprintf(script, "%s/%s",<br>
@@ -241,7 +248,11 @@ int builtin_save_env(struct grub2_script *script,<br>
if (rc || len < siglen || memcmp(buf, signature, siglen))<br>
goto err;<br>
<br>
- for (i = 1; i < argc; i++) {<br>
+ /* For "-f", skip the "-f <file>" arguments in picking the<br>
+ * variables to save. */<br>
+ i = (using_dash_f ? 3 : 1);<br>
+<br>
+ for (; i < argc; i++) {<br>
const char *name, *value;<br>
<br>
name = argv[i];<br>
diff --git a/test/parser/Makefile.am b/test/parser/Makefile.am<br>
index d69ca7f..dddb472 100644<br>
--- a/test/parser/Makefile.am<br>
+++ b/test/parser/Makefile.am<br>
@@ -28,6 +28,7 @@ parser_TESTS = \<br>
test/parser/test-grub2-single-line-if \<br>
test/parser/test-grub2-load-env \<br>
test/parser/test-grub2-save-env \<br>
+ test/parser/test-grub2-save-env-dash-f \<br>
test/parser/test-grub2-saved-default \<br>
test/parser/test-grub2-nondefault-prefix \<br>
test/parser/test-grub2-f18-ppc64 \<br>
diff --git a/test/parser/test-grub2-save-env-dash-f.c b/test/parser/test-grub2-save-env-dash-f.c<br>
new file mode 100644<br>
index 0000000..7d48f67<br>
--- /dev/null<br>
+++ b/test/parser/test-grub2-save-env-dash-f.c<br>
@@ -0,0 +1,37 @@<br>
+<br>
+#include <string.h><br>
+<br>
+#include <talloc/talloc.h><br>
+<br>
+#include "parser-test.h"<br>
+<br>
+#if 0 /* PARSER_EMBEDDED_CONFIG */<br>
+hello=world<br>
+save_env -f env_file hello<br>
+#endif<br>
+<br>
+static const char *envsig = "# GRUB Environment Block\n";<br>
+<br>
+void run_test(struct parser_test *test)<br>
+{<br>
+ const char *env_before, *env_after;<br>
+<br>
+ /* The environment file must be preallocated */<br>
+<br>
+ /* The padding at the end of the environment block is the length of<br>
+ * "hello=world\n" */<br>
+ env_before = talloc_asprintf(test, "%s%s", envsig,<br>
+ "############");<br>
+ test_add_file_data(test, test->ctx->device, "/boot/grub/env_file",<br>
+ env_before, strlen(env_before));<br>
+<br>
+ env_after = talloc_asprintf(test, "%s%s", envsig,<br>
+ "hello=world\n");<br>
+<br>
+ test_read_conf_embedded(test, "/boot/grub/grub.cfg");<br>
+<br>
+ test_run_parser(test, "grub2");<br>
+<br>
+ check_file_contents(test, test->ctx->device, "/boot/grub/env_file",<br>
+ env_after, strlen(env_after));<br>
+}<br>
<span class="HOEnZb"><font color="#888888">--<br>
2.6.0.rc2.230.g3dd15c0<br>
<br>
</font></span></blockquote></div><br></div></div>