Re: variable hooks & global variables

[Robert Millan <rmh@aybabtu.com>]

[-- Attachment #1: Type: text/plain, Size: 1780 bytes --]


I propose this patch to preserve hooks only when variables are already
marked as global.

Additionally, it exports "root" variable.

On Thu, Jan 03, 2008 at 04:05:58PM +0100, Robert Millan wrote:
> On Thu, Jan 03, 2008 at 04:03:11PM +0100, Robert Millan wrote:
> > 
> > When you set a variable hook (grub_register_variable_hook), this hook isn't
> > preserved after someone (e.g. configfile command) opens a new context
> > (grub_env_context_open), unless the variable has been set as global
> > (grub_env_export).
> > 
> > Is this what we want?
> > 
> > The only current user of variable hooks is "root" variable, and that hook
> > contains a sanity check that seems to be more suitable for global scope.
> > 
> > The color-related variables for which I wanted to add hooks would also
> > like to keep their hooks across contexts.
> > 
> > One option is to export these variables, or to modify grub_env_context_open()
> > to preserve hooks as well as exported variables.  I'm more inclined for the
> > latter.
> > 
> > Comments?
> 
> Erm, ignore the part about global variables.  Exporting them doesn't help:
> 
>       for (var = context->prev->vars[i]; var; var = var->next)
>         {
>           if (var->type == GRUB_ENV_VAR_GLOBAL)
>             if (grub_env_set (var->name, var->value) != GRUB_ERR_NONE)
>               {
>                 grub_env_context_close ();
>                 return grub_errno;
>               }
>         }
> 
> So, we just preserve hooks ?
> 
> -- 
> Robert Millan
> 
> <GPLv2> I know my rights; I want my phone call!
> <DRM> What use is a phone call, if you are unable to speak?
> (as seen on /.)

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)

[-- Attachment #2: context.diff --]
[-- Type: text/x-diff, Size: 2061 bytes --]


	* kern/env.c (grub_env_context_open): Propagate hooks for global
	variables to new context.

	* kern/main.c (grub_set_root_dev): Export `root' variable.

diff -x '*~' -x '*.mk' -Nurp grub2/kern/env.c grub2.color/kern/env.c
--- grub2/kern/env.c	2007-12-30 09:52:04.000000000 +0100
+++ grub2.color/kern/env.c	2008-01-03 16:13:20.000000000 +0100
@@ -1,7 +1,7 @@
 /* env.c - Environment variables */
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2003,2005,2006,2007  Free Software Foundation, Inc.
+ *  Copyright (C) 2003,2005,2006,2007,2008  Free Software Foundation, Inc.
  *
  *  GRUB is free software: you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -96,11 +96,14 @@ grub_env_context_open (void)
       for (var = context->prev->vars[i]; var; var = var->next)
 	{
 	  if (var->type == GRUB_ENV_VAR_GLOBAL)
-	    if (grub_env_set (var->name, var->value) != GRUB_ERR_NONE)
-	      {
-		grub_env_context_close ();
-		return grub_errno;
-	      }
+	    {
+	      if (grub_env_set (var->name, var->value) != GRUB_ERR_NONE)
+		{
+		  grub_env_context_close ();
+		  return grub_errno;
+		}
+	      grub_register_variable_hook (var->name, var->read_hook, var->write_hook);
+	    }
 	}
     }
   
diff -x '*~' -x '*.mk' -Nurp grub2/kern/main.c grub2.color/kern/main.c
--- grub2/kern/main.c	2007-07-22 01:32:26.000000000 +0200
+++ grub2.color/kern/main.c	2008-01-03 16:29:19.000000000 +0100
@@ -1,7 +1,7 @@
 /* main.c - the kernel main routine */
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2002,2003,2005  Free Software Foundation, Inc.
+ *  Copyright (C) 2002,2003,2005,2006,2008  Free Software Foundation, Inc.
  *
  *  GRUB is free software: you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -78,6 +78,7 @@ grub_set_root_dev (void)
   const char *prefix;
 
   grub_register_variable_hook ("root", 0, grub_env_write_root);
+  grub_env_export ("root");
   
   prefix = grub_env_get ("prefix");