[PATCH] hurd: Support device entries with @/dev/disk: qualifier

[PATCH] hurd: Support device entries with @/dev/disk: qualifier

[Samuel Thibault]

Those are used with non-bootstrap disk drivers, for which libstore has to
open /dev/disk before calling device_open on it instead of on the device
master port.  Normally in that case all /dev/ entries also have the @/dev/disk:
qualifier, so we can just drop it.

Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

Index: grub2-2.06/grub-core/osdep/hurd/getroot.c
===================================================================
--- grub2-2.06.orig/grub-core/osdep/hurd/getroot.c
+++ grub2-2.06/grub-core/osdep/hurd/getroot.c
@@ -112,11 +112,23 @@ grub_util_find_hurd_root_device (const c
   if (strncmp (name, "device:", sizeof ("device:") - 1) == 0)
     {
       char *dev_name = name + sizeof ("device:") - 1;
-      size_t size = sizeof ("/dev/") - 1 + strlen (dev_name) + 1;
-      char *next;
-      ret = malloc (size);
-      next = stpncpy (ret, "/dev/", size);
-      stpncpy (next, dev_name, size - (next - ret));
+
+      if (dev_name[0] == '@')
+        {
+          /* non-bootstrap disk driver, the /dev/ entry is normally set up with
+             the same @.  */
+          char *next_name = strchr (dev_name, ':');
+          if (next_name)
+            dev_name = next_name + 1;
+        }
+
+      {
+        size_t size = sizeof ("/dev/") - 1 + strlen (dev_name) + 1;
+        char *next;
+        ret = malloc (size);
+        next = stpncpy (ret, "/dev/", size);
+        stpncpy (next, dev_name, size - (next - ret));
+      }
     }
   else if (!strncmp (name, "file:", sizeof ("file:") - 1))
     ret = strdup (name + sizeof ("file:") - 1);
@@ -166,12 +178,27 @@ grub_util_part_to_disk (const char *os_d
 		    || strncmp ("/dev/hd", os_dev, 7) == 0)))
     {
       *is_part = !is_fulldisk (path, os_dev);
+
+      if (path[0] == '@')
+        {
+          /* non-bootstrap disk driver, the /dev/ entry is normally set up with
+             the same @.  */
+          char *next_path = strchr (path, ':');
+          if (next_path)
+            {
+              char *n = strdup (next_path + 1);
+              free (path);
+              path = n;
+            }
+        }
+
       if (path[0] != '/')
 	{
 	  char *n = xasprintf ("/dev/%s", path);
 	  free (path);
 	  path = n;
 	}
+
       return path;
     }
   free (path);

[PATCHv2] hurd: Support device entries with @/dev/disk: qualifier

[Samuel Thibault]

Those are used with non-bootstrap disk drivers, for which libstore has to
open /dev/disk before calling device_open on it instead of on the device
master port.  Normally in that case all /dev/ entries also have the @/dev/disk:
qualifier, so we can just drop it.

Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

---
Difference with v1: better drop the @/dev/disk: qualifier right from
grub_util_hurd_get_disk_info so it benefits alls the callees and not
only grub_util_part_to_disk.

Index: grub2-2.06/grub-core/osdep/hurd/getroot.c
===================================================================
--- grub2-2.06.orig/grub-core/osdep/hurd/getroot.c
+++ grub2-2.06/grub-core/osdep/hurd/getroot.c
@@ -112,11 +112,23 @@ grub_util_find_hurd_root_device (const c
   if (strncmp (name, "device:", sizeof ("device:") - 1) == 0)
     {
       char *dev_name = name + sizeof ("device:") - 1;
-      size_t size = sizeof ("/dev/") - 1 + strlen (dev_name) + 1;
-      char *next;
-      ret = malloc (size);
-      next = stpncpy (ret, "/dev/", size);
-      stpncpy (next, dev_name, size - (next - ret));
+
+      if (dev_name[0] == '@')
+        {
+          /* non-bootstrap disk driver, the /dev/ entry is normally set up with
+             the same @.  */
+          char *next_name = strchr (dev_name, ':');
+          if (next_name)
+            dev_name = next_name + 1;
+        }
+
+      {
+        size_t size = sizeof ("/dev/") - 1 + strlen (dev_name) + 1;
+        char *next;
+        ret = malloc (size);
+        next = stpncpy (ret, "/dev/", size);
+        stpncpy (next, dev_name, size - (next - ret));
+      }
     }
   else if (!strncmp (name, "file:", sizeof ("file:") - 1))
     ret = strdup (name + sizeof ("file:") - 1);
Index: grub2-2.06/grub-core/osdep/hurd/hostdisk.c
===================================================================
--- grub2-2.06.orig/grub-core/osdep/hurd/hostdisk.c
+++ grub2-2.06/grub-core/osdep/hurd/hostdisk.c
@@ -87,6 +87,19 @@ grub_util_hurd_get_disk_info (const char
 	  *parent = xmalloc (len+1);
 	  memcpy (*parent, data, len);
 	  (*parent)[len] = '\0';
+
+	  if ((*parent)[0] == '@')
+	    {
+	      /* non-bootstrap disk driver, the /dev/ entry is normally set up with
+		 the same @.  */
+	      char *next_path = strchr (*parent, ':');
+	      if (next_path)
+		{
+		  char *n = strdup (next_path + 1);
+		  free (*parent);
+		  *parent = n;
+		}
+	    }
 	}
     }
   if (offset)

Re: [PATCHv2] hurd: Support device entries with @/dev/disk: qualifier

[Daniel Kiper]

Sorry for late reply but I am busy...

On Thu, Feb 24, 2022 at 12:34:13AM +0100, Samuel Thibault wrote:
> Those are used with non-bootstrap disk drivers, for which libstore has to
> open /dev/disk before calling device_open on it instead of on the device
> master port.  Normally in that case all /dev/ entries also have the @/dev/disk:
> qualifier, so we can just drop it.
>
> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
>
> ---
> Difference with v1: better drop the @/dev/disk: qualifier right from
> grub_util_hurd_get_disk_info so it benefits alls the callees and not
> only grub_util_part_to_disk.
>
> Index: grub2-2.06/grub-core/osdep/hurd/getroot.c
> ===================================================================
> --- grub2-2.06.orig/grub-core/osdep/hurd/getroot.c
> +++ grub2-2.06/grub-core/osdep/hurd/getroot.c
> @@ -112,11 +112,23 @@ grub_util_find_hurd_root_device (const c
>    if (strncmp (name, "device:", sizeof ("device:") - 1) == 0)
>      {
>        char *dev_name = name + sizeof ("device:") - 1;
> -      size_t size = sizeof ("/dev/") - 1 + strlen (dev_name) + 1;
> -      char *next;
> -      ret = malloc (size);
> -      next = stpncpy (ret, "/dev/", size);
> -      stpncpy (next, dev_name, size - (next - ret));
> +
> +      if (dev_name[0] == '@')
> +        {
> +          /* non-bootstrap disk driver, the /dev/ entry is normally set up with
> +             the same @.  */

Please format comments according to this [1].

> +          char *next_name = strchr (dev_name, ':');

Could you put empty line here?

> +          if (next_name)
> +            dev_name = next_name + 1;
> +        }
> +
> +      {
> +        size_t size = sizeof ("/dev/") - 1 + strlen (dev_name) + 1;
> +        char *next;

I would prefer if you leave variable definitions above and put code
without {} here.

> +        ret = malloc (size);

I think this should be xmalloc() call instead of malloc() one. Or you
should check ret != NULL before use.

> +        next = stpncpy (ret, "/dev/", size);
> +        stpncpy (next, dev_name, size - (next - ret));
> +      }
>      }
>    else if (!strncmp (name, "file:", sizeof ("file:") - 1))
>      ret = strdup (name + sizeof ("file:") - 1);
> Index: grub2-2.06/grub-core/osdep/hurd/hostdisk.c
> ===================================================================
> --- grub2-2.06.orig/grub-core/osdep/hurd/hostdisk.c
> +++ grub2-2.06/grub-core/osdep/hurd/hostdisk.c
> @@ -87,6 +87,19 @@ grub_util_hurd_get_disk_info (const char
>  	  *parent = xmalloc (len+1);
>  	  memcpy (*parent, data, len);
>  	  (*parent)[len] = '\0';
> +
> +	  if ((*parent)[0] == '@')
> +	    {
> +	      /* non-bootstrap disk driver, the /dev/ entry is normally set up with
> +		 the same @.  */

Please fix this comment too.

> +	      char *next_path = strchr (*parent, ':');

Please add empty line here.

> +	      if (next_path)
> +		{
> +		  char *n = strdup (next_path + 1);

Ditto.

> +		  free (*parent);
> +		  *parent = n;
> +		}
> +	    }
>  	}
>      }
>    if (offset)

[1] https://www.gnu.org/software/grub/manual/grub-dev/grub-dev.html#Comments

Daniel

[PATCHv3] hurd: Support device entries with @/dev/disk: qualifier

[Samuel Thibault]

Those are used with non-bootstrap disk drivers, for which libstore has to
open /dev/disk before calling device_open on it instead of on the device
master port.  Normally in that case all /dev/ entries also have the @/dev/disk:
qualifier, so we can just drop it.

Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

Message-Id: <20220223233413.wkk66pxp5p2q2wrf@begin>

---
Difference with v2: formatting, using xmalloc instead of malloc.

Difference with v1: better drop the @/dev/disk: qualifier right from
grub_util_hurd_get_disk_info so it benefits alls the callees and not
only grub_util_part_to_disk.

diff --git a/grub-core/osdep/hurd/getroot.c b/grub-core/osdep/hurd/getroot.c
index c66b206fa..5f0e366d1 100644
--- a/grub-core/osdep/hurd/getroot.c
+++ b/grub-core/osdep/hurd/getroot.c
@@ -112,9 +112,21 @@ grub_util_find_hurd_root_device (const char *path)
   if (strncmp (name, "device:", sizeof ("device:") - 1) == 0)
     {
       char *dev_name = name + sizeof ("device:") - 1;
-      size_t size = sizeof ("/dev/") - 1 + strlen (dev_name) + 1;
+      size_t size;
       char *next;
-      ret = malloc (size);
+
+      if (dev_name[0] == '@')
+        {
+          /* non-bootstrap disk driver, the /dev/ entry is normally set up with
+           * the same @. */
+          char *next_name = strchr (dev_name, ':');
+
+          if (next_name)
+            dev_name = next_name + 1;
+        }
+
+      size = sizeof ("/dev/") - 1 + strlen (dev_name) + 1;
+      ret = xmalloc (size);
       next = stpncpy (ret, "/dev/", size);
       stpncpy (next, dev_name, size - (next - ret));
     }
diff --git a/grub-core/osdep/hurd/hostdisk.c b/grub-core/osdep/hurd/hostdisk.c
index c47b5a5ea..73c442ae5 100644
--- a/grub-core/osdep/hurd/hostdisk.c
+++ b/grub-core/osdep/hurd/hostdisk.c
@@ -87,6 +87,21 @@ grub_util_hurd_get_disk_info (const char *dev, grub_uint32_t *secsize, grub_disk
 	  *parent = xmalloc (len+1);
 	  memcpy (*parent, data, len);
 	  (*parent)[len] = '\0';
+
+	  if ((*parent)[0] == '@')
+	    {
+	      /* non-bootstrap disk driver, the /dev/ entry is normally set up with
+	       * the same @. */
+	      char *next_path = strchr (*parent, ':');
+
+	      if (next_path)
+		{
+		  char *n = strdup (next_path + 1);
+
+		  free (*parent);
+		  *parent = n;
+		}
+	    }
 	}
     }
   if (offset)

Re: [PATCHv3] hurd: Support device entries with @/dev/disk: qualifier

[Daniel Kiper]

On Wed, Apr 27, 2022 at 11:00:29PM +0200, Samuel Thibault wrote:
> Those are used with non-bootstrap disk drivers, for which libstore has to
> open /dev/disk before calling device_open on it instead of on the device
> master port.  Normally in that case all /dev/ entries also have the @/dev/disk:
> qualifier, so we can just drop it.
>
> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
>
> Message-Id: <20220223233413.wkk66pxp5p2q2wrf@begin>
>
> ---
> Difference with v2: formatting, using xmalloc instead of malloc.
>
> Difference with v1: better drop the @/dev/disk: qualifier right from
> grub_util_hurd_get_disk_info so it benefits alls the callees and not
> only grub_util_part_to_disk.
>
> diff --git a/grub-core/osdep/hurd/getroot.c b/grub-core/osdep/hurd/getroot.c
> index c66b206fa..5f0e366d1 100644
> --- a/grub-core/osdep/hurd/getroot.c
> +++ b/grub-core/osdep/hurd/getroot.c
> @@ -112,9 +112,21 @@ grub_util_find_hurd_root_device (const char *path)
>    if (strncmp (name, "device:", sizeof ("device:") - 1) == 0)
>      {
>        char *dev_name = name + sizeof ("device:") - 1;
> -      size_t size = sizeof ("/dev/") - 1 + strlen (dev_name) + 1;
> +      size_t size;
>        char *next;
> -      ret = malloc (size);
> +
> +      if (dev_name[0] == '@')
> +        {
> +          /* non-bootstrap disk driver, the /dev/ entry is normally set up with
> +           * the same @. */
> +          char *next_name = strchr (dev_name, ':');
> +
> +          if (next_name)
> +            dev_name = next_name + 1;
> +        }
> +
> +      size = sizeof ("/dev/") - 1 + strlen (dev_name) + 1;
> +      ret = xmalloc (size);
>        next = stpncpy (ret, "/dev/", size);
>        stpncpy (next, dev_name, size - (next - ret));
>      }
> diff --git a/grub-core/osdep/hurd/hostdisk.c b/grub-core/osdep/hurd/hostdisk.c
> index c47b5a5ea..73c442ae5 100644
> --- a/grub-core/osdep/hurd/hostdisk.c
> +++ b/grub-core/osdep/hurd/hostdisk.c
> @@ -87,6 +87,21 @@ grub_util_hurd_get_disk_info (const char *dev, grub_uint32_t *secsize, grub_disk
>  	  *parent = xmalloc (len+1);
>  	  memcpy (*parent, data, len);
>  	  (*parent)[len] = '\0';
> +
> +	  if ((*parent)[0] == '@')
> +	    {
> +	      /* non-bootstrap disk driver, the /dev/ entry is normally set up with
> +	       * the same @. */
> +	      char *next_path = strchr (*parent, ':');
> +
> +	      if (next_path)
> +		{
> +		  char *n = strdup (next_path + 1);

I think this should be xstrdup() instead of strdup(). I can fix this for
you before push.

Otherwise Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel

> +		  free (*parent);
> +		  *parent = n;
> +		}
> +	    }
>  	}
>      }
>    if (offset)

Re: [PATCHv3] hurd: Support device entries with @/dev/disk: qualifier

[Samuel Thibault]

Daniel Kiper, le mar. 17 mai 2022 16:22:10 +0200, a ecrit:
> On Wed, Apr 27, 2022 at 11:00:29PM +0200, Samuel Thibault wrote:
> > Those are used with non-bootstrap disk drivers, for which libstore has to
> > open /dev/disk before calling device_open on it instead of on the device
> > master port.  Normally in that case all /dev/ entries also have the @/dev/disk:
> > qualifier, so we can just drop it.
> >
> > Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> >
> > Message-Id: <20220223233413.wkk66pxp5p2q2wrf@begin>
> >
> > ---
> > Difference with v2: formatting, using xmalloc instead of malloc.
> >
> > Difference with v1: better drop the @/dev/disk: qualifier right from
> > grub_util_hurd_get_disk_info so it benefits alls the callees and not
> > only grub_util_part_to_disk.
> >
> > diff --git a/grub-core/osdep/hurd/getroot.c b/grub-core/osdep/hurd/getroot.c
> > index c66b206fa..5f0e366d1 100644
> > --- a/grub-core/osdep/hurd/getroot.c
> > +++ b/grub-core/osdep/hurd/getroot.c
> > @@ -112,9 +112,21 @@ grub_util_find_hurd_root_device (const char *path)
> >    if (strncmp (name, "device:", sizeof ("device:") - 1) == 0)
> >      {
> >        char *dev_name = name + sizeof ("device:") - 1;
> > -      size_t size = sizeof ("/dev/") - 1 + strlen (dev_name) + 1;
> > +      size_t size;
> >        char *next;
> > -      ret = malloc (size);
> > +
> > +      if (dev_name[0] == '@')
> > +        {
> > +          /* non-bootstrap disk driver, the /dev/ entry is normally set up with
> > +           * the same @. */
> > +          char *next_name = strchr (dev_name, ':');
> > +
> > +          if (next_name)
> > +            dev_name = next_name + 1;
> > +        }
> > +
> > +      size = sizeof ("/dev/") - 1 + strlen (dev_name) + 1;
> > +      ret = xmalloc (size);
> >        next = stpncpy (ret, "/dev/", size);
> >        stpncpy (next, dev_name, size - (next - ret));
> >      }
> > diff --git a/grub-core/osdep/hurd/hostdisk.c b/grub-core/osdep/hurd/hostdisk.c
> > index c47b5a5ea..73c442ae5 100644
> > --- a/grub-core/osdep/hurd/hostdisk.c
> > +++ b/grub-core/osdep/hurd/hostdisk.c
> > @@ -87,6 +87,21 @@ grub_util_hurd_get_disk_info (const char *dev, grub_uint32_t *secsize, grub_disk
> >  	  *parent = xmalloc (len+1);
> >  	  memcpy (*parent, data, len);
> >  	  (*parent)[len] = '\0';
> > +
> > +	  if ((*parent)[0] == '@')
> > +	    {
> > +	      /* non-bootstrap disk driver, the /dev/ entry is normally set up with
> > +	       * the same @. */
> > +	      char *next_path = strchr (*parent, ':');
> > +
> > +	      if (next_path)
> > +		{
> > +		  char *n = strdup (next_path + 1);
> 
> I think this should be xstrdup() instead of strdup(). I can fix this for
> you before push.

Ok, please do so :)

Samuel

> Otherwise Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
> 
> Daniel
> 
> > +		  free (*parent);
> > +		  *parent = n;
> > +		}
> > +	    }
> >  	}
> >      }
> >    if (offset)
>