Reputation: 2628
I have written a small program that is going to become a text editor in the console for windows. This is what I have written right now:
#include <Windows.h>
#include <conio.h>
#include <stdlib.h>
int main()
{
int space = 20;
int used = 0;
int key = 0;
char* text = malloc(sizeof(char) * space);
while (key != 27)
{
if (used >= space)
{
space += 20;
text = realloc(text, space);
}
key = getch();
if (key == 8)
{
used--;
system("cls");
for (int i = 0; i < used; i++)
{
putch(text[i]);
}
}
else
{
used++;
text[used] = key;
putch(text[used]);
}
}
free(text);
return 0;
}
The printing when I press on letters at the keyboard works fine. The problem is when I hit backspace and try to remove a character. It starts printing random 'I' characters in the text I have written. What mistake am I making and how can I solve it?
Upvotes: 0
Views: 258
Reputation: 16540
the following code still has the 'off-by-one' error,
the following code needs the error checking added for the calls to malloc() and realloc().
however, it is portable, compiles cleanly, and performs the desired functionality without outputting trash characters.
However, it still has the logic problem with newlines.
//#include <Windows.h>
//#include <conio.h>
#include <stdio.h>
#include <stdlib.h>
int main( void )
{
int space = 20;
int used = 0;
int key = 0;
char* text = malloc(sizeof(char) * space);
while (key != 27) // escape
{
if (used >= space)
{
space += 20;
text = realloc(text, space);
}
key = getchar();
if (key == 8) // backspace
{
used--;
system("cls");
for (int i = 0; i < used; i++)
{
putc(text[i], stdout);
}
}
else
{ // all other key strokes
used++;
text[used] = key;
putc(text[used], stdout);
}
} // end while
free(text);
return 0;
} // end function: main
Upvotes: 1
Reputation: 151
I'm not sure that this is the only problem, but one problem I can see in first look - the increase of 'used' is before the inserting (means - you start inserting to text[1]) and the reprintig starts from zero (means - you start printing with text[0]) the first value is fertilized.
Sorry for the bad english and hope It's help.
Upvotes: 1
Reputation: 123578
Off by one error. Change
used++;
text[used] = key;
to
text[used] = key;
...
used++;
Some style notes:
First, use character literals instead of raw numeric codes; for example, instead of writing
if (key == 8)
use
if (key == '\b')
Where there isn't a predefined character literal (such as for the escape character) create a symbolic constant and use that:
#define ESC 27
...
while (key != ESC)
This will make your code a bit easier to understand for others.
Secondly, be careful with realloc
; if it can't satisfy the request, it will return NULL. If you assign that NULL value to your text
pointer, you will lose your only reference to the memory you had already allocated. It's better to do something like the following:
char *tmp = realloc( text, space + 20 );
if ( tmp )
{
text = tmp;
space += 20;
}
This way, if the realloc
fails, you still have your reference to the previously allocated memory, allowing you to free
it cleanly, or to recover from the error somehow.
Upvotes: 4
Reputation: 18381
text[0]
is never assigned. First time text[used]
is written the used
is already 1
, because it is getting incremented before. But on backspace
you are printing text
starting from 0
, which is uninitialized and containing garbage.
Upvotes: 1