Printing hex numbers

[Pyrofan1]

so i'm trying to display hex numbers in my os, but it isn't working
here is a screenshot

and the relevant code
printf

Code: Select all

void Printf(char *fmt,...)
{
	va_list args;
	char number[10];
	int num;
	char buf;
	int buffer;
	int buffer2;
	
	va_start(args,fmt);

	while(*fmt!=NULL)
	{
		if(*fmt=='%')
		{
			fmt++;
			switch(*fmt)
			{
				case 'i':
				case 'd':
				case 'u':
				itoa(va_arg(args,int),number);
				Put(number);
				break;

				case 'c':
				putchar((unsigned char)va_arg(args,int));
				break;

				case 'x':
				buffer=va_arg(args,int);
				buffer2=buffer;
				buffer=buffer<<28;
				buffer=buffer>>28;
				putchar(return_hex(buffer));

				buffer=buffer2;
				buffer=buffer<<24;
				buffer=buffer>>28;
				putchar(return_hex(buffer));

				buffer=buffer2;
				buffer=buffer<<20;
				buffer=buffer>>24;
				putchar(return_hex(buffer));

				buffer=buffer2;
				buffer=buffer<<16;
				buffer=buffer>>20;
				putchar(return_hex(buffer));

				buffer=buffer2;
				buffer=buffer<<12;
				buffer=buffer>>16;
				putchar(return_hex(buffer));

				buffer=buffer2;
				buffer=buffer<<8;
				buffer=buffer>>12;
				putchar(return_hex(buffer));

				buffer=buffer2;
				buffer=buffer<<4;
				buffer=buffer>>8;
				putchar(return_hex(buffer));

				buffer=buffer2;
				buffer=buffer|0x0F;
				putchar(return_hex(buffer));
				break;

				case 's':
				Put(va_arg(args,char*));
				break;	
				
				default:
				putchar(*fmt);
				break;
			}
			fmt++;
		}
		else
		{
			putchar(*fmt);
			fmt++;
		}
	}
	va_end(args);
}

and my return_hex function

Code: Select all

char return_hex(unsigned short num)
{
	switch(num)
	{
		case 0x00:
		return '0';
		break;

		case 0x01:
		return '1';
		break;

		case 0x02:
		return '2';
		break;

		case 0x03:
		return '3';
		break;

		case 0x04:
		return '4';
		break;

		case 0x05:
		return '5';
		break;

		case 0x06:
		return '6';
		break;

		case 0x07:
		return '7';
		break;

		case 0x08:
		return '8';
		break;

		case 0x09:
		return '9';
		break;

		case 0x0A:
		return 'A';
		break;

		case 0x0B:
		return 'B';
		break;

		case 0x0C:
		return 'C';
		break;

		case 0x0D:
		return 'D';
		break;

		case 0x0E:
		return 'E';
		break;

		case 0x0F:
		return 'F';
		break;

		default:
		break;
	}
}

and my main

Code: Select all

void _main(struct MULTIBOOT *mbd,unsigned int magic)
{
	char *code;
	int c=0xAF;

	clrscr(BLACK,BLACK);
	set_color(WHITE,BLACK);
	set_echo(1);

	if((mbd->flags&0x08)==0x08)
	{
		Printf("Modules were loaded succesfully\n");
		Printf("Number of modules loaded: %d\n",mbd->mods_count);
		code=(char*)mbd->mods_addr;
		
		
	}
	else
	{
		Printf("Modules were not succesfully loaded\n");
	}

	idt_install();
	gdt_install();

	Printf("Welcome to PyroS\n");
	Printf("c is 0x%x",c);
	for(;;);
}

[cg123]

I'm not sure what the problem with that code is, as I really don't feel like deciphering it at this hour.. but I can tell you that there's a much better approach you can take for converting numbers into an arbitrary base. Here's some pseudocode:

Code: Select all

char* int_to_string(int num, int base) {
	char* buffer = allocation_function_or_maybe_static_buffer_i_dont_know();
	int idx = 0;
	while (num != 0) {
		int digit = num % base;
		char c = (digit <= 9) ? '0'+digit : 'A'+digit-9;
		buffer[idx++] = c;
		num /= base;
	}
	magical_buffer_reversing_function(buffer);
	return buffer;
}

Here's an actual implementation from my kernel, with support for unsigned and signed numbers:

Code: Select all

char tbuf[32];
char bchars[] = {'0','1','2','3','4','5','6','7','8','9','A','B','C','D','E','F'};

void itoa_s(int i,u8 base,char* buf) {
	if (base > 16) return;
	if (i < 0) {
		*buf++ = '-';
		i *= -1;
	}
	itoa((u64)i,base,buf);
}

void itoa(u64 i,u8 base,char* buf) {
	int pos = 0;
	int opos = 0;
	int top = 0;

	if (i == 0 || base > 16) {
		buf[0] = '0';
		buf[1] = '\0';
		return;
	}

	while (i != 0) {
		tbuf[pos] = bchars[i % base];
		pos++;
		i /= base;
	}
	top=pos--;
	for (opos=0; opos<top; pos--,opos++) {
		buf[opos] = tbuf[pos];
	}
	buf[opos] = 0;
}

[XCHG]

For 32-bit Hexadecimal integral values, you can do these:


1) Set a counter to 8 (32bits / 4bits = 8 nibbles to print)
2) Rotate the number to the left 4 bits.
3) Get the leftmost 4 bits that are rotated to the rightmost bits of the value.
4) See if the 4 rightmost bits are bigger than 10 (0x0A)
5) If they aren't, jump to step 7.
6) Add 0x07 to the value. Don't skip step 7. Fall through.
7) Add 0x30 to the value.
8) Print the value (that is a character now, after our additions).
9) Decrement Counter once.
10) Jump to step #2 if counter > 0


There you go. For example, this is a procedure that I wrote yesterday that prints a 64-bit Integral value to the screen as a hexadecimal value including the "0x" prefix:

Code: Select all

; ——————————————————————————————————————————————————
  __WriteQWORDAsHexExact:
    ; void __WriteQWORDAsHexExact (DWORD HighDWORD, DWORD LowDWORD); StdCall;
    PUSH    EAX
    PUSH    EBX
    PUSH    ECX
    PUSH    EDX
    PUSH    ESI
    PUSH    EDI
    PUSH    EBP
    MOV     EBP , ESP
    ; [EBP + 0x20] = HighDWORD 
    ; [EBP + 0x24] = LowDWORD
    
    INVOKE  __WriteChar , '0'
    INVOKE  __WriteChar , 'x'
    MOV     ESI , DWORD PTR [EBP + 0x24]
    MOV     EDI , DWORD PTR [EBP + 0x20]
    ; EDI:ESI holds the 64-bit QWORD value
    MOV     ECX , 8*2
    
    .Loop:
      MOV     EAX , EDI
      SHR     EAX , 28
      SHL     EDI , 0x00000004
      MOV     EBX , ESI
      SHL     ESI , 0x00000004
      SHR     EBX , 28
      OR      EDI , EBX
      ; EAX contains the hex digit to be printed
      CMP     EAX , 0x0000000A
      JB      .ItIsaDigit
      .ItIsaLetter:
        ADD     EAX , 0x00000007
      .ItIsaDigit:
        ADD     EAX , 0x00000030
      .PrintTheCharacter:
        INVOKE  __WriteChar , EAX
      
      DEC     ECX
      JNZ     .Loop

    .EP:
      POP     EBP
      POP     EDI
      POP     ESI
      POP     EDX
      POP     ECX
      POP     EBX
      POP     EAX
    RET     0x08
    
   
; ——————————————————————————————————————————————————

And this is the one that I have written for 32-bit values to be printed as hexadecimals:

Code: Select all

; ——————————————————————————————————————————————————
  __WriteDWORDAsHex:
    ; void __WriteDWORDAsHex (DWORD InDWORD); StdCall;
    PUSH    EAX
    PUSH    ECX
    PUSH    EDX
    PUSH    EBP
    MOV     EBP , ESP
    MOV     EAX , DWORD PTR [EBP + 0x14]
    INVOKE  __WriteChar, '0'
    INVOKE  __WriteChar, 'x'
    MOV     ECX , 0x08
    .Loop:
      ROL     EAX , 0x04
      MOV     EDX , EAX
      AND     EDX , 0x0000000F
      OR      EDX , 0x00000030
      CMP     EDX , 0x0000003A
      JB      .Rest
      ADD     EDX , 0x00000007
      .Rest:
        INVOKE  __WriteChar, EDX
        DEC     ECX
        JNZ     .Loop
    POP     EBP
    POP     EDX
    POP     ECX
    POP     EAX
    RET     0x04
; ——————————————————————————————————————————————————

I hope that helps.

[JamesM]

Seeing as we're sharing implementations:

Code: Select all

//
// writeHex -- Outputs the given signed 32-bit integer to the monitor
//             as a hexidecimal number, prefixed with "0x", with the
//             foreground/background colours specified.
//
void MonitorDriver::writeHex(s32int n, Colour foreColour, Colour backColour)
{

  s32int tmp;

  write("0x", foreColour, backColour);

  bool noZeroes = true;

  int i;
  for (i = 28; i > 0; i -= 4)
  {
    tmp = (n >> i) & 0xF;
    if (tmp == 0 && noZeroes)
    {
      continue;
    }
    
    if (tmp >= 0xA)
    {
      noZeroes = false;
      put (tmp-0xA+'a', foreColour, backColour);
    }
    else
    {
      noZeroes = false;
      put( tmp+'0', foreColour, backColour);
    }
  }
  
  tmp = n & 0xF;
  if (tmp >= 0xA)
  {
    put (tmp-0xA+'a', foreColour, backColour);
  }
  else
  {
    put( tmp+'0', foreColour, backColour);
  }
  
}

The code is slightly longer than it could be because I wanted to remove any leading zeroes.

[jal]

so i'm trying to display hex numbers in my os, but it isn't working

Code: Select all

				case 'x':
				buffer=va_arg(args,int);
				buffer2=buffer;
				buffer=buffer<<28;
				buffer=buffer>>28;
				putchar(return_hex(buffer));

				buffer=buffer2;
				buffer=buffer<<24;
				buffer=buffer>>28;
				putchar(return_hex(buffer));

	switch(num)
	{
		case 0x00:
		return '0';
		break;

		case 0x01:
		return '1';
		break;

Man, what a total crap, that code. Enough to make a grown man cry... Really, quit programming, let alone make an OS. Jeez, if you can't even make a decent hex printing function...

EDIT: The above statement may be formulated a little harsh. I hearby apologize for any cause of inconvenience to the OP. That said, I will reformulate with well-meant advise:

"Dear OP, the source code you posted leads me to believe you are not a very experienced programmer. To create an OS, you must be a very experienced programmer, or you will get stuck immediately. In fact, you did. Besides some programming skills, learn some debugging skills. The problem you are having with your printing routine can easily be debugged in e.g. Visual Studio, as it does not concern the OS itself, merily a support routine. In general, it is a good idea to test such functions under Windows or Linux, especially when you lack bugtracking skills brought on by years of programming experience."


JAL

[JamesM]

jal: That was extremely harsh and not a little unfair. The solution is simple when thought about in a specific way, and it is more than likely the OP hasn't been to college or university and been taught the best ways of tackling such a problem (It's more mathematically based than most).

An apology is on order IMHO.

Although you are right, that code is enough to make my eyes bleed!

[jal]

jal: That was extremely harsh (...) An apology is on order IMHO.

You are right, Monday mornings are not the right time to write replies.

Although you are right, that code is enough to make my eyes bleed!

Indeed, but one can say that a little nicer than I did. Hope the edit above is better.


JAL

[JamesM]

It takes a man to apologise, well said

[jal]

so i'm trying to display hex numbers in my os

Ok, to compensate for being a bit harsh, I'll try to explain some basic programming tricks, as well as the bugs you made.

Code: Select all

	case 'x':
	buffer=va_arg(args,int);
	buffer2=buffer;
	buffer=buffer<<28;
	buffer=buffer>>28;
	putchar(return_hex(buffer));

Ok, take a good look here. The first thing to print is the highest nibble (i.e. 4 bits), so you want to feed that to your return_hex() function. The highest nibble is located in bits 28-31. return_hex() wants them in the lower 4 bits however, so you need to shift right 28 positions. That'll make bit 28 be bit 0, bit 29 bit 1, bit 30 bit 2 and bit 31 bit 3. However, for some reason that totally eludes me, you are shifting first 28 bits to the left, then do the right shift. This has the effect of removing all bits bit the lower 4. So you are printing first the lower four bits, which are the least significant. So this is bug #1 (not really related to the fact you get garbage on the screen though).

Tips 'n' tricks:
1) You have a integere variable named 'buffer'. This is a misnomer. 'buffer' should be reserved for (typically character) arrays that buffer something. You'd better call it 'value' or the like.
2) You make a copy of the buffer variable. This is not needed, as the shifting operations you perform can be performed in the function call, e.g. putchar (return_hex ((buffer << 28) >> 28)).
3) Although << and >> gets you where you (think you) want to be, use bitmasking instead. "(value & 0x0f)" reads a lot better than "(value << 28) >> 28)", and can also be used for variables that aren't 32 bits.

Code: Select all

	buffer=buffer2;
	buffer=buffer<<24;
	buffer=buffer>>28;
	putchar(return_hex(buffer));

This works for getting the second least significant nibble. Like stated above, you are printing in reverse.

Code: Select all

	buffer=buffer2;
	buffer=buffer<<20;
	buffer=buffer>>24;
	putchar(return_hex(buffer));

Now you introduce another bug. Shifting 20 positions to the left leaves you 12 bits (32-20). Then, shifting 24 positions back, you're not left with a nibble, but with a byte, since you are shifting 4 more right than left (so 12 - 4 remain).

Code: Select all

	buffer=buffer2;
	buffer=buffer<<16;
	buffer=buffer>>20;
	putchar(return_hex(buffer));

Same bug, now even worse. But shifting 16 positions you leave 16 bits, shifting 20 leaves you 12 bits remaining as input to return_hex(). You repeat that for the other digits (except the last one).

Code: Select all

	buffer=buffer2;
	buffer=buffer|0x0F;
	putchar(return_hex(buffer));

Here, you OR the buffer with 0x0f. You no doubt wanted to AND it, but you didn't. So now you feed the original number with all lower four nibbles set to 1 to return_hex().

and my return_hex function

Code: Select all

	case 0x00:
	return '0';
	break;

	case 0x01:
	return '1';
	break;

Tips 'n' tricks:
Yes, this works. However, since the numerals in the ASCII set are consecutive, you could also do an add like this: return num + '0'. Only for those nums in range 0-9 of course. Similar for the hex numbers: return (num - 10) + 'A'. An alternative is a simple array, e.g. char nums[] = "0123456789ABCDEF", then return nums[num].

Code: Select all

	default:
	break;

You probably didn't expect to get values that are outside the range of 0x00-0x0f, but as I explained above, you do.

Tips 'n' tricks:
1) Never, ever have a code path in a function that can result in no (or rather, an uninitialized) return value being returned.
2) Have your compiler generate warnings at a warning level that at least includes these cases, or if it already does: read the warnings! It's probably stuff you want to know!

Code: Select all

	char *code;
	int c=0xAF;
...
	Printf("c is 0x%x",c);

I must honestly say that at first I was a bit of a loss why you get the output you get. Although your code is severely buggy as explained above, I would've expect it to say "FA??????" (? being any random character), not "??AAAAAA". So what I did is compile and debug the code (something you should've before asking for our help, really), and I found another bug. In the PrintF function, 'buffer' is a signed int. When shifting signed ints, most (all?) compilers will do an arithmetic shift, i.e. they copy the sign bit as well. So when shifting 0xAF 28 to the left, you get 0xF0000000, when shifting that back 28 positions, you get 0xFFFFFFFF (-1). That is fed to the return_hex() function, which, as I explained, returns something semi-random.

One more tips 'n' tricks:
You have function names with camel case ("PrintF") and underscore style ("return_hex"). It is generally considerd ugly to have both styles withing one program.


JAL

[jal]

It takes a man to apologise, well said :)

Thanks. I even felt a little bit guilty, so I actually made an analysis of the code. Hope my karma points are level now :).


JAL

[blound]

2) Rotate the number to the left 4 bits.
3) Get the rightmost 4 bits that are rotated to the rightmost bits of the value.
4) See if the 4 rightmost bits are bigger than 10 (0x0A)

Maybe I am reading this wrong.. but I think 3) and 4) should be "get the leftmost 4 bits that are rotated" and should switch right with left in the two other places.. either way I implemented your steps (switching right with left in the places mentioned ) in C for anyone interested.

Code: Select all

void printhex(int i){

        puts("0x");

        uint counter = 8;
        uchar cur;

        while(counter-- > 0){
                cur = (i & 0xf0000000) >> 28;

                i <<= 4;

                if (cur >= 0xA)
                        cur += 0x07;

                cur += 0x30;

                putch(cur);
        }
}

[Combuster]

every bit you rotate out on the left reappears on the right. rotating left 4 bits cause the top 4 bits to appear in the rightmost 4 bits.

[JamesM]

Alas, rotation isn't possible in C without inline assembler. His solution looks good.

[bewing]

The fast way, in assembler, with a lookup table:

Code: Select all

; display half a long, in hex -- designed to be called twice
; inputs: ecx = long to print, ah = attrib value, edi -> video buffer
; destroys: al (-- after 2 long calls, ecx is unchanged)
dhex_lng:
	ror ecx, 16			; swap the high & low words of ecx
dhex_s:
	push edx
	mov edx, ecx
	movzx ecx, ch
	call dohdig			; convert to 2 hex digits + 2 attrib bytes
	movzx ecx, dl
	call dohdig
	mov ecx, edx
	pop edx
	ret


; dohdig copies 2 hex digits plus attribute bytes into edi
; inputs: ecx = zero extended byte, ah = attrib value, edi -> video buffer
; destroys: al
dohdig:
	push edx
	mov dx, [hex_lkup + ecx*2]		; get 2 bytes from lookup table
	mov al, dl				; get 1st (high digit) ready to store
	; Note: the high digit is supposed to be written 1st, so it ends up in dl
	stosw
	mov al, dh				; get 2nd ready to store
	stosw					; attrib was still in ah
	pop edx
	ret

hex_lkup looks like:
hex_lkup: dw 0x3030 ; ascii hex chars, byteswapped
dw 0x3130
...

[cyr1x]

I do it that way (C++):

Code: Select all

void VideoManager::PrintHex(u32int num)
{
	char buffer[11];
	buffer[0] = '0';
	buffer[1] = 'x';
	for (int i = 7; i >= 0; i--)
	{
		buffer[i+2] = static_cast<char>(num % 16 < 10 ? '0' + num % 16 : 'A' - 10 + num % 16);
		num /= 16;
	}
	buffer[10]= '\0';         /* terminate string */

	PrintString(buffer);     /* print it */
}