Re: variable hooks global variables
On Sat, Jan 05, 2008 at 02:22:17AM +0100, Yoshinori K. Okuji wrote:
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.
Fixed then.
Local ones should not.
If you want to have a variable to be inherited, you must make it global.
Ok.
--
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 /.)
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: variable hooks global variables
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
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: variable hooks global variables
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 /.)
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: variable hooks global variables
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 /.)
* 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.0 +0100
+++ grub2.color/kern/env.c 2008-01-03 16:13:20.0 +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.0 +0200
+++ grub2.color/kern/main.c 2008-01-03 16:29:19.0 +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);
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
