Hi,
I found some bugs in kernel/printk.c file. To test printk() and
panic(), I inserted the next code in init_task(), after mounting the
root filesystem.
printk("Printing 1234 using %%d, %d\n", 1234);
printk("Printing -1234 using %%d, %d\n", -1234);
printk("Printing 1234 using %%8d, %8d\n", 1234);
printk("Printing -1234 using %%8d, %8d\n", -1234);
printk("Printing 1234 using %%08d, %08d\n", 1234);
printk("Printing -1234 using %%08d, %08d\n", -1234);
panic("Now testing panic with 2 int parameters: %d, %d\n", 22, 33);
After booting ELKS with the original printk.c, the console shows:
Printing 1234 using %d, 1234
Printing -1234 using %d, -1234
Printing 1234 using %8d, 1234
Printing -1234 using %8d, - 1234
Printing 1234 using %08d, 00001234
Printing -1234 using %08d, -00001234
panic: Now testing panic with 2 int parameters: 22, 1
apparent call stack:
Line: Addr Parameters
~~~~: ~~~~ ~~~~~~~~~~
0: 012D => 0000 0000 0000 0000 0000 0000 0000
SYSTEM LOCKED - Press CTRL-ALT-DEL to reboot:
With format %8d and %08d and negative numbers, the resulting field
width is 9 instead of 8. With format %8d the sign is separated from
the number. In the function panic(), only the first parameter is
printed correctly, in the place of the second parameter prints garbage.
Only one stack frame is printed, instead of the 9 frames that
the code suggest.
After booting ELKS with the modified printk.c, the console shows:
Printing 1234 using %d, 1234
Printing -1234 using %d, -1234
Printing 1234 using %8d, 1234
Printing -1234 using %8d, -1234
Printing 1234 using %08d, 00001234
Printing -1234 using %08d, -0001234
panic: Now testing panic with 2 int parameters: 22, 1
apparent call stack:
Line: Addr Parameters
~~~~: ~~~~ ~~~~~~~~~~
0: 012D => 0000 0000 0000 0000 0000 0000 0000
1: 0000 => 23E8 00AD 9F40 2CD8 E76C 0092 3FF2
2: 009E => E5E0 1F20 E76C 0000 0000 0001 0000
3: 001E => 0000 0000 0000 0000 0000 0000 0000
4: 0000 => 0000 0000 0000 0000 0000 0000 0000
5: 0000 => 0000 0000 0000 0000 0000 0000 0000
6: 0000 => 0000 0000 0000 0000 0000 0000 0000
7: 0000 => 0000 0000 0000 0000 0000 0000 0000
8: 0000 => 0000 0000 0000 0000 0000 0000 0000
SYSTEM LOCKED - Press CTRL-ALT-DEL to reboot:
Negative numbers are now printed correctly. With panic(),
9 stack frames are printed. Still, only the first parameter
is printed correctly. Code size is reduced in 32 bytes.
The patch to fix printk() and panic() bugs follows:
diff -Nurb elks.orig/kernel/printk.c elks/kernel/printk.c
--- elks.orig/kernel/printk.c 2004-05-31 08:49:25.000000000 -0500
+++ elks/kernel/printk.c 2012-01-16 09:46:18.000000000 -0600
@@ -25,18 +25,14 @@
* MTK: Sep 97 - Misc hacks to shrink generated code
*/
-#include <linuxmt/fcntl.h>
-#include <linuxmt/mm.h>
-#include <linuxmt/sched.h>
-#include <linuxmt/types.h>
-
#include <arch/segment.h>
+#include <linuxmt/mm.h>
/*
* Just to make it work for now
*/
-extern void con_charout();
+extern void con_charout(char);
static void con_write(register char *buf, int len)
{
@@ -48,11 +44,10 @@
static char colour[8] = { 27, '[', '3', '0', ';', '4', '0', 'm' };
- p = colour;
-
- if (++(p[3]) == '8')
- p[3] = '1';
+ if (++(colour[3]) == '8')
+ colour[3] = '1';
+ p = colour;
do
con_charout(*p);
while (*p++ != 'm');
@@ -81,42 +76,51 @@
static void numout(char *ptr, int len, int width, int base, int useSign,
int Upper, int Zero)
{
- long int vs;
unsigned long int v;
- register char *bp, *bp2;
- char buf[32];
-
- if (width > 31) /* Error-check width specified */
- width = 31;
+ register char *bp;
+ char buf[12];
- bp = bp2 = buf + 31;
+ if (width > sizeof(buf)) /* Error-check width specified */
+ width = sizeof(buf);
- if (useSign) {
- vs = (len == 2) ? *((short *) ptr) : *((long *) ptr);
- if (vs < 0) {
- v = - vs;
- *bp = '-';
- con_write(bp, 1);
- } else
- v = vs;
- } else
v = (len == 2) ? *((unsigned short *) ptr) : *((unsigned long *) ptr);
+ if (useSign) {
+ if (len == 2)
+ v = ((long)(*((short *) ptr)));
+ if ((long)v < 0)
+ v = (-(long)v);
+ else
+ useSign = 0;
+ }
+
+ bp = buf + sizeof(buf);
- *bp = 0;
+ {
+ register char *bp2;
+
+ bp2 = Upper ? hex_string : hex_lower;
do {
- if (Upper)
- *--bp = hex_string[v % base]; /* Store digit */
- else
- *--bp = hex_lower[v % base]; /* Store digit */
- } while ((v /= base) && (bp > buf));
+ *--bp = *(bp2 + (v % base)); /* Store digit */
+ } while ((v /= base));
+ }
+
+ if (useSign && !Zero)
+ *--bp = '-';
- while (bp2 - bp < width) /* Process width */
+ width -= buf - bp + sizeof(buf);
+ while (--width >= 0) /* Process width */
if (Zero)
*--bp = '0';
else
*--bp = ' ';
- con_write(bp, buf - bp + sizeof(buf) - 1);
+ if (useSign && Zero) {
+ if (*bp != '0')
+ bp--;
+ *bp = '-';
+ }
+
+ con_write(bp, buf + sizeof(buf) - bp);
}
void printk(register char *fmt,int a1)
@@ -180,7 +184,7 @@
con_write(cp++, 1);
width--;
}
- while (width-- > 0)
+ while (--width >= 0)
con_write(" ", 1);
break;
case 't':
@@ -191,11 +195,11 @@
cp++;
width--;
}
- while (width-- > 0)
+ while (--width >= 0)
con_write(" ", 1);
break;
case 'c':
- while (width-- > 1)
+ while (--width >= 1)
con_write(" ", 1);
con_write(p, 1);
p += 2;
@@ -224,7 +228,7 @@
for (j = 2; j <= 8; j++)
printk(" %04X", bp[j]);
printk("\n");
- } while (++i > 9);
+ } while (++i < 9);
/* Lock up with infinite loop */
Regards,
Juan
--
To unsubscribe from this list: send the line "unsubscribe linux-8086" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
