Re: [PATCH] Fix packing issue of machine_mmap_entry

[Christian Franke <Christian.Franke@t-online.de>]

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

Marco Gerards wrote:

>> ...
>> 	Add compile time assert to check packing.
>>     
>
> Can you remove the compile time assert?

Done.

> We usually check stuff like
> this using configure.  If you can send in a patch for configure.ac,
> that would be appreciated.
>
>   
Yes, but be patient.

The configure approach is quite different: In configure, you can check 
whether compiler supports attr packed and whether it works as expected. 
With the good old typedef compile time assert, you can check whether 
this *specific* structure is properly declared and packed.


>> --- grub2.orig/include/grub/i386/pc/init.h	2007-07-22 01:32:23.000000000 +0200
>> +++ grub2/include/grub/i386/pc/init.h	2007-10-13 21:25:24.000000000 +0200
>> @@ -40,10 +40,14 @@ grub_uint32_t grub_get_eisa_mmap (void);
>>  struct grub_machine_mmap_entry
>>  {
>>    grub_uint32_t size;
>> -  grub_uint64_t addr;
>> +  grub_uint64_t addr; /* must be at offset 4, see startup.S */ 
>>     
>
> I do not think this comment is required.  It's fixed now :-)
>
>   

Hmm...OK removed. Now there is no clue why this struct has packing 
requirements. And this also is no longer checked.

Christian


2007-11-09  Christian Franke  <franke@computer.org>

	* include/grub/i386/pc/init.h (struct grub_machine_mmap_entry):
	Add attribute packed, gcc 3.4.4 on Cygwin aligns this
	to 64 bit boundary by default.


[-- Attachment #2: grub2-machine_mmap_entry-2.patch --]
[-- Type: text/x-patch, Size: 361 bytes --]

--- grub2.orig/include/grub/i386/pc/init.h	2007-10-31 23:55:57.093750000 +0100
+++ grub2/include/grub/i386/pc/init.h	2007-11-09 20:27:52.312500000 +0100
@@ -39,7 +39,7 @@
   grub_uint64_t addr;
   grub_uint64_t len;
   grub_uint32_t type;
-};
+} __attribute__((packed));
 
 /* Get a memory map entry. Return next continuation value. Zero means
    the end.  */