problem with my gets function !

Question about which tools to use, bugs, the best way to implement a function, etc should go here. Don't forget to see if your question is answered in the wiki first! When in doubt post here.
mohammed
Member
Member
Posts: 93
Joined: Mon Jan 30, 2006 12:00 am
Location: Mansoura, Egypt

problem with my gets function !

Post by mohammed »

Code: Select all

gets(unsigned char *string)
{
int strcounter = 0;
while (1)
{
scancode = inportb(0x60);
if(scancode != '\n')
{
putch(kbdus[scancode]);
string[strcounter] =scancode;
strcounter++;

}
else
return;
}
}
every time i press a key it full the whole screen with the letter why ??
OrOS
Member
Member
Posts: 143
Joined: Sat Sep 08, 2007 11:26 pm
Location: Canada

Post by OrOS »

Can we see your putch function?
mohammed
Member
Member
Posts: 93
Joined: Mon Jan 30, 2006 12:00 am
Location: Mansoura, Egypt

Post by mohammed »

for sure but it'snot mine i mean i didn't write it i only wrote the part that handle the backspace i mean to clear the letter screen after moving the cursor back !

Code: Select all

void putch(unsigned char c)
{
    unsigned short *where;
    unsigned att = attrib << 8;

    /* Handle a backspace, by moving the cursor back one space */
    if(c == 0x08)
    {
        if(csr_x != 0) 
            {
        where = textmemptr + (csr_y * 80 + csr_x-1);
        *where = 0x20 | att;
         csr_x--;
           }
    }
    /* Handles a tab by incrementing the cursor's x, but only
    *  to a point that will make it divisible by 8 */
    else if(c == 0x09)
    {
        csr_x = (csr_x + 8) & ~(8 - 1);
    }
    /* Handles a 'Carriage Return', which simply brings the
    *  cursor back to the margin */
    else if(c == '\r')
    {
        csr_x = 0;
    }
    /* We handle our newlines the way DOS and the BIOS do: we
    *  treat it as if a 'CR' was also there, so we bring the
    *  cursor to the margin and we increment the 'y' value */
    else if(c == '\n')
    {
        csr_x = 0;
        csr_y++;
    }
    /* Any character greater than and including a space, is a
    *  printable character. The equation for finding the index
    *  in a linear chunk of memory can be represented by:
    *  Index = [(y * width) + x] */
    else if(c >= ' ')
    {
        where = textmemptr + (csr_y * 80 + csr_x);
        *where = c | att;	/* Character AND attributes: color */
        csr_x++;
    }

    /* If the cursor has reached the edge of the screen's width, we
    *  insert a new line in there */
    if(csr_x >= 80)
    {
        csr_x = 0;
        csr_y++;
    }
User avatar
JamesM
Member
Member
Posts: 2935
Joined: Tue Jul 10, 2007 5:27 am
Location: York, United Kingdom
Contact:

Post by JamesM »

Code: Select all

gets(unsigned char *string) 
{ 
int strcounter = 0; 
while (1) 
{ 
scancode = inportb(0x60); 
if(scancode != '\n') 
{ 
putch(kbdus[scancode]); 
string[strcounter] =scancode; 
strcounter++; 

} 
else 
return; 
} 
}
You wonder why it fills the whole screen? Look at your code and tell me under what conditions does the function exit. You'll find that there's only one way that function can exit - if inportb(0x60) returns '\n'. That will never happen, as inportb(0x60) returns you a scancode - you decode the scancode in your putch(...) line, but not anywhere else.

Firstly, move the kbdus[scancode] line up out of the if() test, so you get

Code: Select all

scancode = kbdus[inportb(0x60)];
Then, you should know that inportb() will not hang until input is ready. You need to spin until there is data available.

Code: Select all

while (! (inportb(0x64) & 0x1) );
So, your code becomes:

Code: Select all

gets(unsigned char *string) 
{ 
  int strcounter = 0; 
  while (1) 
  { 
    while (! (inportb(0x64) & 0x1) )
      ;
    scancode = kbdus[inportb(0x60)]; 
    if(scancode != '\n') 
    { 
      putch(scancode); 
      string[strcounter] =scancode; 
      strcounter++; 
    }
    else 
      return; 
  } 
}
JamesM
mohammed
Member
Member
Posts: 93
Joined: Mon Jan 30, 2006 12:00 am
Location: Mansoura, Egypt

Post by mohammed »

thank youuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuu
it worked ;)

Code: Select all

while (! (inportb(0x64) & 0x1) ) ;
then this is a loop do nothing untill i press a key again, right??
User avatar
JamesM
Member
Member
Posts: 2935
Joined: Tue Jul 10, 2007 5:27 am
Location: York, United Kingdom
Contact:

Post by JamesM »

Yes, that is what the loop does. If you look at the keyboard page on the wiki, it will tell you all.

inportb(0x64) is reading the status byte of the keyboard controller. Bit 0 (mask 0x1) in that byte is "data present". You're spinning until that becomes 1.
mohammed
Member
Member
Posts: 93
Joined: Mon Jan 30, 2006 12:00 am
Location: Mansoura, Egypt

Post by mohammed »

thank you it's really useful
but i have another problem with this

Code: Select all

char *medo;
puts("type something:");
gets(medo);
puts("\n");
puts(medo);
the problem is that it typed only two characters why ?!!!!!
do i have to change something with puts function ?

Code: Select all

void puts(unsigned char *text)
{
    int i;

    for (i = 0; i < strlen(text); i++)
    {
        putch(text[i]);
    }
}

Code: Select all

size_t strlen(const char *str)
{
    size_t retval;
    for(retval = 0; *str != '\0'; str++) retval++;
    return retval;
}
Last edited by mohammed on Mon Sep 24, 2007 9:29 pm, edited 1 time in total.
thewonderidiot
Posts: 19
Joined: Sat May 05, 2007 1:27 pm

Post by thewonderidiot »

The only thing that I can think of looking at your code is that you pulled a character pointer out of thin air. It would probably be best to create an actual character array and make sure you don't overflow its bounds by passing a size limiter argument to your gets(); for example:

Code: Select all

char medo[256];
puts("type something:");
gets(medo,256);
puts("\n");
puts(medo);
Without actually proclaiming the space after that pointer as yours, it could belong to anything. My guess is that something in your OS wrote a null over the third character of your string, causing your puts function to think the string had ended.

If that doesn't work, you could write a simple function to dump the hex of the first 20 or so characters after the memory location of medo, to see exactly what's been written to memory in that area...
pcmattman
Member
Member
Posts: 2566
Joined: Sun Jan 14, 2007 9:15 pm
Libera.chat IRC: miselin
Location: Sydney, Australia (I come from a land down under!)
Contact:

Post by pcmattman »

@thewonderidiot: I was about to say that, but you beat me to it...

Another thing is, it may not work because something may not be working properly in your gets() function. I'd suggest a lot of puts( "..." ); calls to find out what's happening in your gets function.
mohammed
Member
Member
Posts: 93
Joined: Mon Jan 30, 2006 12:00 am
Location: Mansoura, Egypt

Post by mohammed »

i don't know if there is something wrong with gets
i tried that

Code: Select all

gets(unsigned char *string)
{
  int strcounter = 0;
  while (1)
  {
     while (! (inportb(0x64) & 0x1) );
    scancode = kbdus[inportb(0x60)];
    if(scancode != '\n'&&)
    {
       putch(scancode);
      *string=scancode;
     *string++;
    
    }
    else
      *string++;
      *string = '\0';
      return *string;
  }
}
do you know what happened ??
accept only one letter then print it : ( (
pcmattman
Member
Member
Posts: 2566
Joined: Sun Jan 14, 2007 9:15 pm
Libera.chat IRC: miselin
Location: Sydney, Australia (I come from a land down under!)
Contact:

Post by pcmattman »

It should be:

Code: Select all

gets( unsigned char* string )
{
	while( 1 )
	{
		while( ! (inportb(0x64) & 0x1) );
		unsigned char key = kbdus[ inportb( 0x60 ) ];
		if( key != '\n' )
		{
			putch( key );
			*string++ = key;
		}
		else
		{
			*string++ = 0; // null char at end of string
			return;
		}
	}
}
Try that and tell us what happens.
mohammed
Member
Member
Posts: 93
Joined: Mon Jan 30, 2006 12:00 am
Location: Mansoura, Egypt

Post by mohammed »

i made the same but i preferred switch case cause i think it separates conditions in an organized matter

Code: Select all

gets(unsigned char *string)
{
  int strcounter = 0;
  while (1)
  {
while (! (inportb(0x64) & 0x1) );
 scancode = kbdus[inportb(0x60)];
switch(scancode)
 {
case '\n':
*string++ = 0;
return *string;
break;
default:
 putch(scancode);
 *string ++ = scancode;

break;    
}
         }
}
do you know what was the result ?? printing random number of the string sometimes 2 sometimes 3 hahahahahah
here the file of kb.c may be something wrong on it :(

Code: Select all

/* bkerndev - Bran's Kernel Development Tutorial
*  By:   Brandon F. ([email protected])
*  Desc: Keyboard driver
*
*  Notes: No warranty expressed or implied. Use at own risk. */
#include <system.h>

/* KBDUS means US Keyboard Layout. This is a scancode table
*  used to layout a standard US keyboard. I have left some
*  comments in to give you an idea of what key is what, even
*  though I set it's array index to 0. You can change that to
*  whatever you want using a macro, if you wish! */
 unsigned char scancode;
unsigned char kbdus[128] =
{
    0,  27, '1', '2', '3', '4', '5', '6', '7', '8',	/* 9 */
  '9', '0', '-', '=', '\b',	/* Backspace */
  '\t',			/* Tab */
  'q', 'w', 'e', 'r',	/* 19 */
  't', 'y', 'u', 'i', 'o', 'p', '[', ']', '\n',		/* Enter key */
    0,			/* 29   - Control */
  'a', 's', 'd', 'f', 'g', 'h', 'j', 'k', 'l', ';',	/* 39 */
 '\'', '`',   0,		/* Left shift */
 '\\', 'z', 'x', 'c', 'v', 'b', 'n',			/* 49 */
  'm', ',', '.', '/',   0,					/* Right shift */
  '*',
    0,	/* Alt */
  ' ',	/* Space bar */
    0,	/* Caps lock */
    0,	/* 59 - F1 key ... > */
    0,   0,   0,   0,   0,   0,   0,   0,
    0,	/* < ... F10 */
    0,	/* 69 - Num lock*/
    0,	/* Scroll Lock */
    0,	/* Home key */
    0,	/* Up Arrow */
    0,	/* Page Up */
  '-',
    0,	/* Left Arrow */
    0,
    0,	/* Right Arrow */
  '+',
    0,	/* 79 - End key*/
    0,	/* Down Arrow */
    0,	/* Page Down */
    0,	/* Insert Key */
    0,	/* Delete Key */
    0,   0,   0,
    0,	/* F11 Key */
    0,	/* F12 Key */
    0,	/* All other keys are undefined */
};

/* Handles the keyboard interrupt */
void keyboard_handler(struct regs *r)
{
 

    /* Read from the keyboard's data buffer */
    scancode = inportb(0x60);

    /* If the top bit of the byte we read from the keyboard is
    *  set, that means that a key has just been released */
    if (scancode & 0x80)
    {
        /* You can use this one to see if the user released the
        *  shift, alt, or control keys... */
    }
    else
    {
        /* Here, a key was just pressed. Please note that if you
        *  hold a key down, you will get repeated key press
        *  interrupts. */

        /* Just to show you how this works, we simply translate
        *  the keyboard scancode into an ASCII value, and then
        *  display it to the screen. You can get creative and
        *  use some flags to see if a shift is pressed and use a
        *  different layout, or you can add another 128 entries
        *  to the above layout to correspond to 'shift' being
        *  held. If shift is held using the larger lookup table,
        *  you would add 128 to the scancode when you look for it */
        putch(kbdus[scancode]);
    }
}

/* Installs the keyboard handler into IRQ1 */
void keyboard_install()
{
    irq_install_handler(1, keyboard_handler);
}
gets(unsigned char *string)
{
  int strcounter = 0;
  while (1)
  {
while (! (inportb(0x64) & 0x1) );
 scancode = kbdus[inportb(0x60)];
switch(scancode)
 {
case '\n':
*string++ = 0;
return *string;
break;
default:
 putch(scancode);
 *string ++ = scancode;

break;    
}
         }
}
pcmattman
Member
Member
Posts: 2566
Joined: Sun Jan 14, 2007 9:15 pm
Libera.chat IRC: miselin
Location: Sydney, Australia (I come from a land down under!)
Contact:

Post by pcmattman »

I see the problem: your code will actually not have any keys to read because the IRQ firing will stop that.

Instead, try something like this:

Code: Select all

/* bkerndev - Bran's Kernel Development Tutorial
*  By:   Brandon F. ([email protected])
*  Desc: Keyboard driver
*
*  Notes: No warranty expressed or implied. Use at own risk. */
#include <system.h>

// -1 when no keys, >= 0 when keys
volatile char keybuff = -1;

/* KBDUS means US Keyboard Layout. This is a scancode table
*  used to layout a standard US keyboard. I have left some
*  comments in to give you an idea of what key is what, even
*  though I set it's array index to 0. You can change that to
*  whatever you want using a macro, if you wish! */
 unsigned char scancode;
unsigned char kbdus[128] =
{
    0,  27, '1', '2', '3', '4', '5', '6', '7', '8',   /* 9 */
  '9', '0', '-', '=', '\b',   /* Backspace */
  '\t',         /* Tab */
  'q', 'w', 'e', 'r',   /* 19 */
  't', 'y', 'u', 'i', 'o', 'p', '[', ']', '\n',      /* Enter key */
    0,         /* 29   - Control */
  'a', 's', 'd', 'f', 'g', 'h', 'j', 'k', 'l', ';',   /* 39 */
 '\'', '`',   0,      /* Left shift */
 '\\', 'z', 'x', 'c', 'v', 'b', 'n',         /* 49 */
  'm', ',', '.', '/',   0,               /* Right shift */
  '*',
    0,   /* Alt */
  ' ',   /* Space bar */
    0,   /* Caps lock */
    0,   /* 59 - F1 key ... > */
    0,   0,   0,   0,   0,   0,   0,   0,
    0,   /* < ... F10 */
    0,   /* 69 - Num lock*/
    0,   /* Scroll Lock */
    0,   /* Home key */
    0,   /* Up Arrow */
    0,   /* Page Up */
  '-',
    0,   /* Left Arrow */
    0,
    0,   /* Right Arrow */
  '+',
    0,   /* 79 - End key*/
    0,   /* Down Arrow */
    0,   /* Page Down */
    0,   /* Insert Key */
    0,   /* Delete Key */
    0,   0,   0,
    0,   /* F11 Key */
    0,   /* F12 Key */
    0,   /* All other keys are undefined */
};

/* Handles the keyboard interrupt */
void keyboard_handler(struct regs *r)
{
 

    /* Read from the keyboard's data buffer */
    scancode = inportb(0x60);

    /* If the top bit of the byte we read from the keyboard is
    *  set, that means that a key has just been released */
    if (scancode & 0x80)
    {
        /* You can use this one to see if the user released the
        *  shift, alt, or control keys... */
    }
    else
    {
        /* Here, a key was just pressed. Please note that if you
        *  hold a key down, you will get repeated key press
        *  interrupts. */

        /* Just to show you how this works, we simply translate
        *  the keyboard scancode into an ASCII value, and then
        *  display it to the screen. You can get creative and
        *  use some flags to see if a shift is pressed and use a
        *  different layout, or you can add another 128 entries
        *  to the above layout to correspond to 'shift' being
        *  held. If shift is held using the larger lookup table,
        *  you would add 128 to the scancode when you look for it */
//        putch(kbdus[scancode]);
	keybuff = kbdus[scancode];
    }
}

/* Installs the keyboard handler into IRQ1 */
void keyboard_install()
{
    irq_install_handler(1, keyboard_handler);
}

int gets( unsigned char* string )
{
	int n = 0;
	while( 1 )
	{
		while( keybuff == -1 );
		char tmp = keybuff; // in case another IRQ fires in the mean time
		if( tmp == '\n' )
		{
			*string++ = 0;
			return n;
		}
		else
		{
			putch( tmp );
			*string++ = tmp;
			n++;
		}
	}
	return -1; // something weird happened
}
It basically sets up a single-character buffer that pressed keys go into, gets just waits for that buffer to have keys in it then does the reading.
mohammed
Member
Member
Posts: 93
Joined: Mon Jan 30, 2006 12:00 am
Location: Mansoura, Egypt

Post by mohammed »

compiled without errors but didn't work at all -it didn't print the letters on the screen in gets function
i don't know if that's a hallucination but in my last function that used switch case when i type fastly it handle a lot of characters and when i type so slow it just print the first letters what it prints is depended on my speed freaky isn't it ?
User avatar
Solar
Member
Member
Posts: 7615
Joined: Thu Nov 16, 2006 12:01 pm
Location: Germany
Contact:

Post by Solar »

Not if you have a runaway algorithm that dies after a certain number of iterations.

However, you've been such a pain in the backside in the last few days that I don't feel inclined to look at your problem.
Every good solution is obvious once you've found it.
Post Reply