C: Returned char array becomes garbage

Question

The problem is returned value SQLStatement from function. It is generated exactly in the way I wanted (checked in the debug mode), but when I call the function in the main(), there is a garbage as returned value. Function and sample code is given below:

char *GenerateSQLStatement(char *SQLQuery){

size_t SQLQueryCounter = 0;
size_t TableNameCounter = 0;
size_t CVIQueryConstantCounter = 0;

while(SQLQuery[SQLQueryCounter] != '\0') SQLQueryCounter++; 
while(TableName[TableNameCounter] != '\0') TableNameCounter++;
while(CVIQueryConstant[CVIQueryConstantCounter] != '\0') CVIQueryConstantCounter++; 

char SQLStatement [CVIQueryConstantCounter + TableNameCounter + SQLQueryCounter + 1];

for (int i = 0; i <= (CVIQueryConstantCounter + TableNameCounter + SQLQueryCounter); i++) SQLStatement[i] = '\0';

strcat(SQLStatement, CVIQueryConstant);
strcat(SQLStatement, TableName);
strcat(SQLStatement, SQLQuery);

return SQLStatement;    
}

void main(){
char *SQLStatement = GenerateSQLStatement("test");
}

Any ideas what's wrong?

Why are you not using `strlen` to count string length? Also, you need to dynamically allocate `SQLStatement` with `malloc`, since it will be out of scope after the function call is done. Or, `strcpy` the result in the same line as the function call (or `strdup`). — crashmstr, May 30 '14 at 14:35
Strlen - totally forgot. When to free memory when use malloc? If I do it in the function it is the same thing as I've already did, the array will not exist out of the function? — Nikson, May 30 '14 at 14:40
The way your code is now, `SQLStatement` is a local variable on the stack. It only exists within the scope of the function. After the function returns, that memory is no longer valid. By using `strcpy` or `strdup` on the same line as the function call, you can copy the contents before it goes out of scope. You use `malloc` to create memory that lasts *longer* than the current scope. That memory can be used until you call `free` (which you do when you no longer need the memory). — crashmstr, May 30 '14 at 14:47
You free the memory after use, in this case in the main() function. If you free it in the function, the buffer is invalid and its address should not be returned. — Rudy Velthuis, May 30 '14 at 14:47
Also, use `memset()` or `bzero()` or similar to zero out your array - it can be more efficient than your `for` loop (but it's not necessarily guaranteed to be - depends on hardware capabilities, compiler version, and optimization settings) and is a bit more readable... — twalberg, May 30 '14 at 14:56
I wanted to avoid somehow freeing the memory outside of the function, but it is impossible. I will do it with calloc, and must not forget to free memory every time I call the function. — Nikson, May 30 '14 at 15:00

score 2 · Accepted Answer · answered May 30 '14 at 14:36

Beginners mistake: You are returning a pointer to data on the stack. When the function returns, the data that the pointer points to is gone.

I'd also recommend you learn the use of the strlen, strcpy, and memset functions. Then ask someone what a buffer overflow is, and why your code is likely to create one. Finally look at the strlcpy and strlcat functions.

C: Returned char array becomes garbage

1 Answers1