Re: [PATCH] Use submenus for 10_linux

[Colin Watson]

On Tue, Nov 30, 2010 at 01:36:15PM +0000, Žika wrote:
> I like the idea. But how are we to set default kernel, other than
> first, to boot automatically, now...? Old-school method doesn't
> work...

The following patch should fix handling of default='title' for entries
within submenus.  I haven't bothered defining a way to select submenu
entries by number.

Review welcome!  I'd particularly like confirmation from Vladimir that I'm
using the new extractor interface correctly, as I don't see any other
significant use of it in trunk at the moment beyond the extract_* commands
which nothing seems to be using yet.

2010-12-05  Colin Watson  <address@hidden>

        Fix defaults within submenus.

        * grub-core/normal/main.c (new_menu): New function.  Handles
        propagating `default', `fallback', and `timeout' variables to
        submenu contexts.
        (grub_normal_new_menu_context): Likewise.
        (grub_normal_new_menu_extractor): Likewise.
        * include/grub/normal.h (grub_normal_new_menu_context): Add
        prototype.
        (grub_normal_new_menu_extractor): Likewise.
        * grub-core/normal/menu.c (grub_menu_execute_entry): Use
        grub_normal_new_menu_context.  Unset `default', `fallback', and
        `timeout' if we fall off the end of the menu, to allow escape from a
        failed boot.
        (get_entry_number): Recursively scan submenus.
        (run_menu): Automatically traverse into a submenu if it contains the
        default entry.
        (show_menu): Initialise auto_boot to 0.
        * grub-core/script/execute.c (grub_script_execute_cmdline): Silence
        error messages for commands not valid when extracting menu entries
        (otherwise, if nothing else, we always get errors about setparams).

=== modified file 'grub-core/normal/main.c'
--- grub-core/normal/main.c     2010-09-20 22:47:49 +0000
+++ grub-core/normal/main.c     2010-12-05 00:36:28 +0000
@@ -123,6 +123,62 @@ grub_file_getline (grub_file_t file)
   return cmdline;
 }
 
+/* Open a new nested menu, propagating environment variables to the new
+   context as necessary.  */
+static grub_menu_t
+new_menu (int extractor)
+{
+  const char *default_val, *fallback_val, *timeout_val;
+  grub_menu_t menu;
+
+  default_val = grub_env_get ("default");
+  fallback_val = grub_env_get ("fallback");
+  timeout_val = grub_env_get ("timeout");
+
+  if (extractor)
+    grub_env_extractor_open (0);
+  else
+    grub_env_context_open ();
+
+  menu = grub_zalloc (sizeof (*menu));
+  if (!menu)
+    {
+      if (extractor)
+       grub_env_extractor_close (0);
+      else
+       grub_env_context_close ();
+      return NULL;
+    }
+  grub_env_set_menu (menu);
+
+  if (default_val)
+    {
+      /* Only propagate string defaults.  */
+      (void) grub_strtoul (default_val, 0, 0);
+      if (grub_errno == GRUB_ERR_BAD_NUMBER)
+       grub_env_set ("default", default_val);
+      grub_errno = GRUB_ERR_NONE;
+    }
+  if (fallback_val)
+    grub_env_set ("fallback", fallback_val);
+  if (timeout_val)
+    grub_env_set ("timeout", timeout_val);
+
+  return menu;
+}
+
+grub_menu_t
+grub_normal_new_menu_context (void)
+{
+  return new_menu (0);
+}
+
+grub_menu_t
+grub_normal_new_menu_extractor (void)
+{
+  return new_menu (1);
+}
+
 void
 grub_normal_free_menu (grub_menu_t menu)
 {

=== modified file 'grub-core/normal/menu.c'
--- grub-core/normal/menu.c     2010-09-20 22:47:49 +0000
+++ grub-core/normal/menu.c     2010-12-05 00:43:40 +0000
@@ -175,11 +175,9 @@ grub_menu_execute_entry(grub_menu_entry_
 
   if (entry->submenu)
     {
-      grub_env_context_open ();
-      menu = grub_zalloc (sizeof (*menu));
+      menu = grub_normal_new_menu_context ();
       if (! menu)
        return;
-      grub_env_set_menu (menu);
     }
 
   grub_env_set ("chosen", entry->title);
@@ -201,6 +199,11 @@ grub_menu_execute_entry(grub_menu_entry_
        }
       grub_env_context_close ();
     }
+
+  /* If we reach here, previous first-time defaults no longer make sense.  */
+  grub_env_unset ("default");
+  grub_env_unset ("fallback");
+  grub_env_unset ("timeout");
 }
 
 /* Execute ENTRY from the menu MENU, falling back to entries specified
@@ -311,7 +314,7 @@ grub_menu_register_viewer (struct grub_m
 
 /* Get the entry number from the variable NAME.  */
 static int
-get_entry_number (grub_menu_t menu, const char *name)
+get_entry_number (grub_menu_t menu, const char *name, int *in_submenu)
 {
   char *val;
   int entry;
@@ -323,6 +326,7 @@ get_entry_number (grub_menu_t menu, cons
   grub_error_push ();
 
   entry = (int) grub_strtoul (val, 0, 0);
+  *in_submenu = 0;
 
   if (grub_errno == GRUB_ERR_BAD_NUMBER)
     {
@@ -330,20 +334,43 @@ get_entry_number (grub_menu_t menu, cons
       grub_menu_entry_t e = menu->entry_list;
       int i;
 
+      entry = -1;
       grub_errno = GRUB_ERR_NONE;
 
-      for (i = 0; e; i++)
+      for (i = 0; e && entry == -1; i++)
        {
          if (grub_strcmp (e->title, val) == 0)
+           entry = i;
+         else if (e->submenu)
            {
-             entry = i;
-             break;
+             /* To make this work with submenus, we need to extract entries
+                from the submenu, then return the entry number of the
+                submenu itself if one of its entries (recursively) matches;
+                when the submenu is executed for real, it will then have to
+                call this function again one level down.  This is not going
+                to be very efficient with a deep menu stack, but it seems
+                unlikely that many people will bother with more than a
+                couple of levels.  */
+             grub_menu_t submenu;
+
+             submenu = grub_normal_new_menu_extractor ();
+             if (submenu)
+               {
+                 grub_script_execute_sourcecode (e->sourcecode,
+                                                 e->argc, e->args);
+                 if (get_entry_number (submenu, name, in_submenu) != -1)
+                   {
+                     entry = i;
+                     *in_submenu = 1;
+                   }
+
+                 grub_normal_free_menu (submenu);
+                 grub_env_extractor_close (0);
+               }
            }
+
          e = e->next;
        }
-
-      if (! e)
-       entry = -1;
     }
 
   if (grub_errno != GRUB_ERR_NONE)
@@ -369,16 +396,21 @@ static int
 run_menu (grub_menu_t menu, int nested, int *auto_boot)
 {
   grub_uint64_t saved_time;
-  int default_entry, current_entry;
+  int default_entry = -1, current_entry;
+  int in_submenu = 0;
   int timeout;
 
-  default_entry = get_entry_number (menu, "default");
+  default_entry = get_entry_number (menu, "default", &in_submenu);
 
   /* If DEFAULT_ENTRY is not within the menu entries, fall back to
      the first entry.  */
   if (default_entry < 0 || default_entry >= menu->size)
     default_entry = 0;
 
+  /* If a submenu was automatically selected, traverse into it.  */
+  if (in_submenu)
+    return default_entry;
+
   /* If timeout is 0, drawing is pointless (and ugly).  */
   if (grub_menu_get_timeout () == 0)
     {
@@ -596,7 +628,7 @@ show_menu (grub_menu_t menu, int nested)
     {
       int boot_entry;
       grub_menu_entry_t e;
-      int auto_boot;
+      int auto_boot = 0;
 
       boot_entry = run_menu (menu, nested, &auto_boot);
       if (boot_entry < 0)

=== modified file 'grub-core/script/execute.c'
--- grub-core/script/execute.c  2010-11-07 10:43:14 +0000
+++ grub-core/script/execute.c  2010-12-04 23:58:02 +0000
@@ -628,7 +628,7 @@ grub_script_execute_cmdline (struct grub
 
   if (invert)
     {
-      if (ret == GRUB_ERR_TEST_FAILURE)
+      if (ret == GRUB_ERR_TEST_FAILURE || ret == GRUB_ERR_EXTRACTOR)
        grub_errno = ret = GRUB_ERR_NONE;
       else if (ret == GRUB_ERR_NONE)
        ret = grub_error (GRUB_ERR_TEST_FAILURE, "false");
@@ -642,7 +642,7 @@ grub_script_execute_cmdline (struct grub
   /* Free arguments.  */
   grub_script_argv_free (&argv);
 
-  if (grub_errno == GRUB_ERR_TEST_FAILURE)
+  if (grub_errno == GRUB_ERR_TEST_FAILURE || grub_errno == GRUB_ERR_EXTRACTOR)
     grub_errno = GRUB_ERR_NONE;
 
   grub_print_error ();

=== modified file 'include/grub/normal.h'
--- include/grub/normal.h       2010-09-20 22:47:49 +0000
+++ include/grub/normal.h       2010-12-05 00:36:26 +0000
@@ -125,6 +125,8 @@ grub_normal_add_menu_entry (int argc, co
 grub_err_t
 grub_normal_set_password (const char *user, const char *password);
 
+grub_menu_t grub_normal_new_menu_context (void);
+grub_menu_t grub_normal_new_menu_extractor (void);
 void grub_normal_free_menu (grub_menu_t menu);
 
 void grub_normal_auth_init (void);

-- 
Colin Watson                                       address@hidden