Re: [PATCH] Grub2 cannot find LVM volume groups with a dash (-) in the name

[Felix Zielcke <fzielcke@z-51.de>]

[-- Attachment #1: Type: text/plain, Size: 948 bytes --]

Am Montag, den 11.08.2008, 18:18 +0200 schrieb Felix Zielcke:
> 
> Please comment this, please give me hints how to make it better.
> I don't like the code myself but it seems like we should handle this
> double dash problem :(
> 

Now that a bit time has passed, I think it isn't that ugly.
But I still don't like it.
I still haven't yet an idea how to make it better, I don't think it's
that great to introduce 4 variables just to remove every 2nd dash.

Unfortunately nobody commented on this new LVM part.

Idention was still not that right I think and I moved the `grub_dev =
xmalloc ()' line above the 2 `j = ..' and `l = ..'.

Marco, I know that you think this is a bit lazy, but I think mentioning
the double dash escaping and that it now uses asprintf for the RAID
cases would be a bit too verbose.

2008-08-14  Felix Zielcke  <fzielcke@z-51.de>

        * util/getroot.c: Include <config.h>.
        (grub_util_get_grub_dev): Rewritten.


[-- Attachment #2: getroot.diff --]
[-- Type: text/x-patch, Size: 2900 bytes --]

Index: util/getroot.c
===================================================================
--- util/getroot.c	(Revision 1804)
+++ util/getroot.c	(Arbeitskopie)
@@ -17,6 +17,7 @@
  *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <config.h>
 #include <sys/stat.h>
 #include <unistd.h>
 #include <string.h>
@@ -406,67 +407,56 @@ grub_util_get_grub_dev (const char *os_d
 
   switch (grub_util_get_dev_abstraction (os_dev))
     {
-    case GRUB_DEV_ABSTRACTION_LVM:
-      grub_dev = xmalloc (strlen (os_dev) - 12 + 1);
+      case GRUB_DEV_ABSTRACTION_LVM:
 
-      strcpy (grub_dev, os_dev + 12);
+ 	{
+	  unsigned char i, j, k, l;
 
-      break;
+	  grub_dev = xmalloc (strlen (os_dev) - strlen ("/dev/mapper/") + 1);
 
-    case GRUB_DEV_ABSTRACTION_RAID:
-      grub_dev = xmalloc (20);
-
-      if (os_dev[7] == '_' && os_dev[8] == 'd')
-	{
-	  const char *p;
-
-	  /* This a partitionable RAID device of the form /dev/md_dNNpMM. */
-	  int i;
-
-	  grub_dev[0] = 'm';
-	  grub_dev[1] = 'd';
-	  i = 2;
-	  
-	  p = os_dev + 9;
-	  while (*p >= '0' && *p <= '9')
+	  j = sizeof ("/dev/mapper/") -1;
+	  l = strlen (os_dev) - j + 1;
+	
+	  for (i = 0, k = 0; i < l; i++)
 	    {
-	      grub_dev[i] = *p;
-	      i++;
-	      p++;
+	      grub_dev[k] = os_dev[j + i];
+	      k++;
+	      if (os_dev[j + i] == '-' && os_dev[j + i + 1] == '-')
+		i++;
 	    }
+	}
 
-	  if (*p == '\0')
-	    grub_dev[i] = '\0';
-	  else if (*p == 'p')
-	    {
-	      p++;
-	      grub_dev[i] = ',';
-	      i++;
-
-	      while (*p >= '0' && *p <= '9')
-		{
-		  grub_dev[i] = *p;
-		  i++;
-		  p++;
-		}
+	break;
 
-	      grub_dev[i] = '\0';
-	    }
-	  else
-	    grub_util_error ("Unknown kind of RAID device `%s'", os_dev);
-	}
-      else if (os_dev[7] >= '0' && os_dev[7] <= '9')
-	{
-	  memcpy (grub_dev, os_dev + 5, 7);
-	  grub_dev[7] = '\0';
-	}
-      else
-	grub_util_error ("Unknown kind of RAID device `%s'", os_dev);
+      case GRUB_DEV_ABSTRACTION_RAID:
+
+	if (os_dev[7] == '_' && os_dev[8] == 'd')
+	  {
+	    /* This a partitionable RAID device of the form /dev/md_dNNpMM. */
+
+	    char *p;
+
+	    p = strchr (os_dev, 'p');
+	    if (p)
+	      *p = ',';
+
+	    asprintf (&grub_dev, "md%s", os_dev + sizeof ("/dev/md_d") - 1);
+	  }
+	else if (os_dev[7] >= '0' && os_dev[7] <= '9')
+	  {
+	    asprintf (&grub_dev, "md%s", os_dev + sizeof ("/dev/md") - 1);
+	  }
+	else if (os_dev[7] == '/' && os_dev[8] >= '0' && os_dev[8] <= '9')
+	  {
+	    asprintf (&grub_dev, "md%s", os_dev + sizeof ("/dev/md/") - 1);
+	  }
+	else
+	  grub_util_error ("Unknown kind of RAID device `%s'", os_dev);
 
-      break;
+	break;
 
-    default:  /* GRUB_DEV_ABSTRACTION_NONE */
-      grub_dev = grub_util_biosdisk_get_grub_dev (os_dev);
+      default:  /* GRUB_DEV_ABSTRACTION_NONE */
+	grub_dev = grub_util_biosdisk_get_grub_dev (os_dev);
     }
 
   return grub_dev;