Re: [PATCH] fix disk->id abuse

[Christian Franke <Christian.Franke@t-online.de>]

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

Marco Gerards wrote:
> [...]
>
>   
>>> We could write a macro for ID comparison that would compare both the
>>> "driver ID" (disk->dev->id) and "device ID" (disk->id).  In this case,
>>> we can omit disk->id initialization in the drivers supporting only one
>>> device (e.g. memdisk) and only leave it where it's indeed needed for
>>> identifying separate devices, thus removing potentially confusing code.
>>>       
>> Sounds fine, although what worries me most if the current usage of 'id' in
>> scsi.c, which can lead to collision already.
>>
>> I assume using LUNs is a proper solution for that one?
>>     
>
> No, it is not.  I think I already said so on IRC?
>
>   

If disk->id is supposed to be a GUID ('Grub Unique Identifier' in this 
case :-), then a pointer to the private data structure for the disk 
should work. This id is unique until disk close.

For drivers without disk->data, simply use the address of e.g. the open 
function itself.

See attached patch for an example.

Christian


[-- Attachment #2: grub2-disk-id.patch --]
[-- Type: text/x-diff, Size: 839 bytes --]

diff --git a/disk/memdisk.c b/disk/memdisk.c
index 978eae5..e814890 100644
--- a/disk/memdisk.c
+++ b/disk/memdisk.c
@@ -41,7 +41,7 @@ grub_memdisk_open (const char *name, grub_disk_t disk)
       return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "not a memdisk");
 
   disk->total_sectors = memdisk_size / GRUB_DISK_SECTOR_SIZE;
-  disk->id = (unsigned long) "mdsk";
+  disk->id = (unsigned long) grub_memdisk_open;
   disk->has_partitions = 0;
 
   return GRUB_ERR_NONE;
diff --git a/disk/scsi.c b/disk/scsi.c
index 01ef04e..8a5bd4e 100644
--- a/disk/scsi.c
+++ b/disk/scsi.c
@@ -248,7 +248,7 @@ grub_scsi_open (const char *name, grub_disk_t disk)
     {
       if (! p->open (name, scsi))
 	{
-	  disk->id = (unsigned long) "scsi"; /* XXX */
+	  disk->id = (unsigned long) scsi;
 	  disk->data = scsi;
 	  scsi->dev = p;
 	  scsi->lun = lun;