memory allocation

hi all,
i have written a simple prog tht reads user's input as string and allocates memory as and when a char is read. can anyone tell me why re-alloc fails when i execute thi below code. thanks in advance.

Code:

#include <ctype.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

void readStr(char **str1);
void printStr(char **str1);
void freeStr(char **str1);

int main(void)
{
	char *str1 = NULL;

	printf("Enter a string: ");
	str1 = (char *)malloc(sizeof(char));
	if(str1 == NULL)
	{
		printf("Malloc failed for str1!\n");
		return 1;
	}
	readStr(&str1);
	printStr(&str1);
	freeStr(&str1);

	return 0;
}

void readStr(char **str1)
{
	char ch;
	int i = 0;

	while( (ch = getchar()) != '\n' )
	{
		*str1[i++] = ch;
		if( (str1 = (char **)realloc(str1, (i+1) * sizeof(char))) == NULL )
		{
			printf("Re-alloc failed for str1!\n");
			exit(0);
		}
	}
	*str1[i] = '\0';
	return;
}

void printStr(char **str1)
{
	printf("String2 is %s\n", **str1);
	return;
}

void freeStr(char **str1)
{
	free(str1);
	return;
}

There are numerous errors here, but I think the one you are talking about is located:

Code:

char *str1 = NULL;
printf("Enter a string: ");
str1 = (char *)malloc(sizeof(char));
if(str1 == NULL)

First of all, you do not need to typecast calls to malloc. Secondly, malloc takes for an argument then number of bites you wish to malloc. sizeof(char) returns 1 as it's value. so str1 is 1 character long. Not many strings can be help in 1 byte. It's usually a good idea to read in a value directoly after you prompt a user for it too. Atleast in a program like this.

Edit starts here:
Ok, what I said before still stands, but after reviewing your code somemore I realized just how icky it was. First of all, if you are just going to read in 1 charater at a time, why even bother mallocing anythign at all in main, just do it all in your read function and return a char* with the string. Also, your method is rather inefficient. It takes cycles to realloc every character entered. You are better off by just mallocing one large portion, read in, and then if you need more, reallocing antoher large chunk.

Also, realloc does not return a **, it returns a *, and again you don't need to typecast.

Typecasting malloc isn't really that bad. Many old compilers need this, but then again we are talking real old compilers. Nevertheless, in C++ you usually will need to do a typecast. The bottom line is that it doesn't actually hurt anything, yet everyone complains about doing it.

In C++ you have 'new'.

Typecasting is not evil, but unneeded. And even though this is not completely true, I feel it obfuscates a little bit.

>> I feel it obfuscates a little bit.

Me too, but I just can't seem to keep myself from doing it. I think I'm addicted. And yes C++ has new, but when using malloc() a typecast is necessary.

thanks salem.. i understood abt realloc & how not to lose the memory i have already allocated if realloc fails. but can u clarify just 1 thing...... when i say *str[i] am i saying goto the index & access its value which in my case will be str itself as comapred to (*str)[i] which means the index of memory tht str holds? does this make sense? or am i confused..? sorry but can u please explain......