From: "Soeren D. Schulze" <soeren.d.schulze@gmx.de>
To: grub-devel@gnu.org
Subject: [patch] Bugs in the multiboot example kernel implementation
Date: Sat, 27 Feb 2010 03:25:31 +0100 [thread overview]
Message-ID: <4B88829B.7090201@gmx.de> (raw)
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 <baford@cs.utah.edu>
@@ -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 technique\x7f42685
Node: Example OS code\x7f43902
Node: multiboot.h\x7f45444
-Node: boot.S\x7f53515
-Node: kernel.c\x7f56668
-Node: Other Multiboot kernels\x7f65537
-Node: Example boot loader code\x7f65968
-Node: History\x7f66494
-Node: Index\x7f67716
+Node: boot.S\x7f53558
+Node: kernel.c\x7f56711
+Node: Other Multiboot kernels\x7f65990
+Node: Example boot loader code\x7f66421
+Node: History\x7f66947
+Node: Index\x7f68169
\x1f
End Tag Table
next reply other threads:[~2010-02-27 2:25 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-02-27 2:25 Soeren D. Schulze [this message]
2010-03-14 13:34 ` [patch] Bugs in the multiboot example kernel implementation Vladimir 'φ-coder/phcoder' Serbinenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4B88829B.7090201@gmx.de \
--to=soeren.d.schulze@gmx.de \
--cc=grub-devel@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.