Re: [PATCH] Use submenus for 10_linux

[Colin Watson <cjwatson@ubuntu.com>]

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  <cjwatson@ubuntu.com>

	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                                       [cjwatson@ubuntu.com]