C reverse string function getting weird output

Question

I'm trying to understand pointers and made a reverse string function.
code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

char *reverseString(char string[]){
    int i;
    int len = strlen(string);
    char reversedString[len];  
    for (i = 0; i < len; i++){
        reversedString[i] = string[(len - 1) - i];
    }
    printf("%s", reversedString); //print it out

    return reversedString; //return pointer to first element of reversed string
}

int main (){
    char string[6] = "kitten";
    int i;
    char *p = reverseString(string);
    return (0);

}

My goal is to reverse the string "kitten" and print the reversed string. I expect the output "nettik" but I get "nettik���". Why am I getting these weird characters?

Answer 1

Stop! First of all, you're making a classic mistake beginners often make when they learn about pointers. When you write:

char *reverseString(char string[]) {
    ...
    char reversedString[len];
    return reversedString;
}

You're returning a pointer to where it was, not is: the memory actually gets freed when you leave the function, so there's no guarantee that the memory you think contains your reversed string isn't already getting reused by something else. This way, your program will fail catastrophically.

However, you're printing the reversed string before returning it, so it's no huge problem yet. What's going wrong there, then, is that your string isn't properly zero-terminated.

A string in the C language needs room to store its final '\0' byte: char string[6] = "kitten"; is too small to hold the zero-terminated string { 'k', 'i', 't', 't', 'e', 'n', '\0' }. Similarly, when you call printf("%s", reversedString), you haven't properly terminated the string with '\0', so printf keeps looking for the end of the string and prints out whatever junk is in memory where reversedString is allocated.

You should try:

• making your function return void,
• allocating 7 bytes for your original string,
• allocating strlen(string) + 1 (which will also be 7) bytes for your new string, and
• writing a '\0' to the end of your new string after looping through the old one in reverse.

Answer 2

There are at least two major bugs in your code. There could be more.

First, the call to strlen causes undefined behaviour because you aren't passing it a null-terminated string. The reason for this is that your char array isn't large enough for the null terminator character:

char string[6] = "kitten";

You need

char string[7] = "kitten";

or

char string[] = "kitten";

Second, you are returning a pointer to a local array, namely reversedString. De-referencing that would also cause undefined behaviour. You can solve this by wither reversing the string in-place, or passing a pointer to a buffer of the same length as the input. Remember that the reversed string must also be null-terminated.