Attached is a stack of trivial patches that apply *on top of* your
zfs.diff from 2012-01-29. They each deal with one logical change and
should be very easy to review.

After your original patch and this stack have been dealt with, I'll
submit an updated patch for native ZFS on Linux support, which is much
shorter than before.

                                                                     -- 
                                                                 Richard
Eliminate stray trailing spaces.

Index: grub/util/grub-probe.c
===================================================================
--- grub.orig/util/grub-probe.c	2012-01-31 01:17:56.913537617 -0600
+++ grub/util/grub-probe.c	2012-01-31 01:18:20.531207000 -0600
@@ -269,7 +269,7 @@
   else
     printf ("%s", dname);
   free (dname);
-} 
+}
 
 static void
 probe_abstraction (grub_disk_t disk)
@@ -345,8 +345,8 @@
       grub_util_pull_device (*curdev);
       ndev++;
     }
-  
-  drives_names = xmalloc (sizeof (drives_names[0]) * (ndev + 1)); 
+
+  drives_names = xmalloc (sizeof (drives_names[0]) * (ndev + 1));
 
   for (curdev = device_names, curdrive = drives_names; *curdev; curdev++,
        curdrive++)
@@ -368,7 +368,7 @@
       dev = grub_device_open (drives_names[0]);
       if (! dev)
 	grub_util_error ("%s", _(grub_errmsg));
-      
+
       fs = grub_fs_probe (dev);
       if (! fs)
 	grub_util_error ("%s", _(grub_errmsg));
@@ -470,7 +470,7 @@
 	  grub_device_close (dev);
 	  continue;
 	}
-      
+
       if ((print == PRINT_COMPATIBILITY_HINT || print == PRINT_BIOS_HINT
 	   || print == PRINT_IEEE1275_HINT || print == PRINT_BAREMETAL_HINT
 	   || print == PRINT_EFI_HINT || print == PRINT_ARC_HINT)
Change device to devices in find_root_devices_from_libzfs

Index: grub/util/getroot.c
===================================================================
--- grub.orig/util/getroot.c	2012-01-31 00:34:31.014033000 -0600
+++ grub/util/getroot.c	2012-01-31 00:36:16.026730000 -0600
@@ -512,7 +512,7 @@
 static char **
 find_root_devices_from_libzfs (const char *dir)
 {
-  char **device = NULL;
+  char **devices = NULL;
   char *poolname;
   char *poolfs;
 
@@ -520,13 +520,13 @@
   if (! poolname)
     return NULL;
 
-  device = find_root_devices_from_poolname (poolname);
+  devices = find_root_devices_from_poolname (poolname);
 
   free (poolname);
   if (poolfs)
     free (poolfs);
 
-  return device;
+  return devices;
 }
 
 #ifdef __MINGW32__
Change strlen to sizeof

Index: grub/util/getroot.c
===================================================================
--- grub.orig/util/getroot.c	2012-01-31 00:36:16.026730000 -0600
+++ grub/util/getroot.c	2012-01-31 00:36:25.296311000 -0600
@@ -799,9 +799,9 @@
     grub_util_error (_("Storage name for `%s' not NUL-terminated"), dir);
 
   os_dev = xmalloc (2 * sizeof (os_dev[0]));
-  os_dev[0] = xmalloc (strlen ("/dev/") + data_len);
-  memcpy (os_dev[0], "/dev/", strlen ("/dev/"));
-  memcpy (os_dev[0] + strlen ("/dev/"), data, data_len);
+  os_dev[0] = xmalloc (sizeof ("/dev/") - 1 + data_len);
+  memcpy (os_dev[0], "/dev/", sizeof ("/dev/") - 1);
+  memcpy (os_dev[0] + sizeof ("/dev/") - 1, data, data_len);
   os_dev[1] = 0;
 
   if (ports && num_ports > 0)
Avoid crashing when canonicalize_file_name() fails

I could reproduce this with: grub-probe /dev/sda1

Index: grub/util/getroot.c
===================================================================
--- grub.orig/util/getroot.c	2012-01-30 00:29:30.754018000 -0600
+++ grub/util/getroot.c	2012-01-30 23:31:58.941211000 -0600
@@ -842,7 +931,12 @@
 	{
 	  char *tmp = *cur;
 	  int root, dm;
-	  *cur = canonicalize_file_name (*cur);
+	  *cur = canonicalize_file_name (tmp);
+	  if (*cur == NULL)
+	    {
+	      grub_util_error (_("failed to get canonical path of %s"), tmp);
+	      break;
+	    }
 	  free (tmp);
 	  root = (strcmp (*cur, "/dev/root") == 0);
 	  dm = (strncmp (*cur, "/dev/dm-", sizeof ("/dev/dm-") - 1) == 0);
Drop an unused variable.

Index: grub/util/getroot.c
===================================================================
--- grub.orig/util/getroot.c	2012-01-31 00:50:08.674484255 -0600
+++ grub/util/getroot.c	2012-01-31 00:50:24.200438000 -0600
@@ -293,7 +293,6 @@
 		&& !sscanf (name, "raidz%u", &dummy)
 		&& !strcmp (state, "ONLINE"))
 	      {
-		char *tmp;
 		if (ndevices >= devices_allocated)
 		  {
 		    devices_allocated = 2 * (devices_allocated + 8);
Add braces around and indent the `zpool status` parsing loop

The switch is one statement, so this doesn't change the actual effect
of the code, but it makes it more clear.  Also, if someone were to try
to add a debugging printf() after the if statement, it would work
correctly. ;)

Index: grub/util/getroot.c
===================================================================
--- grub.orig/util/getroot.c	2012-01-31 00:50:24.000000000 -0600
+++ grub/util/getroot.c	2012-01-31 00:51:15.352398000 -0600
@@ -274,36 +274,38 @@
 	
       if (sscanf (line, " %s %256s %256s %256s %256s %256s",
 		  name, state, readlen, writelen, cksum, notes) >= 5)
-	switch (st)
-	  {
-	  case 0:
-	    if (!strcmp (name, "NAME")
-		&& !strcmp (state, "STATE")
-		&& !strcmp (readlen, "READ")
-		&& !strcmp (writelen, "WRITE")
-		&& !strcmp (cksum, "CKSUM"))
-	      st++;
-	    break;
-	  case 1:
-	    if (!strcmp (name, poolname))
-	      st++;
-	    break;
-	  case 2:
-	    if (strcmp (name, "mirror") && !sscanf (name, "mirror-%u", &dummy)
-		&& !sscanf (name, "raidz%u", &dummy)
-		&& !strcmp (state, "ONLINE"))
-	      {
-		if (ndevices >= devices_allocated)
-		  {
-		    devices_allocated = 2 * (devices_allocated + 8);
-		    devices = xrealloc (devices, sizeof (devices[0])
-					* devices_allocated);
-		  }
-		devices[ndevices++] = xasprintf ("/dev/%s", name);
-	      }
-	    break;
-	  }
-	
+	{
+	  switch (st)
+	    {
+	    case 0:
+	      if (!strcmp (name, "NAME")
+		  && !strcmp (state, "STATE")
+		  && !strcmp (readlen, "READ")
+		  && !strcmp (writelen, "WRITE")
+		  && !strcmp (cksum, "CKSUM"))
+		st++;
+	      break;
+	    case 1:
+	      if (!strcmp (name, poolname))
+		st++;
+	      break;
+	    case 2:
+	      if (strcmp (name, "mirror") && !sscanf (name, "mirror-%u", &dummy)
+		  && !sscanf (name, "raidz%u", &dummy)
+		  && !strcmp (state, "ONLINE"))
+		{
+		  if (ndevices >= devices_allocated)
+		    {
+		      devices_allocated = 2 * (devices_allocated + 8);
+		      devices = xrealloc (devices, sizeof (devices[0])
+					  * devices_allocated);
+		    }
+		  devices[ndevices++] = xasprintf ("/dev/%s", name);
+	        }
+	      break;
+	    }
+	}
+
       free (line);
     }
 
Handle pool names with trailing spaces

Index: grub/util/getroot.c
===================================================================
--- grub.orig/util/getroot.c	2012-01-31 01:05:12.387005000 -0600
+++ grub/util/getroot.c	2012-01-31 01:05:13.196100000 -0600
@@ -260,7 +260,7 @@
   char cksum[257], notes[257];
   unsigned int dummy;
 
-  cmd = xasprintf ("zpool status %s", poolname);
+  cmd = xasprintf ("zpool status \"%s\"", poolname);
   fp = popen (cmd, "r");
   free (cmd);
 
@@ -286,7 +286,8 @@
 		st++;
 	      break;
 	    case 1:
-	      if (!strcmp (name, poolname))
+              /* strncmp is used because pools can technically have trailing spaces. */
+	      if (!strncmp (name, poolname, strlen (name)))
 		st++;
 	      break;
 	    case 2:
Handle all raidz types in `zpool status` output

Index: grub/util/getroot.c
===================================================================
--- grub.orig/util/getroot.c	2012-01-31 01:05:13.000000000 -0600
+++ grub/util/getroot.c	2012-01-31 01:06:53.856704000 -0600
@@ -293,6 +293,9 @@
 	    case 2:
 	      if (strcmp (name, "mirror") && !sscanf (name, "mirror-%u", &dummy)
 		  && !sscanf (name, "raidz%u", &dummy)
+		  && !sscanf (name, "raidz1-%u", &dummy)
+		  && !sscanf (name, "raidz2-%u", &dummy)
+		  && !sscanf (name, "raidz3-%u", &dummy)
 		  && !strcmp (state, "ONLINE"))
 		{
 		  if (ndevices >= devices_allocated)

Attachment: signature.asc
Description: This is a digitally signed message part

_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel

Reply via email to