Segmentation fault

[TheBeast32]

Hi, I'm making a little program that will send email. I am fine with the sockets, but I keep getting a segmentation fault and I don't know why:$. I'm trying to make a loop that will let the user enter the data for the email, ending with a '.' on its own line (like when using an smtp server). I'm on windows using cygwin.

char *data, ch;
	char newline;
	unsigned int datalen;
	
	data = (char*)calloc(1, sizeof(char));
	newline = 1;
	datalen = 0;
	
	while (1)
	{
		ch = fgetc(stdin);
		realloc(data, ++datalen * sizeof(char));
		*(data + datalen - 1) = ch;
		
		if (newline)
		{
			if (ch == '.')
			{
				realloc(data, ++datalen * sizeof(char));
				*(data + datalen - 1) = '\n';
				break;
			}
			else if (ch != '\n')
			{
				newline = 0;
			}
		}
		else
		{
			if (ch == '\n')
			{
				newline = 1;
			}
		}
	}

Here's my output using gdb:
GNU gdb 5.2.1
Copyright 2002 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB. Type "show warranty" for details.
This GDB was configured as "i686-pc-mingw32"...(no debugging symbols found)...
(gdb) run
Starting program: g:\C\MailClient/./mail.exe

Program received signal SIGSEGV, Segmentation fault.
0x7c80a6c2 in _cygheap_end1 ()
(gdb)

I get the segmentation fault after I enter my 13th character.

[wildgoose]

Why are you mixing calloc's with realloc's.

Primarily Realloc() does not always stretch the same memory!!!! It returns the pointer to the new (or stretched) memory. In your case it has stretched it as much as it could around character 13 the reallocated a new block of memory, copied your data then destroyed your memory. You never reset your pointer by...

data = realloc( data, +datalen *sizeof(char) );

...so you accessed memory that nolonger existed!

Why are you chewing on memory one character at a time.
Either use a large enough static buffer, or allocate a single array buffer malloc or new, NOT a two dimensional buffer using calloc!

Also note that you are leaving no room for a possible ASCIIz terminator when you've finished copying your ASCII.

And granted your buffer allocations are pretty short, but realloc CAN fail! So why aren't you checking for NULL return?

[TheBeast32]

Thanks. I had to do data = realloc(data, ++datalen * sizeof(char)). I forgot the "data = ". I just looked it up in my little c reference book.

void* realloc(void* p, size_t size);
Returns pointer to newly-allocated space for an object of size size, initialised, to
minimum of old and new sizes, to existing contents of p (if non-null), or NULL on
error. On success, old object deallocated, otherwise unchanged.

Forgot it returns a pointer :/. I also got rid of the original calloc. I'll do error checking on realloc also. I didn't want to use a static buffer because I don't know how much data the user will enter. I could just ask for the amount of bytes I guess. I thought about leaving room for a null character at the end, but I'm going to be sending it to an smtp server, so does it matter?

[wildgoose]

Fair enough.
When I have data that grows with unknown maximum length I tend to track the memory usage and grow (realloc) when I reach the buffer size. Kind of what realloc does but more like growth spurts of 256 or more bytes. Not one at a time (though it appears your memory system is probably growing around 16 bytes at a time, since it did a memory exchange on your 13th realloc).