Re: [PATCH] fix disk->id abuse

[Christian Franke]

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

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;