Problem on Array Declarations

[ealforque]

whats the problem of this declaration

int getLength(char* str)
{
int i;
for(i = 0; str[i] != '\0'; i ++){}
return i;
}

void concatenate(char* s1, char* s2)
{
char temp[getLength(s1) + 1]; /* Error in this line */
}

==================================================================
Errors:
-expected constant expression
-cannot allocate an array of constant size 0
-'temp' : unknown size

[kbw]

The array size is determined at compile time. So must use a compile time constant to declare the size of an array.

You are attempting to use a run time variable to define the array size. You can't do that. You would have received a compile time error to that effect.

[ealforque]

what would be the best solution incase of such problems?

[Breadman]

use std::vector if you want to increase or decrease the size of your array

[webJose]

I beg to differ. If you are to use STL, use std::string. But it seems that the point of your exercise is to provide the functionality already given by std::string, so maybe, for academic purposes, your best solution would be to allocate memory dynamically:

char *temp = new char[getLength(s1) + 1];
//Now concatenate inside temp
//Now you are ready to return the concatenated string.  But how will you do that?
//Plus, you allocated memory.  You must deallocate it at some point using operator delete[].
//Where would such a place be? 

So, as you can see, you need to think a bit better how the concatenate() function works.

[GzwXoG1T]

Exactly as webJose, you will have to use dynamic memory. Remember to delete the memory when you are finished with it and BEFORE you return a value.

[ealforque]

char* concatenate(char* dest, char* source, int n)
{
int len = getLength(dest);
char* temp = new char[len];
char* retVar;
int i = 0, j;

while(dest[i] != '\0')
{
temp[i] = dest[i++];
}

j = i; i = 0;
while(i != n)
{
temp[j++] = source[i++];
}
temp[j] = '\0';

retVar = temp;
delete[] temp; //------------------> Error on this part: HEAP CORRUPTION DETECTED

return retVar;
}

Any advices?

[webJose]

Debug your program to understand where the problem is. For example, not wrong, but why did you assign i to j, and then set i to zero? Why not continue to use i on temp and set j to zero and use it to traverse source? Anyway, even though strange, it is not incorrect. The real problem is: You are not allocating enough memory in temp to hold both strings: The source and the dest, not to mention the terminating null char. You just allocate enough memory to hold dest.

You cannot set retVar to point to temp and then delete[] temp because it does delete what retVar points to.

As you can see, it is more difficult that one guesses at first. You have two options:

1. Do it just like the Windows SDK and the standard C functions do it: They don't allocate memory themselves. It is the function caller's responsibility to allocate the memory.
2. You allocate the memory yourself, and then leave the responsibility to the caller to delete the string him/herself once outside your function.

Sure there are more solutions, but they are more elaborated.

BTW, 2 things:

1. Use [code]//Sample code. [/code] tags to enclose code. It formats it. We all hate unformatted code.
2. The line temp[i] = dest[i++]; might not be safe in the sense that it might not work ok on all compilers. I am almost 100% sure that this doesn't work in Visual Studio. In Visual Studio (I am almost 100% sure) you need to write it as temp[i++] = dest[i];, because the other way around would leave the first char in temp unused.

[ealforque]

i assigned variable i to j because i want that the index will continue on temp and i will start from index 0 on source variable..

char* concatenate(char* dest, char* source, int n)
{
int len = getLength(dest);
char* temp = new char[len];
char* retVar; 
int i = 0, j;

while(dest[i] != '\0')
{ 
     temp[i] = dest[i++];
}

j = i; i = 0; 
while(i != n)
{
     temp[j++] = source[i++];
}
temp[j] = '\0';

retVar = temp;
delete[] temp; //------------------> Error on this part: HEAP CORRUPTION DETECTED

return retVar;
}

[kbw]

Your loop is independent of the size of the buffer. I think you're overrunning that buffer. You should consider checking against the minimum of len and n:

j = i; i = 0;
mx = std::min(len, n);
while(i != mx)

[ealforque]

kbw.. this results to memory overrun.. but it runs.. i just want to get rid of the overrun error.. because when i want to try to delete temp, retvar is also emptied..

[kbw]

Sorry, I wasn't paying close enough attention to the previous loop and the value of j.

Why don't you walk through the code (by hand) with two small input strings. It'll be pretty clear on what's wrong.