A printf() problem O_o

[HelpMePlease]

Firstly, let me say Hey to everyone! Im new here, though not new to this topic...

I'm developing a unix-like microkernel based os, and have some problems in meeting the standards, so to say O_o

I have started all over with this project and implemented most of low-level functions for my os in past two days and started working on other stuf when I noticed that my printf function wasn't quite compatible with existing standards... The problem occures when a new line needs to be inserted, not always, just in cases i have another flag right infront of '\n' flag...

For example, if i would write:

Code: Select all

printf("Hey guys,\nMy name is Petar.");

I would get:

Code: Select all

Hey guys,
My name is Petar.

but,

Code: Select all

int i = 25;
printf("The number is: %i\nBecause I said so...", i);

would produce:

Code: Select all

The number is: 25Because I said so...

This again doesn't happen if the \n flag isnt right after %i flag... e.g.

Code: Select all

printf("The number is: %i \nBecause I said so...", i);

would give

Code: Select all

The number is: 25
Because I said so...

So even a space in between the two makes a difference...

This only happens when %(something) type of flags precedes \n or simmilir flag (e.g. \t, \r)

The printf function code depends upon putchar function and it (the putchar) takes care of \n flag. The code is mostly taken from linux v0.01

Here is the putchar() fuunction:

Code: Select all

// Writes a single character out to the screen.
extern void putchar(char c)
{
    // The background colour is black (0), the foreground is white (15).
    u8int backColour = 0;
    u8int foreColour = 15;

    // The attribute byte is made up of two nibbles - the lower being the 
    // foreground colour, and the upper the background colour.
    u8int  attributeByte = (backColour << 4) | (foreColour & 0x0F);
    // The attribute byte is the top 8 bits of the word we have to send to the
    // VGA board.
    u16int attribute = attributeByte << 8;
    u16int *location;

    // Handle a backspace, by moving the cursor back one space
    if (c == 0x08 && cursor_x)
    {
        cursor_x--;
    }

    // Handle a tab by increasing the cursor's X, but only to a point
    // where it is divisible by 8.
    else if (c == 0x09)
    {
        cursor_x = (cursor_x+8) & ~(8-1);
    }

    // Handle carriage return
    else if (c == '\r')
    {
        cursor_x = 0;
    }

    // Handle newline by moving cursor back to left and increasing the row
    else if (c == '\n')
    {
        cursor_x = 0;
        cursor_y++;
    }
    // Handle any other printable character.
    else if(c >= ' ')
    {
        location = video_memory + (cursor_y*80 + cursor_x);
        *location = c | attribute;
        cursor_x++;
    }

    // Check if we need to insert a new line because we have reached the end
    // of the screen.
    if (cursor_x >= 80)
    {
        cursor_x = 0;
        cursor_y ++;
    }

    // Scroll the screen if needed.
    scroll();
    // Move the hardware cursor.
    move_cursor();

}

and here is the printf() function:

Code: Select all

     /* Format a string and print it on the screen, just like the libc
        function printf. */
     extern void
     printf (const char *format, ...)
{
       char **arg = (char **) &format;
       int c;
       char buf[20];

	int len;
	int i = 0;
	char * str;
	char *s;
	int *ip;

	int flags;		/* flags to number() */

	int field_width;	/* width of output field */
	int precision;		/* min. # of digits for integers; max
				   number of chars for from string */
	int qualifier;		/* 'h', 'l', or 'L' for integer fields */

while ((c = *format++) != 0)
         {
	if (c != '%') 
		putchar(c);
           else
             {
               char *p;
     
               c = *format++;

	/* process flags */
		flags = 0;
		repeat:
			++format;		/* this also skips first '%' */
			switch (*format) {
				case '-': flags |= LEFT; goto repeat;
				case '+': flags |= PLUS; goto repeat;
				case ' ': flags |= SPACE; goto repeat;
				case '#': flags |= SPECIAL; goto repeat;
				case '0': flags |= ZEROPAD; goto repeat;
				}
		
		/* get field width */
		field_width = -1;
		if (is_digit(*format))
			field_width = skip_atoi(&format);
		else if (*format == '*') {
			/* it's the next argument */
			field_width = va_arg(arg, int);
			if (field_width < 0) {
				field_width = -field_width;
				flags |= LEFT;
			}
		}

		/* get the precision */
		precision = -1;
		if (*format == '.') {
			++format;	
			if (is_digit(*format))
				precision = skip_atoi(&format);
			else if (*format == '*') {
				/* it's the next argument */
				precision = va_arg(arg, int);
			}
			if (precision < 0)
				precision = 0;
		}

		/* get the conversion qualifier */
		qualifier = -1;
		if (*format == 'h' || *format == 'l' || *format == 'L') {
			qualifier = *format;
			++format;
		} 
       arg++;

		switch (c) {
		case 'c':
			if (!(flags & LEFT))
				while (--field_width > 0)
					*str++ = ' ';
			*str++ = (unsigned char) va_arg(arg, int);
			while (--field_width > 0)
				*str++ = ' ';
			break;

		case 's':
			p = *arg++;
                   if (! p)
                     p = "(null)";

		case 'o':
			str = number(str, va_arg(arg, unsigned long), 8,
				field_width, precision, flags);
			break;

		case 'p':
			if (field_width == -1) {
				field_width = 8;
				flags |= ZEROPAD;
			}
			str = number(str,
				(unsigned long) va_arg(arg, void *), 16,
				field_width, precision, flags);
			break;

		case 'x':
		   itoa (buf, c, *((int *) arg++));
                   p = buf;
                   goto string;
                   break;
		case 'X':
			str = number(str, va_arg(arg, unsigned long), 16,
				field_width, precision, flags);
			break;

		case 'd':
		case 'u':
		case 'i':
	           itoa (buf, c, *((int *) arg++));
                   p = buf;
                   goto string;
                   break;
		case 'n':
			ip = va_arg(arg, int *);
			*ip = (str - buf);
			break;
                string:
                   while (*p)
                     putchar (*p++);
                   break;

		default:
                   putchar (*((int *) arg++));
                   break;
			}
		}
	}
}

In my opinion, this might be solved by putting a

Code: Select all

if (c = '\n')
putchar('\n');
else ...

somewhere in the printf function code... The greatest problem I have is i just dont know where... I've been over this for almost a day... So, I guess what I'm tring to say is: Can someone help me, please??

[Combuster]

ouch.

Bad use of varargs
goto's scattered all around
poorly structured code.

Start with rewriting it according good coding practices, that'll likely save you headaches later (and most likely, fix the bug).

[Solar]

I second Combuster's remark - very poor code.

However:

Code: Select all

   if (c != '%')
      putchar(c);
           else
             {
               char *p;
     
               c = *format++;

   /* process flags */
      flags = 0;
      repeat:
         ++format;      /* this also skips first '%' */

Note how format is incremented twice...

[Alboin]

My God. That code...

I don't see a reason to have a standards compliant printf inside the kernel itself. That's for an external library. I'd just implement what'ya need as you go. A simplistic version would be:

Code: Select all

void write(char *frm, ...) {
	__builtin_va_list list;
	
	__builtin_va_start(list, frm);
	
	for(; *frm != '\0'; frm++) {
		if(*frm == '%') {
			frm++;
			
			if(*frm == 'i')
				write_int(__builtin_va_arg(list, int));
			else if(*frm == 's')
				write_str(__builtin_va_arg(list, char *));
			else if(*frm == '%')
				write_char('%');
		}
		else if(*frm == '\n')
			newline();
		else
			write_char(*frm);
	}
}

You see how things have been spread across various functions? Small, delicate, wonderful, happy functions all working in tandem. No more than a few variables per each, and each fits on a single screen. Each function does one thing well.