Re: [PATCH] Set size of partition correctly in grub_disk_open()

[Jeroen Dekkers]

At Sat, 8 Jul 2006 18:07:44 +0200,
Yoshinori K. Okuji wrote:
> 
> Hi Jeroen,
> 
> On Wednesday 05 July 2006 22:54, Jeroen Dekkers wrote:
> > +
> > +      disk->total_sectors = grub_partition_get_len (disk->partition);
> > +    }
> 
> But I don't agree with this part. The member "total_sectors" must describe 
> the 
> size of the whole disk but not of the partition. This is because GRUB 
> accesses partitions through the disk interface. When accessing a partition, 
> GRUB tweaks offsets by the start sector of the partition. If not, it is 
> another bug.

grub_disk_check_range() increases the sector read so sector 0 is the
first sector of the partition. So if you open a partition, you will
have to read from 0 until the length of the partition, and that will
give you the partition. Now I'm reading the code again I see that the
"out of disk" check needs to be adapted to this change too.

But it's a little bit illogical that the size you get from the same
disk structure isn't the size of the partition, but the size of
something else. It's also not really useful: if you're opening a
partition, your are interested in the size of the partition most of
the time, not the size of the disk the partition is on. The AFFS code
already assumes that the total_sectors is the size of the partition
and the blocklist code does that too, if you want to allow to read a
blocklist from a partition.

Not having total_sectors the size of the partition also makes it
impossible to write generic code for both disks and
partitions. Everytime you want to get the size of a device, you've to
check whether the device is a disk or a partition. This will enlarge
the code unnecessary.

So I don't really see why total_sectors should be the size of the disk
the partition is on instead of the size of the partition.

Jeroen Dekkers