At Sun, 9 Jul 2006 15:29:17 +0200, Yoshinori K. Okuji wrote: > > On Sunday 09 July 2006 14:01, Jeroen Dekkers wrote: > > I don't see anything like that in the code when I grep for > > total_sectors. The only code that is using total_sectors as the size > > of the disk is the grub_disk_check_range(), but that function is also > > interested in whether the offset/size fits in the partition when > > reading from a partition. If total_sectors was the size of the > > partition, the two ifs in grub_disk_check_range() could be merged to > > just one if for both cases. > > Probably the idea was only in my mind. There are so many things that are not > implemented yet! > > > Well, both users of total_sectors are assuming the semantics I'm > > proposing, which only reinforces my point that it's the more logical > > thing to do. > > No. I don't think we can agree on this, since you don't see my point. > > GRUB is based on an object-oriented design. In object-oriented programming, > how attributes are stored is an implementation detail, so attributes should > not be accessed directly from the outside. On the other hand, it is sometimes > more convenient or just faster to access attributes directly. In this sense, > the current code in GRUB is a mixture. > > However, my wish is to enforce the object-oriented paradigm as much as > possible. What should the disk structure store as information? Of course, > about the disk itself. What should the partition structure as information? Of > course, about the partition itself. What should total_sectors store? Of > course, about the disk, since it is a part of the disk structure. Is it > inconvenient? Then, define an accessor appropriately. > > Your proposal simply breaks the rule of object-encapsulation in > object-oriented programming, so I never agree with you about this.
Well, if you consider the total_sectors a private variable, and you want to have accessor functions for accessing it, then I can understand your point a bit, but such things will make the kernel bigger and I thought it was a goal to keep it as small as possible... I was however thinking about total_sectors as a public variable that gives the size of the device being opened. I don't really think a partition is an attribute of a disk (all the partitions might be an attribute of the disk, but just not a random one). I think it makes more sense to consider a partition an object of the same class as a disk, given that they have the same semantics. We might have to refactor this code for LVM too, if we consider LVM to be an advanced partition table. I haven't really looked into implementing LVM yet, but it's more like a partition table than a disk. It might make sense to allow people to access it using (volumegroup, volumename) syntax, like (hardisk, partitionnumber). You can't just add offsets in grub_disk_check_range like you can with DOS partitions however. But maybe implementing LVM by adding a new kind of disk device might be better. Anyway, do you want me to write a grub_disk_get_size() function? Jeroen Dekkers _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
