Thread: what is wrong here??? dynamic array of chars

t is pointing to a constant literal (t="Hello world";). You cannot change these. It would also crash because you're trying to free it with delete.

Not to mention you completely change the point to something new. In other words, you allocated memory is now lost.
So 1) you can't free it and 2) string literals are const, so you get an access violation trying to modify any of the data and 3) there's not enough space.
There's a reason why we avoid using C-style strings in C++.
To copy a string into an allocated buffer, use strcpy.

No, this is bad too. It's a C way. It only increases the chance for buffer overruns and stops at whitespace.
Better to use cin.get, or even better use std::string and std::getline instead.

You can also do it your old way by doing:

Code:

		strcpy(t, "Hello world");
		strcpy(s, "avi");

Again, however, I must note that this is insecure, especially in C++ since you do no checking if there's enough room in the buffer.
The C++ way is much better and easier:

Code:

std::string str1 = "Hello world";
std::string str2 = "avi";
str1 += str2;

Originally Posted by lionheart

Thanks, people...
Now it is working and I don't understand why (the idea of constant vs. cin... it's stupid)...
here's the working version...

Code:

void main(){
	char *t, *s;
	t=new char[200];
	s=new char[100];
	if (t==NULL || s==NULL)
		exit(1);
	else
	{
		cin>>t;     //here's the correction
		cin>>s;     //here's the correction
		strcat(t,s);
		cout<<t;
	}	
	delete []t;
	delete []s;
}

What's the idea of constant vs cin? You can use string literals, you just have to COPY it into the char array using strncpy or in the constructor of the char array.

Code:

void main()
{
    char t[200], s[100];  //why use dynamic allocation?
    strncpy(t, "Hello World", sizeof(t)); //copy "Hello World" to t
    strncpy(s, "avi", sizeof(s));   //Copy "avi" to s
    strcat(t,s);    //call that function you made
    printf("%s\n", t);  //display results
}

You may also copy string literals like this:

Code:

char t[] = "Hello World"

It only works in the constructor and the resulting size of the array will only be as long as the string + 1 so you couldnt copy anything else to it

Strncpy has its own quirks, however.
If the buffer is not large enough to hold the data you copy, it will copy sizeof(t) characters, but it won't put a null char, so you will get a non-null terminated string and therefore C string functions will mostly likely crash the application.
There's just no safe way of doing it in today's C world without a lot of additional checks.

Now that I see it, don't use void main:
http://cpwiki.sourceforge.net/Void_main

OK, I'm confused here.
I can swear this was in the C++ section some time ago.
Furthermore, you ARE using C++ with new/delete, cin/cout.
So which is it here - C or C++?
If it's C, you should replace new/delete with malloc/free, and cin/cout with fgets/printf,
and if it's C++, you should probably use std::string and std::getline.

I also found an additional problem:

Code:

	if (t==NULL || s==NULL)
		exit(1);

If this condition is true, you never free your allocation buffers.

Originally Posted by lionheart

the size of 's' cannot be over the physical size of 't'...

But even with this argument, even if t is just +1 greater than s, you can still get an overrun. And you're going to make the mistake someday. So it's critical that you make checks to avoid buffer overruns, so it won't come back and bite you one day, somewhere in the code.

Well, you're still using C++ code (cin/cout/new/delete).
So if you're using C, then how come you're using C++ code?

Originally Posted by 39ster

I also never check the result of malloc() or new. I dont know if that's good practice or not but generally if malloc or new fails, than you have bigger problems and your whole program is going to fail wether you check it or not.
EDIT: Unless its for HUGE allocations

Yes, it's bad practice not to check since if it fails, you may attempt to access an invalid pointer (or in the case of C++, it will throw an exception).
It's better to fail with some elegant error or try to recover than crash.

A better way...

Code:

int main()
{
	char* t = malloc(200);
	char* s = malloc(100);
	if (t==NULL || s==NULL)
		goto Exit;
	else
	{
		strcpy(t, "Hello world");
		strcpy(s, "avi");
		strcat(t, s);
		printf("%s", t);
	}	

Exit:
	if (t) free(t);
	if (s) free(s);
}

You still haven't explained why there's C++ in your C code.