From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1NlCND-0001Ct-7S for mharc-grub-devel@gnu.org; Fri, 26 Feb 2010 21:25:43 -0500 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NlCNB-0001Cn-49 for grub-devel@gnu.org; Fri, 26 Feb 2010 21:25:41 -0500 Received: from [140.186.70.92] (port=44919 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NlCN8-0001CW-Vc for grub-devel@gnu.org; Fri, 26 Feb 2010 21:25:39 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1NlCN7-0005oH-AR for grub-devel@gnu.org; Fri, 26 Feb 2010 21:25:38 -0500 Received: from mail.gmx.net ([213.165.64.20]:52895) by eggs.gnu.org with smtp (Exim 4.69) (envelope-from ) id 1NlCN6-0005nu-PV for grub-devel@gnu.org; Fri, 26 Feb 2010 21:25:37 -0500 Received: (qmail invoked by alias); 27 Feb 2010 02:25:33 -0000 Received: from p5B3B2949.dip.t-dialin.net (EHLO [192.168.1.1]) [91.59.41.73] by mail.gmx.net (mp011) with SMTP; 27 Feb 2010 03:25:33 +0100 X-Authenticated: #9357126 X-Provags-ID: V01U2FsdGVkX18cGZs14Mwclcex/uyIOZ73BXMwtpxlxhDBRUx39K Afr16Wbkov3eAz Message-ID: <4B88829B.7090201@gmx.de> Date: Sat, 27 Feb 2010 03:25:31 +0100 From: "Soeren D. Schulze" User-Agent: Mozilla-Thunderbird 2.0.0.22 (X11/20090706) MIME-Version: 1.0 To: grub-devel@gnu.org Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Y-GMX-Trusted: 0 X-FuHaFi: 0.46000000000000002 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) Subject: [patch] Bugs in the multiboot example kernel implementation X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 27 Feb 2010 02:25:41 -0000 I think there are two bugs in the example implementation of the multiboot standard version 0.6.96. Both concern the display of the memory map. First, the `multiboot_uint64_t' type does not seem to be working as expected. I always get 32-bit there. I personally wouldn't use that type anyway, so I used two 32-bit variables in place. Second, the memory addresses *must* be zero-padded when chaining the output of two `itoa' calls this way, so I introduced the (non-standard) '%X' specifier. As this is the first time I tinker with GRUB code and also almost the first time I edit kernel code, it probably breaks lots of conventions... But I hope it can be turned into something useful. For the legal stuff: I hereby put this patch into the public domain; as I'm not sure if this is possible in Germany, I explicitly grant anyone the right to use this patch in any thinkable way, including removing any reference to me and relicensing, as a whole or in parts, and claiming copyright for it. diff -aur multiboot-0.6.96/docs/kernel.c multiboot-0.6.96.new/docs/kernel.c --- multiboot-0.6.96/docs/kernel.c 2009-12-24 15:23:08.000000000 +0100 +++ multiboot-0.6.96.new/docs/kernel.c 2010-02-27 02:34:52.000000000 +0100 @@ -141,13 +141,13 @@ (unsigned long) mmap < mbi->mmap_addr + mbi->mmap_length; mmap = (multiboot_memory_map_t *) ((unsigned long) mmap + mmap->size + sizeof (mmap->size))) - printf (" size = 0x%x, base_addr = 0x%x%x," - " length = 0x%x%x, type = 0x%x\n", + printf (" size = 0x%x, base_addr = 0x%X%X," + " length = 0x%X%X, type = 0x%x\n", (unsigned) mmap->size, - mmap->addr >> 32, - mmap->addr & 0xffffffff, - mmap->len >> 32, - mmap->len & 0xffffffff, + mmap->addr1, + mmap->addr0, + mmap->len1, + mmap->len0, (unsigned) mmap->type); } } @@ -185,7 +185,7 @@ buf++; ud = -d; } - else if (base == 'x') + else if (base == 'x' || base == 'X') divisor = 16; /* Divide UD by DIVISOR until UD == 0. */ @@ -213,6 +213,23 @@ } } +static void +pad0 (char *numstr, int n) +{ + int i, len; + + /* strlen() */ + for (len = 0; numstr[len]; len++); + + /* shift right */ + for (i = len - 1; i >= 0; i--) + numstr[i + n - len] = numstr[i]; + + /* pad */ + for (i = 0; i < n - len; i++) + numstr[i] = '0'; +} + /* Put the character C on the screen. */ static void putchar (int c) @@ -260,8 +277,11 @@ case 'd': case 'u': case 'x': + case 'X': itoa (buf, c, *((int *) arg++)); p = buf; + if (c == 'X') + pad0 (p, 8); goto string; break; diff -aur multiboot-0.6.96/docs/kernel.c.texi multiboot-0.6.96.new/docs/kernel.c.texi --- multiboot-0.6.96/docs/kernel.c.texi 2009-12-24 15:23:11.000000000 +0100 +++ multiboot-0.6.96.new/docs/kernel.c.texi 2010-02-27 02:35:32.000000000 +0100 @@ -141,13 +141,13 @@ (unsigned long) mmap < mbi->mmap_addr + mbi->mmap_length; mmap = (multiboot_memory_map_t *) ((unsigned long) mmap + mmap->size + sizeof (mmap->size))) - printf (" size = 0x%x, base_addr = 0x%x%x," - " length = 0x%x%x, type = 0x%x\n", + printf (" size = 0x%x, base_addr = 0x%X%X," + " length = 0x%X%X, type = 0x%x\n", (unsigned) mmap->size, - mmap->addr >> 32, - mmap->addr & 0xffffffff, - mmap->len >> 32, - mmap->len & 0xffffffff, + mmap->addr1, + mmap->addr0, + mmap->len1, + mmap->len0, (unsigned) mmap->type); @} @} @@ -185,7 +185,7 @@ buf++; ud = -d; @} - else if (base == 'x') + else if (base == 'x' || base == 'X') divisor = 16; /* @r{Divide UD by DIVISOR until UD == 0.} */ @@ -213,6 +213,23 @@ @} @} +static void +pad0 (char *numstr, int n) +@{ + int i, len; + + /* @r{strlen()} */ + for (len = 0; numstr[len]; len++); + + /* @r{shift right} */ + for (i = len - 1; i >= 0; i--) + numstr[i + n - len] = numstr[i]; + + /* @r{pad} */ + for (i = 0; i < n - len; i++) + numstr[i] = '0'; +@} + /* @r{Put the character C on the screen.} */ static void putchar (int c) @@ -260,8 +277,11 @@ case 'd': case 'u': case 'x': + case 'X': itoa (buf, c, *((int *) arg++)); p = buf; + if (c == 'X') + pad0 (p, 8); goto string; break; diff -aur multiboot-0.6.96/docs/multiboot.h multiboot-0.6.96.new/docs/multiboot.h --- multiboot-0.6.96/docs/multiboot.h 2009-12-24 15:19:40.000000000 +0100 +++ multiboot-0.6.96.new/docs/multiboot.h 2010-02-27 01:50:47.000000000 +0100 @@ -196,12 +196,14 @@ struct multiboot_mmap_entry { multiboot_uint32_t size; - multiboot_uint64_t addr; - multiboot_uint64_t len; + multiboot_uint32_t addr0; + multiboot_uint32_t addr1; + multiboot_uint32_t len0; + multiboot_uint32_t len1; #define MULTIBOOT_MEMORY_AVAILABLE 1 #define MULTIBOOT_MEMORY_RESERVED 2 multiboot_uint32_t type; -} __attribute__((packed)); +}; typedef struct multiboot_mmap_entry multiboot_memory_map_t; struct multiboot_mod_list diff -aur multiboot-0.6.96/docs/multiboot.h.texi multiboot-0.6.96.new/docs/multiboot.h.texi --- multiboot-0.6.96/docs/multiboot.h.texi 2009-12-24 15:19:41.000000000 +0100 +++ multiboot-0.6.96.new/docs/multiboot.h.texi 2010-02-27 01:51:10.000000000 +0100 @@ -196,12 +196,14 @@ struct multiboot_mmap_entry @{ multiboot_uint32_t size; - multiboot_uint64_t addr; - multiboot_uint64_t len; + multiboot_uint32_t addr0; + multiboot_uint32_t addr1; + multiboot_uint32_t len0; + multiboot_uint32_t len1; #define MULTIBOOT_MEMORY_AVAILABLE 1 #define MULTIBOOT_MEMORY_RESERVED 2 multiboot_uint32_t type; -@} __attribute__((packed)); +@}; typedef struct multiboot_mmap_entry multiboot_memory_map_t; struct multiboot_mod_list diff -aur multiboot-0.6.96/docs/multiboot.info multiboot-0.6.96.new/docs/multiboot.info --- multiboot-0.6.96/docs/multiboot.info 2009-12-24 16:38:18.000000000 +0100 +++ multiboot-0.6.96.new/docs/multiboot.info 2010-02-27 02:35:32.000000000 +0100 @@ -1,4 +1,4 @@ -This is multiboot.info, produced by makeinfo version 4.11 from +This is multiboot.info, produced by makeinfo version 4.13 from multiboot.texi. Copyright (C) 1995,96 Bryan Ford @@ -1269,12 +1269,14 @@ struct multiboot_mmap_entry { multiboot_uint32_t size; - multiboot_uint64_t addr; - multiboot_uint64_t len; + multiboot_uint32_t addr0; + multiboot_uint32_t addr1; + multiboot_uint32_t len0; + multiboot_uint32_t len1; #define MULTIBOOT_MEMORY_AVAILABLE 1 #define MULTIBOOT_MEMORY_RESERVED 2 multiboot_uint32_t type; - } __attribute__((packed)); + }; typedef struct multiboot_mmap_entry multiboot_memory_map_t; struct multiboot_mod_list @@ -1551,13 +1553,13 @@ (unsigned long) mmap < mbi->mmap_addr + mbi->mmap_length; mmap = (multiboot_memory_map_t *) ((unsigned long) mmap + mmap->size + sizeof (mmap->size))) - printf (" size = 0x%x, base_addr = 0x%x%x," - " length = 0x%x%x, type = 0x%x\n", + printf (" size = 0x%x, base_addr = 0x%X%X," + " length = 0x%X%X, type = 0x%x\n", (unsigned) mmap->size, - mmap->addr >> 32, - mmap->addr & 0xffffffff, - mmap->len >> 32, - mmap->len & 0xffffffff, + mmap->addr1, + mmap->addr0, + mmap->len1, + mmap->len0, (unsigned) mmap->type); } } @@ -1595,7 +1597,7 @@ buf++; ud = -d; } - else if (base == 'x') + else if (base == 'x' || base == 'X') divisor = 16; /* Divide UD by DIVISOR until UD == 0. */ @@ -1623,6 +1625,23 @@ } } + static void + pad0 (char *numstr, int n) + { + int i, len; + + /* strlen() */ + for (len = 0; numstr[len]; len++); + + /* shift right */ + for (i = len - 1; i >= 0; i--) + numstr[i + n - len] = numstr[i]; + + /* pad */ + for (i = 0; i < n - len; i++) + numstr[i] = '0'; + } + /* Put the character C on the screen. */ static void putchar (int c) @@ -1670,8 +1689,11 @@ case 'd': case 'u': case 'x': + case 'X': itoa (buf, c, *((int *) arg++)); p = buf; + if (c == 'X') + pad0 (p, 8); goto string; break; @@ -1800,11 +1822,11 @@ Node: I/O restriction technique42685 Node: Example OS code43902 Node: multiboot.h45444 -Node: boot.S53515 -Node: kernel.c56668 -Node: Other Multiboot kernels65537 -Node: Example boot loader code65968 -Node: History66494 -Node: Index67716 +Node: boot.S53558 +Node: kernel.c56711 +Node: Other Multiboot kernels65990 +Node: Example boot loader code66421 +Node: History66947 +Node: Index68169  End Tag Table