This restructures code flow in the setup() function to make it easier to
work with. The current layout is really confusing; instead, I propose
a "try-fail" approach, like:
if (problem1)
{
grub_util_warn ("warning1");
goto unable_to_embed;
}
if (problem2)
{
grub_util_warn ("warning2");
goto unable_to_embed;
}
[...]
To make my patch more readable I intentionally omitted indentation changes
from it (which I'll add when committing, of course).
--
Robert Millan
The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
how) you may access your data; but nobody's threatening your freedom: we
still allow you to remove your data and not access it at all."
2009-05-10 Robert Millan <[email protected]>
* util/i386/pc/grub-setup.c (setup): Restructure code flow to make
it easier to understand / work with.
Index: util/i386/pc/grub-setup.c
===================================================================
--- util/i386/pc/grub-setup.c (revision 2199)
+++ util/i386/pc/grub-setup.c (working copy)
@@ -310,17 +310,19 @@
dos_part, bsd_part);
if (! dest_dev->disk->has_partitions)
+ {
grub_util_warn ("Attempting to install GRUB to a partitionless disk. This is a BAD idea.");
+ goto unable_to_embed;
+ }
if (dest_dev->disk->partition)
+ {
grub_util_warn ("Attempting to install GRUB to a partition instead of the MBR. This is a BAD idea.");
+ goto unable_to_embed;
+ }
- /* If the destination device can have partitions and it is the MBR,
- try to embed the core image into after the MBR. */
- if (dest_dev->disk->has_partitions && ! dest_dev->disk->partition)
- {
/* Unlike root_dev, with dest_dev we're interested in the partition map even
- if dest_dev itself is a whole disk. */
+ if dest_dev itself is a whole disk. */
auto int NESTED_FUNC_ATTR identify_partmap (grub_disk_t disk,
const grub_partition_t p);
int NESTED_FUNC_ATTR identify_partmap (grub_disk_t disk __attribute__ ((unused)),
@@ -330,16 +332,22 @@
return 1;
}
grub_partition_iterate (dest_dev->disk, identify_partmap);
-
+
grub_partition_iterate (dest_dev->disk, (strcmp (dest_partmap, "pc_partition_map") ?
find_usable_region_gpt : find_usable_region_msdos));
-
- if (embed_region.end != embed_region.start)
- embedding_area_exists = 1;
-
- /* If there is enough space... */
- if ((unsigned long) core_sectors <= embed_region.end - embed_region.start)
- {
+ if (embed_region.end == embed_region.start)
+ {
+ grub_util_warn ("Embedding area is not present at all!");
+ goto unable_to_embed;
+ }
+
+ if ((unsigned long) core_sectors > embed_region.end - embed_region.start)
+ {
+ grub_util_warn ("Embedding area is too small for core.img.");
+ goto unable_to_embed;
+ }
+
+
grub_util_info ("will embed the core image at sector 0x%llx", embed_region.start);
*install_dos_part = grub_cpu_to_le32 (dos_part);
@@ -374,16 +382,9 @@
grub_util_error ("%s", grub_errmsg);
goto finish;
- }
- }
- /* If we reached this point, it means we were unable to embed. */
+unable_to_embed:
- if (embedding_area_exists)
- grub_util_warn ("Embedding area is too small for core.img.");
- else
- grub_util_warn ("Embedding area is not present at all!");
-
if (must_embed)
grub_util_error ("Embedding is not possible, but this is required when "
"the root device is on a RAID array or LVM volume.");
_______________________________________________
Grub-devel mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/grub-devel
