* Re: variable hooks & global variables
2008-01-03 15:05 ` Robert Millan
@ 2008-01-03 15:31 ` Robert Millan
2008-01-05 1:22 ` Yoshinori K. Okuji
1 sibling, 0 replies; 5+ messages in thread
From: Robert Millan @ 2008-01-03 15:31 UTC (permalink / raw)
To: grub-devel
[-- 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");
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: variable hooks & global variables
2008-01-03 15:05 ` Robert Millan
2008-01-03 15:31 ` Robert Millan
@ 2008-01-05 1:22 ` Yoshinori K. Okuji
2008-01-05 12:03 ` Robert Millan
1 sibling, 1 reply; 5+ messages in thread
From: Yoshinori K. Okuji @ 2008-01-05 1:22 UTC (permalink / raw)
To: The development of GRUB 2
On Thursday 03 January 2008 16:05, 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 ?
Global variables should preserve hooks. This is a bug. Local ones should not.
If you want to have a variable to be inherited, you must make it global.
Okuji
^ permalink raw reply [flat|nested] 5+ messages in thread