What is wrong with this function?

Question

I got a problem today. It had a method and I need to find the problem in that function. The objective of the function is to append new line to the string that is passed. Following is the code

char* appendNewLine(char* str){
    int len = strlen(str);
    char buffer[1024];
    strcpy(buffer, str);
    buffer[len] = '\n';
    return buffer;
}

I had identified the problem with this method. Its kind of straight forward. The method is having a potential of having array's index out of range. That is not my doubt. In java, I use '\n' for newline. (I am basically a Java programmer, its been many years I've worked in C). But I vaguely remember '\n' is to denote termination for a string in C. Is that also a problem with this program?

Please advise.

Answer 1

Theres quite a few problems in this code.

1. strlen and not strlent, unless you have an odd library function there.
2. You're defining a static buffer on the stack. This is a potential bug (and a security one as well) since a line later, you're copying the string to it without checking for length. Possible solutions to that can either be allocating the memory on the heap (with a combination of strlen and malloc), or using strncpy and accepting the cut off of the string.
3. Appending '\n' indeed solves the problem of adding a new line, but this creates a further bug in that the string is currently not null terminated. Solution: Append '\n' and '\0' to null terminate the new string.
4. As others have mentioned, you're returning a pointer to a local variable, this is a severe bug and makes the return value corrupt within a short time.

To expand your understanding of these problems, please look up what C-style strings are, potentially from here. Also, teach yourself the difference between variables allocated on the stack and variables allocated on the heap.

EDITed: AndreyT is correct, the definition of length is valid

Answer 2

No, a '\n' is a new-line in c, just like in Java (Java grabbed that from C). You've identified one problem: if the input string is longer than your buffer, you'll write past the end of buffer. Worse, your return buffer; returns the address of memory that's local to the function and will cease to exist when the function exits.