[PATCH] use of grub_dprintf in grub_mm_init_region() is misleading

[PATCH] use of grub_dprintf in grub_mm_init_region() is misleading

[Robert Millan]

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


Using grub_dprintf() in grub_mm_init_region() easily leads to think that one
can activate this debug message by issuing `grub_env_set ("debug", "mem")' in
grub_machine_init(), before memory initialisation.

However, that's completely false.  grub_env_set will silently [1] ignore your
call because grub_malloc() ain't usable, possibly making you think for a while
that no free memory was found, and that memory was not initialized at all!

I propose turning it into a disabled grub_printf().

[1] well, it'll return an error code.  but who remembers about checking that?

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)

[-- Attachment #2: mem_dprintf.diff --]
[-- Type: text/x-diff, Size: 694 bytes --]


	* kern/mm.c (grub_mm_init_region): Replace grub_dprintf() call with
	#ifdef'ed out grub_printf().

diff -x CVS -x '*~' -x '*.mk' -urp ../grub2/kern/mm.c ./kern/mm.c
--- ../grub2/kern/mm.c	2008-01-21 22:13:11.000000000 +0100
+++ ./kern/mm.c	2008-01-23 13:30:53.000000000 +0100
@@ -143,7 +143,9 @@ grub_mm_init_region (void *addr, grub_si
   grub_mm_header_t h;
   grub_mm_region_t r, *p, q;
 
-  grub_dprintf ("mem", "Using memory for heap: start=%p, end=%p\n", addr, addr + (unsigned int) size);
+#if 0
+  grub_printf ("Using memory for heap: start=%p, end=%p\n", addr, addr + (unsigned int) size);
+#endif
 
   /* If this region is too small, ignore it.  */
   if (size < GRUB_MM_ALIGN * 2)

Re: [PATCH] use of grub_dprintf in grub_mm_init_region() is misleading

[Marco Gerards]

Robert Millan <rmh@aybabtu.com> writes:

> Using grub_dprintf() in grub_mm_init_region() easily leads to think that one
> can activate this debug message by issuing `grub_env_set ("debug", "mem")' in
> grub_machine_init(), before memory initialisation.
>
> However, that's completely false.  grub_env_set will silently [1] ignore your
> call because grub_malloc() ain't usable, possibly making you think for a while
> that no free memory was found, and that memory was not initialized at all!
>
> I propose turning it into a disabled grub_printf().

Fine for me.

--
Marco