Skip to contentSkip to conversation

Add host open devicename check

PATCH In review Christian Franke 3 participants 10 messages Oct 25, 2007 – Nov 18, 2007

lore
Share & tools
lore

Christian Franke

author
#0 lore raw

PATCH 1/1 Add host open devicename check

grub-emu ls command does not list any partitions.

Device scan in grub_disk_open() stops early because grub_host_open() returns success on all device names.

Christian

2007-10-25 Christian Franke <[email protected]>

* disk/host.c (grub_host_open): Add check for "host". This fixes the problem that grub-emu does not find partitions.

disk/host.c +4 -1
@@ -34,8 +34,11 @@ grub_host_iterate (int (*hook) (const ch
 }
 
 static grub_err_t
-grub_host_open (const char *name __attribute((unused)), grub_disk_t disk)
+grub_host_open (const char *name, grub_disk_t disk)
 {
+  if (grub_strcmp(name, "host"))
+      return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "not a host disk");
+
   disk->total_sectors = 0;
   disk->id = (int) "host";
#1 lore raw
Quoted 11 lines · Christian Franke

grub-emu ls command does not list any partitions.

Device scan in grub_disk_open() stops early because grub_host_open() returns success on all device names.

Christian

2007-10-25 Christian Franke <[email protected]>

* disk/host.c (grub_host_open): Add check for "host". This fixes the problem that grub-emu does not find partitions.

Please mention the attribute change.

disk/host.c grub_host_iterate (int (*hook) (const ch · line 40

Quoted 11 lines
disk/host.c +3 -1
@@ -34,8 +34,11 @@ grub_host_iterate (int (*hook) (const ch
 }
 
 static grub_err_t
-grub_host_open (const char *name __attribute((unused)), grub_disk_t disk)
+grub_host_open (const char *name, grub_disk_t disk)
 {
+  if (grub_strcmp(name, "host"))
+      return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "not a host disk");

Please add a space after the function name and before the "(".

--
Marco

author
#2 lore raw
Quoted 11 lines · Marco Gerards
Quoted 7 lines

..

2007-10-25 Christian Franke <[email protected]>

* disk/host.c (grub_host_open): Add check for "host". This fixes the problem that grub-emu does not find partitions.

Please mention the attribute change.

Quoted 12 lines

Review

Quoted 8 lines

...
static grub_err_t

(quoted diff) +3 -1
-grub_host_open (const char *name __attribute((unused)), grub_disk_t disk)
+grub_host_open (const char *name, grub_disk_t disk)
 {
+  if (grub_strcmp(name, "host"))
+      return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "not a host disk");

Please add a space after the function name and before the "(".

Done

Christian

2007-11-10 Christian Franke <[email protected]>

* disk/host.c (grub_host_open): Remove attribute unused from name parameter. Add check for "host". This fixes the problem that grub-emu does not find partitions.

disk/host.c +4 -1
@@ -34,8 +34,11 @@ grub_host_iterate (int (*hook) (const ch
 }
 
 static grub_err_t
-grub_host_open (const char *name __attribute((unused)), grub_disk_t disk)
+grub_host_open (const char *name, grub_disk_t disk)
 {
+  if (grub_strcmp (name, "host"))
+      return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "not a host disk");
+
   disk->total_sectors = 0;
   disk->id = (int) "host";
#3 lore raw
Quoted 27 lines · Christian Franke
Quoted 11 lines · Marco Gerards
Quoted 7 lines

..

2007-10-25 Christian Franke <[email protected]>

* disk/host.c (grub_host_open): Add check for "host". This fixes the problem that grub-emu does not find partitions.

Please mention the attribute change.

Quoted 12 lines

disk/host.c grub_host_iterate (int (*hook) (const ch · line 40

Quoted 8 lines

...
static grub_err_t

disk/host.c +3 -1
-grub_host_open (const char *name __attribute((unused)), grub_disk_t disk)
+grub_host_open (const char *name, grub_disk_t disk)
 {
+  if (grub_strcmp(name, "host"))
+      return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "not a host disk");

Please add a space after the function name and before the "(".

Done

Quoted 7 lines

Christian

2007-11-10 Christian Franke <[email protected]>

* disk/host.c (grub_host_open): Remove attribute unused from name parameter. Add check for "host". This fixes the problem that grub-emu does not find partitions.

Looks fine to me.

--
Marco

#4 lore raw
Quoted 8 lines · Marco Gerards
Quoted 6 lines

2007-11-10 Christian Franke <[email protected]>

* disk/host.c (grub_host_open): Remove attribute unused from name parameter. Add check for "host". This fixes the problem that grub-emu does not find partitions.

Looks fine to me.

Committed.

Signature · 5 lines
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)
#5 lore raw
In reply to Christian Franke

disk/host.c grub_host_iterate (int (*hook) (const ch · line 41

Quoted 7 lines · Christian Franke

static grub_err_t

disk/host.c +4 -1
-grub_host_open (const char *name __attribute((unused)), grub_disk_t disk)
+grub_host_open (const char *name, grub_disk_t disk)
 {
+  if (grub_strcmp(name, "host"))
+      return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "not a host disk");
+

I would test for (disk->id == GRUB_DISK_DEVICE_HOST_ID) instead. Faster, and also cleaner/simpler IMHO.

Signature · 5 lines
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)
#6 lore raw
Quoted 11 lines · Robert Millan

disk/host.c grub_host_iterate (int (*hook) (const ch · line 41

Quoted 7 lines · Christian Franke

static grub_err_t

disk/host.c +4 -1
-grub_host_open (const char *name __attribute((unused)), grub_disk_t disk)
+grub_host_open (const char *name, grub_disk_t disk)
 {
+  if (grub_strcmp(name, "host"))
+      return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "not a host disk");
+

I would test for (disk->id == GRUB_DISK_DEVICE_HOST_ID) instead. Faster, and also cleaner/simpler IMHO.

It's not possible unfortunately :-(. This information is about to be filled in in this same function. Besides that, you have grub_disk_dev in mind.

--
Marco

#7 lore raw
Quoted 14 lines · Marco Gerards
Quoted 11 lines · Robert Millan

disk/host.c grub_host_iterate (int (*hook) (const ch · line 41

Quoted 7 lines · Christian Franke

static grub_err_t

disk/host.c +4 -1
-grub_host_open (const char *name __attribute((unused)), grub_disk_t disk)
+grub_host_open (const char *name, grub_disk_t disk)
 {
+  if (grub_strcmp(name, "host"))
+      return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "not a host disk");
+

I would test for (disk->id == GRUB_DISK_DEVICE_HOST_ID) instead. Faster, and also cleaner/simpler IMHO.

Quoted 2 lines

It's not possible unfortunately :-(. This information is about to be filled in in this same function.

Still seems like an ugly hack to me. Oh well :-/

Quoted 2 lines

Besides that, you have grub_disk_dev
in mind.

Ouch ;-)

Signature · 5 lines
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)
author
#8 lore raw
Quoted 18 lines · Robert Millan
Quoted 16 lines · Marco Gerards
Quoted 13 lines · Robert Millan

disk/host.c grub_host_iterate (int (*hook) (const ch · line 41

Quoted 8 lines · Christian Franke

static grub_err_t

disk/host.c +4 -1
-grub_host_open (const char *name __attribute((unused)), grub_disk_t disk)
+grub_host_open (const char *name, grub_disk_t disk)
 {
+  if (grub_strcmp(name, "host"))
+      return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "not a host disk");
+

I would test for (disk->id == GRUB_DISK_DEVICE_HOST_ID) instead. Faster, and also cleaner/simpler IMHO.

Quoted 7 lines
Quoted 3 lines

It's not possible unfortunately :-(. This information is about to be filled in in this same function.

Still seems like an ugly hack to me. Oh well :-/

All disk/* modules' open routines check whether the name (hd%d, ata%d, ...) is valid and return UNKNOWN_DEVICE on error. The missing name check in host.c is a bug which can IMO only be fixed this way.

Christian

#9 lore raw
In reply to Robert Millan
Quoted 15 lines · Robert Millan
Quoted 14 lines · Marco Gerards
Quoted 11 lines · Robert Millan

disk/host.c grub_host_iterate (int (*hook) (const ch · line 41

Quoted 7 lines · Christian Franke

static grub_err_t

disk/host.c +4 -1
-grub_host_open (const char *name __attribute((unused)), grub_disk_t disk)
+grub_host_open (const char *name, grub_disk_t disk)
 {
+  if (grub_strcmp(name, "host"))
+      return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "not a host disk");
+

I would test for (disk->id == GRUB_DISK_DEVICE_HOST_ID) instead. Faster, and also cleaner/simpler IMHO.

Quoted 4 lines
Quoted 2 lines

It's not possible unfortunately :-(. This information is about to be filled in in this same function.

Still seems like an ugly hack to me. Oh well :-/

It isn't. The driver gets a string that it can use to determine if it has control over this disk or not. `grub_disk_t disk' is there to be filled in if it has, it isn't initialised yet.

--
Marco