Re: [PATCH] fix disk->id abuse

[Christian Franke]

Robert Millan wrote:
> On Tue, Sep 02, 2008 at 09:12:04PM +0200, Christian Franke wrote:
>   
> > 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.
> >     
>
> This is fine for single-disk drivers, but for multi-disk ones we need it to
> be unique among different disks provided by the same driver.
>
>   

I apparently made the false assumption, that multi-disk drivers always 
have disk->data as a real pointer.


> Although, of course, I don't see why can't we just make it use a pointer to
> itself:
>
>   disk->id = (unsigned long) &disk->id;
>
> but then what's the point of storing that in a variable anyway.  We might as
> well just remove this variable and whoever uses it can use a pointer to the
> structure instead?
>
> This works on the assumption that disk structures are never reallocated, but
> I suppose that's a sane thing to assume...
>
>   

Sounds good.

Probably add an inline function to improve readability:

typedef unsigned long grub_disk_id_t;

inline grub_disk_id_t disk_id (const grub_disk_t * disk)
{
 return (grub_id_t)(disk);
}

...
data = grub_disk_cache_fetch (disk->dev->id, disk_id(disk), start_sector);


Christian