Boot options error handling

[Ognyan Kulev]

Marcus Brinkmann wrote:
> I think this usually points to a small typo in the GRUB boot script.

Yes, this was the reason for the error!  One Google search reveals it all.

> Mach
> is extremely picky about it, and it doesn't provide useful diagnostics on
> certain errors.  Fixing that would be nice.

Using undeclared variables in module lines automatically declares them 
as empty: gnumach/kern/boot_script.c:325.  What is the reasoning behind 
this logic?

I made a patch for hurd/libdiskfs/opts-std-startup.c.  It is attached to 
this mail.  I'm not able to compile it right now and test it.  This will 
happen a bit later.  Is this patch in the right direction?  It assumes 
that either all boot options must be present, or not one of them.  When 
port must not change, value `-' can be passed as an argument.

Regards
--
Ognyan Kulev <ogi@fmi.uni-sofia.bg>, "\"Programmer\""

--- opts-std-startup.c.~1.20.~  Wed May  1 19:02:02 2002
+++ opts-std-startup.c  Mon Feb  3 09:02:25 2003
@@ -70,6 +70,51 @@ startup_options[] =
   {0}
 };
 
+#define ALL_BOOT_OPTIONS       ((1 << 4) - 1) /* 4 options. */
+static int passed_boot_options;
+
+static void
+boot_option (int key, char *arg)
+{
+  mach_port_t *portp;
+  char *end_char;
+  long value;
+
+  if (key == OPT_BOOT_CMDLINE)
+    {
+      passed_boot_options |= 1;
+      return;
+    }
+  else if (key == OPT_HOST_PRIV_PORT)
+    {
+      passed_boot_options |= 2;
+      portp = &_hurd_host_priv;
+    }
+  else if (key == OPT_DEVICE_MASTER_PORT)
+    {
+      passed_boot_options |= 4;
+      portp = &_hurd_device_master;
+    }
+  else if (key == OPT_EXEC_SERVER_TASK)
+    {
+      passed_boot_options |= 8;
+      portp = &diskfs_exec_server_task;
+    }
+  else
+    assert (! "Invalid option key passed to boot_option");
+
+  if (arg[0] == '-' && arg[1] == '\0')
+    return;
+
+  value = strtol (arg, &end_char, 0);
+  if (*end_char != '\0' || value < 0)
+    {
+      *portp = MACH_PORT_DEAD;
+      return;
+    }
+  *portp = value;
+}
+
 static error_t
 parse_startup_opt (int opt, char *arg, struct argp_state *state)
 {
@@ -99,13 +144,24 @@ parse_startup_opt (int opt, char *arg, s
 
       /* Boot options */
     case OPT_DEVICE_MASTER_PORT:
-      _hurd_device_master = atoi (arg); break;
+      boot_option (OPT_DEVICE_MASTER_PORT, arg);
+      if (_hurd_device_master == MACH_PORT_DEAD)
+       return EINVAL;
+      break;
     case OPT_HOST_PRIV_PORT:
-      _hurd_host_priv = atoi (arg); break;
+      boot_option (OPT_HOST_PRIV_PORT, arg);
+      if (_hurd_host_priv == MACH_PORT_DEAD)
+       return EINVAL;
+      break;
     case OPT_EXEC_SERVER_TASK:
-      diskfs_exec_server_task = atoi (arg); break;
+      boot_option (OPT_EXEC_SERVER_TASK, arg);
+      if (diskfs_exec_server_task == MACH_PORT_DEAD)
+       return EINVAL;
+      break;
     case OPT_BOOT_CMDLINE:
-      diskfs_boot_command_line = arg; break;
+      boot_option (OPT_BOOT_CMDLINE, arg); /* only registers the option */
+      diskfs_boot_command_line = arg;
+      break;
     case OPT_BOOT_INIT_PROGRAM:
       diskfs_boot_init_program = arg; break;
     case OPT_BOOT_PAUSE:
@@ -151,9 +207,15 @@ parse_store_startup_opt (int opt, char *
   switch (opt)
     {
     case ARGP_KEY_INIT:
+      passed_boot_options = 0;
       /* Propagate our input to our STORE_ARGP child , which it will use to
         return what it parses.  */
       state->child_inputs[1] = state->input; break;
+    case ARGP_KEY_FINI:
+      /* All boot options or no boot options!  */
+      if (passed_boot_options != 0 && passed_boot_options != ALL_BOOT_OPTIONS)
+       return EINVAL;          /* ogi: More verbose message here?  */
+      break;
     default:
       return ARGP_ERR_UNKNOWN;
     }