[PATCH] save_env variable_name=value

[PATCH] save_env variable_name=value

[Colin Watson]

This implements saving an environment variable with a given value
without having to set that variable first, as suggested by Pavel here:

  http://lists.gnu.org/archive/html/grub-devel/2009-06/msg00190.html

2009-09-02  Colin Watson  <cjwatson@ubuntu.com>

	* commands/loadenv.c (grub_cmd_save_env): Allow an optional
	explicit value (`save_env variable_name=value').
	(GRUB_MOD_INIT (loadenv)): Update save_env summary.

Index: commands/loadenv.c
===================================================================
--- commands/loadenv.c	(revision 2558)
+++ commands/loadenv.c	(working copy)
@@ -351,18 +351,32 @@
 
   while (argc)
     {
-      char *value;
+      char *equals;
+      char *value = NULL;
 
-      value = grub_env_get (args[0]);
+      equals = grub_strchr (args[0], '=');
+      if (equals)
+        {
+          *equals = 0;
+          value = equals + 1;
+        }
+
+      if (! value)
+        value = grub_env_get (args[0]);
       if (value)
         {
           if (! grub_envblk_set (envblk, args[0], value))
             {
               grub_error (GRUB_ERR_BAD_ARGUMENT, "environment block too small");
+              if (equals)
+                *equals = '=';
               goto fail;
             }
         }
 
+      if (equals)
+        *equals = '=';
+
       argc--;
       args++;
     }
@@ -396,7 +410,7 @@
   cmd_save =
     grub_register_extcmd ("save_env", grub_cmd_save_env,
 			  GRUB_COMMAND_FLAG_BOTH,
-			  "save_env [-f FILE] variable_name [...]",
+			  "save_env [-f FILE] variable_name[=value] [...]",
 			  "Save variables to environment block file.",
 			  options);
 }

-- 
Colin Watson                                       [cjwatson@ubuntu.com]

Re: [PATCH] save_env variable_name=value

[Robert Millan]

On Wed, Sep 02, 2009 at 12:01:42PM +0100, Colin Watson wrote:
> This implements saving an environment variable with a given value
> without having to set that variable first, as suggested by Pavel here:
> 
>   http://lists.gnu.org/archive/html/grub-devel/2009-06/msg00190.html
> 
> 2009-09-02  Colin Watson  <cjwatson@ubuntu.com>
> 
> 	* commands/loadenv.c (grub_cmd_save_env): Allow an optional
> 	explicit value (`save_env variable_name=value').
> 	(GRUB_MOD_INIT (loadenv)): Update save_env summary.

Maybe it is better to handle the extra logic at grub-mkconfig level
(for smaller/faster loadenv code).  Is this feasible?

In any case, this would have to be post-1.97.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [PATCH] save_env variable_name=value

[Colin Watson]

On Thu, Sep 03, 2009 at 04:33:23PM +0200, Robert Millan wrote:
> On Wed, Sep 02, 2009 at 12:01:42PM +0100, Colin Watson wrote:
> > 2009-09-02  Colin Watson  <cjwatson@ubuntu.com>
> > 
> > 	* commands/loadenv.c (grub_cmd_save_env): Allow an optional
> > 	explicit value (`save_env variable_name=value').
> > 	(GRUB_MOD_INIT (loadenv)): Update save_env summary.
> 
> Maybe it is better to handle the extra logic at grub-mkconfig level
> (for smaller/faster loadenv code).  Is this feasible?

It's feasible either way; I just did this because Pavel seemed to prefer
it ...

-- 
Colin Watson                                       [cjwatson@ubuntu.com]

Re: [PATCH] save_env variable_name=value

[Robert Millan]

On Thu, Sep 03, 2009 at 03:52:43PM +0100, Colin Watson wrote:
> On Thu, Sep 03, 2009 at 04:33:23PM +0200, Robert Millan wrote:
> > On Wed, Sep 02, 2009 at 12:01:42PM +0100, Colin Watson wrote:
> > > 2009-09-02  Colin Watson  <cjwatson@ubuntu.com>
> > > 
> > > 	* commands/loadenv.c (grub_cmd_save_env): Allow an optional
> > > 	explicit value (`save_env variable_name=value').
> > > 	(GRUB_MOD_INIT (loadenv)): Update save_env summary.
> > 
> > Maybe it is better to handle the extra logic at grub-mkconfig level
> > (for smaller/faster loadenv code).  Is this feasible?
> 
> It's feasible either way; I just did this because Pavel seemed to prefer
> it ...

Pavel, please comment on this when you can.  It seems to me that doing it
in grub-mkconfig would require less ad-hoc code in loadenv.mod and make it
more efficient.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [PATCH] save_env variable_name=value

[Pavel Roskin]

On Thu, 2009-09-03 at 17:08 +0200, Robert Millan wrote:

> Pavel, please comment on this when you can.  It seems to me that doing it
> in grub-mkconfig would require less ad-hoc code in loadenv.mod and make it
> more efficient.

I don't see how grub-mkconfig could compensate for a missing feature in
save_env.  Perhaps I'm missing the context here.

-- 
Regards,
Pavel Roskin

Re: [PATCH] save_env variable_name=value

[Colin Watson]

On Fri, Sep 11, 2009 at 05:43:06PM -0400, Pavel Roskin wrote:
> On Thu, 2009-09-03 at 17:08 +0200, Robert Millan wrote:
> > Pavel, please comment on this when you can.  It seems to me that doing it
> > in grub-mkconfig would require less ad-hoc code in loadenv.mod and make it
> > more efficient.
> 
> I don't see how grub-mkconfig could compensate for a missing feature in
> save_env.  Perhaps I'm missing the context here.

The choices are:

  var=value
  save_env var

(uglier grub-mkconfig code, smaller core)

  save_env var=value

(simpler grub-mkconfig, more code in core)

-- 
Colin Watson                                       [cjwatson@ubuntu.com]

Re: [PATCH] save_env variable_name=value

[Robert Millan]

On Fri, Sep 11, 2009 at 05:43:06PM -0400, Pavel Roskin wrote:
> On Thu, 2009-09-03 at 17:08 +0200, Robert Millan wrote:
> 
> > Pavel, please comment on this when you can.  It seems to me that doing it
> > in grub-mkconfig would require less ad-hoc code in loadenv.mod and make it
> > more efficient.
> 
> I don't see how grub-mkconfig could compensate for a missing feature in
> save_env.  Perhaps I'm missing the context here.

Instead of issuing:

  save_env variable=value

grub-mkconfig can issue:

  set variable=value
  save_env

which doesn't need additional logic in save_env.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [PATCH] save_env variable_name=value

[richardvoigt]

On Fri, Sep 11, 2009 at 4:43 PM, Pavel Roskin <proski@gnu.org> wrote:
> On Thu, 2009-09-03 at 17:08 +0200, Robert Millan wrote:
>
>> Pavel, please comment on this when you can.  It seems to me that doing it
>> in grub-mkconfig would require less ad-hoc code in loadenv.mod and make it
>> more efficient.
>
> I don't see how grub-mkconfig could compensate for a missing feature in
> save_env.  Perhaps I'm missing the context here.

AFAICT, it's grub.cfg that has to work around using two commands instead of one.

Therefore grub-mkconfig has to generate a longer grub.cfg (not sure
how this makes grub-mkconfig uglier), but that's an incomplete
assessment.  There's also a different burden placed on user-edited
configs and usage of the grub console, correct?

I don't think that the suggestion was meant to save a few bytes in
grub-mkconfig.  I think it was suggesting a nicer interface for users
working in the console.

>
> --
> Regards,
> Pavel Roskin
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>

Re: [PATCH] save_env variable_name=value

[Pavel Roskin]

On Sat, 2009-09-12 at 09:54 -0500, richardvoigt@gmail.com wrote:
> On Fri, Sep 11, 2009 at 4:43 PM, Pavel Roskin <proski@gnu.org> wrote:
> > I don't see how grub-mkconfig could compensate for a missing feature in
> > save_env.  Perhaps I'm missing the context here.
> 
> AFAICT, it's grub.cfg that has to work around using two commands instead of one.

Yes, if we want to keep backward compatibility with older modules.

> Therefore grub-mkconfig has to generate a longer grub.cfg (not sure
> how this makes grub-mkconfig uglier), but that's an incomplete
> assessment.  There's also a different burden placed on user-edited
> configs and usage of the grub console, correct?

Correct.

> I don't think that the suggestion was meant to save a few bytes in
> grub-mkconfig.  I think it was suggesting a nicer interface for users
> working in the console.

Yes.  Also, grub-mkconfig could use it once we decide to break backward
compatibility with the last version that didn't support save_env with a
name.

-- 
Regards,
Pavel Roskin