[Patch] Use absolute offset for embedding area in grub-setup (i386-pc)

[Patch] Use absolute offset for embedding area in grub-setup (i386-pc)

[Grégoire Sutre]

[-- Attachment #1: Type: text/plain, Size: 573 bytes --]

Hi,

The attached patch uses grub_partition_get_start (p) instead of p->start
in grub-setup's iteration over partitions.  While we are at it, also
use grub_partition_get_len (p) instead of p->len.

An alternative would be to assume that partitions are properly nested,
and therefore iterate only over the top-level ones (i.e. those with
p->parent == NULL).  In that case, we could equivalently use p->start.


In the check for non-empty embedding area, the patch uses <= instead of
== (as we may have embed_region.start == 1 && embed_region.end == 0).

Comments?

Grégoire

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: grub-setup.diff --]
[-- Type: text/x-patch; name="grub-setup.diff", Size: 1781 bytes --]

=== modified file 'ChangeLog'
--- ChangeLog	2010-06-03 08:48:23 +0000
+++ ChangeLog	2010-06-05 00:14:38 +0000
@@ -1,3 +1,8 @@
+2010-06-05  Grégoire Sutre  <gregoire.sutre@gmail.com>
+
+	* util/i386/pc/grub-setup.c (setup): Use absolute offsets for start of
+	embedding area.  Use <= instead of == when checking for non-emptyness.
+
 2010-06-03  Colin Watson  <cjwatson@ubuntu.com>
 
 	* INSTALL: Document several build requirements for optional features

=== modified file 'util/i386/pc/grub-setup.c'
--- util/i386/pc/grub-setup.c	2010-05-17 19:26:16 +0000
+++ util/i386/pc/grub-setup.c	2010-06-04 23:53:08 +0000
@@ -126,8 +126,8 @@ setup (const char *dir,
       /* There's always an embed region, and it starts right after the MBR.  */
       embed_region.start = 1;
 
-      if (embed_region.end > p->start)
-	embed_region.end = p->start;
+      if (embed_region.end > grub_partition_get_start (p))
+	embed_region.end = grub_partition_get_start (p);
 
       return 0;
     }
@@ -147,8 +147,8 @@ setup (const char *dir,
       /* If there's an embed region, it is in a dedicated partition.  */
       if (! memcmp (&gptdata.type, &grub_gpt_partition_type_bios_boot, 16))
 	{
-	  embed_region.start = p->start;
-	  embed_region.end = p->start + p->len;
+	  embed_region.start = grub_partition_get_start (p);
+	  embed_region.end = grub_partition_get_start (p) + grub_partition_get_len (p);
 
 	  return 1;
 	}
@@ -361,7 +361,7 @@ setup (const char *dir,
   else
     grub_util_error (_("No DOS-style partitions found"));
 
-  if (embed_region.end == embed_region.start)
+  if (embed_region.end <= embed_region.start)
     {
       if (! strcmp (dest_partmap, "msdos"))
 	grub_util_warn (_("This msdos-style partition label has no post-MBR gap; embedding won't be possible!"));

Re: [Patch] Use absolute offset for embedding area in grub-setup (i386-pc)

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1.1: Type: text/plain, Size: 901 bytes --]

On 06/05/2010 02:22 AM, Grégoire Sutre wrote:
> Hi,
>
> The attached patch uses grub_partition_get_start (p) instead of p->start
> in grub-setup's iteration over partitions.  While we are at it, also
> use grub_partition_get_len (p) instead of p->len.
Go ahead
>
> An alternative would be to assume that partitions are properly nested,
> and therefore iterate only over the top-level ones (i.e. those with
> p->parent == NULL).  In that case, we could equivalently use p->start.
>
>
> In the check for non-empty embedding area, the patch uses <= instead of
> == (as we may have embed_region.start == 1 && embed_region.end == 0).
>
> Comments?
>
> Grégoire
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>   


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko


[-- Attachment #1.2: Type: text/html, Size: 1678 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]