On 07/08/2010 02:28, Vladimir 'φ-coder/phcoder' Serbinenko wrote:

Attached is the new version of the patch.

> As I already told you in real dprintf isn't seen by user. One need to
> grub_dprintf (....);
> #ifdef GRUB_UTIL
> grub_util_warn (...);
> #endif

Ok.  For partition.c, this is now done in the checking function to avoid
code duplication (and it makes the code easier to read).

For bsdlabel.c, I re-ordered the code so that no warning is shown for
partitions of type unused (such as the raw partition).

>> I simplified the check in grub_disk_adjust_range: no need to check for
>> the ``ancestor'' partitions, but we still check for the given partition.
>>
> You're right. This means we can't screw this test to save space. In this
> case it's better to do the complete check for early bug catch.

I removed the simplification from the patch.

> We shouldn't check for partitions being outside of disk since BIOS disk
> size limitations are common. Consider following situation:
> (hd0,msdos1,bsd1)           /boot
> BIOS LIMIT
> (hd0,msdos1,bsd2)          /
> This system is perfectly capable of booting but with your patch it won't.

Right.  The patch now only checks sub-partitions (no check is done on
top-level partitions).

 > We must always exercice best effort strategy. If something can bee
 > booted, boot it.

Here, we discard improperly nested partitions, even though they could
be accessed.  So one may argue that this breaks the best effort
strategy.  However, such improperly nested partitions can in general be
accessed by a properly nested identifier, so I guess it's fine.

> We should warn if a used final-nestedness partition is partialy outside
> the limit. Simple message usually scrolls way too fast and so usually
> ignored (if someone sees boot process at all). Perhaps we need a way to
> pass such warnings to kernel which then can take appropriate action
> (e.g. notify sysadmin)

I'm not sure where we should check that.  If one attempts to load a
kernel or module that is beyond the disk limit, grub_disk_read will
fail anyway.

Grégoire