[PATCH] Fix or remove %b support in grub_printf()

[Pavel Roskin]

Hello!

I was looking how to fix a warning about "strict-aliasing rules", but
found a potential bug in grub_printf() instead.  The variable "mask" is
never reset to 0xffffffff once it becomes 0xff.

mask is set to 0xff for "%b" format string, which is not standard.  In
fact, it's not implemented in glibc and gcc warns about it.  I don't see
any occurrences of "%b" in GRUB sources other than in assembly.

I see from the implementation that %b should print a byte.  But the
failure to reset mask would affect the rest of the string.  I think it's
broken unused code.

I've attached two alternative patches.  If %b is not used, it should be
removed.  See remove_percent_b.diff.  If %b is used or may be used, it
would be easier to implement it properly in convert_to_ascii().  See
fix_percent_b.diff.

-- 
Regards,
Pavel Roskin

--- stage2/char_io.c
+++ stage2/char_io.c
@@ -107,6 +107,9 @@ convert_to_ascii (char *buf, int c,...)
   char *ptr = buf;
 
 #ifndef STAGE1_5
+  if (c == 'b')
+    num &= 0xFFuL;
+
   if (c == 'x' || c == 'X' || c == 'b')
     mult = 16;
 
@@ -154,7 +157,6 @@ grub_printf (const char *format,...)
 {
   int *dataptr = (int *) &format;
   char c, str[16];
-  unsigned long mask = 0xFFFFFFFF;
   
   dataptr++;
 
@@ -167,15 +169,12 @@ grub_printf (const char *format,...)
          {
 #ifndef STAGE1_5
          case 'b':
-           mask = 0xFF;
-           /* Fall down intentionally!  */
          case 'd':
          case 'x':
          case 'X':
 #endif
          case 'u':
-           *convert_to_ascii (str, c, *((unsigned long *) dataptr++) & mask)
-             = 0;
+           *convert_to_ascii (str, c, *((unsigned long *) dataptr++)) = 0;
            grub_putstr (str);
            break;
 

--- stage2/char_io.c
+++ stage2/char_io.c
@@ -107,7 +107,7 @@ convert_to_ascii (char *buf, int c,...)
   char *ptr = buf;
 
 #ifndef STAGE1_5
-  if (c == 'x' || c == 'X' || c == 'b')
+  if (c == 'x' || c == 'X')
     mult = 16;
 
   if ((num & 0x80000000uL) && c == 'd')
@@ -154,7 +154,6 @@ grub_printf (const char *format,...)
 {
   int *dataptr = (int *) &format;
   char c, str[16];
-  unsigned long mask = 0xFFFFFFFF;
   
   dataptr++;
 
@@ -166,16 +165,12 @@ grub_printf (const char *format,...)
        switch (c = *(format++))
          {
 #ifndef STAGE1_5
-         case 'b':
-           mask = 0xFF;
-           /* Fall down intentionally!  */
          case 'd':
          case 'x':
          case 'X':
 #endif
          case 'u':
-           *convert_to_ascii (str, c, *((unsigned long *) dataptr++) & mask)
-             = 0;
+           *convert_to_ascii (str, c, *((unsigned long *) dataptr++)) = 0;
            grub_putstr (str);
            break;
 

_______________________________________________
Bug-grub mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/bug-grub