0

I'm trying to write a function in C to return the reverse of the a string that is passed into to the function.

Once the string is passed in I declare a local char [] to fill with the reverse of the string.

However, when I compile the program I receive a warning stating warning: function returns address of local variable return test;

Am I allowed to return local variable pointers from functions in C?

char *reverseString(char *str)
{
    int i, j;
    char test[strlen(str)];

    if(str == 0)
        return;

    for(i = 0, j = strlen(str); i < strlen(str), j >= 0; i++, j--)
        str[j] = test[i];

    return test;
}
Delfino
  • 967
  • 4
  • 21
  • 46
  • 4
    No, it will invoke undefined behaviour. – Marco Mar 10 '15 at 19:54
  • 3
    Don't use `strlen()` that way. – Iharob Al Asimi Mar 10 '15 at 19:54
  • 2
    You want to allocate enough memory for `strlen(str) + 1` characters to properly account for the terminating `\0` character. – wolfPack88 Mar 10 '15 at 19:57
  • Out of curiosity, any chance you can just reverse the string in-place? it is trivial and puts the onus on the *caller* to make a copy if they want to retain the original. – WhozCraig Mar 10 '15 at 19:58
  • I want to store a reversed copy of the string AND the original copy so that I can compare them both, and check to see if they're a palindrome. – Delfino Mar 10 '15 at 20:00
  • Local variables are allocated/stored on the stack, and cease to exist as soon as the function returns. So the array `test` cannot be returned from the function. The key is that `test` is not a pointer, it is an array. – user3386109 Mar 10 '15 at 20:01
  • I realize its unrelated to your original question, but you don't need to make a copy to check for a palindrome. [Example](http://pastebin.com/EdkbU60h). Don't do more work than you need to. – WhozCraig Mar 10 '15 at 20:06
  • "Am I allowed to ...?" Well, yes. That's why it generates a warning and still compiles instead of aborting with an error. However, it is almost always a very bad idea - I'm not sure I can even imagine any possible "valid" use cases for returning a pointer to a variable that ceases to exist upon execution of the return statement... But, despite being a bad idea and invoking undefined behavior, it is still allowed... – twalberg Mar 10 '15 at 20:08
  • 2
    Also, seeing your comment about your actual algorithm - you can check a string for being a palindrome without reversing it. The algorithm is identical to the reverse-in-place algorithm, except it compares pairs of characters instead of swapping them... – twalberg Mar 10 '15 at 20:10

1 Answers1

3

  1. You need more space for a string, strlen() gives you the number of characters, but c strings use an extra character to mark the end of the string, the '\0' ascii nul character, so 1 + strlen(otherString) is always required to copy a string.

  2. You check str for NULL after dereferencing it. That doesn't make sense, fist check for NULL and then call strlen().

  3. Using strlen() that way is not a good idea, because in c the string length is not stored anywhere, calling strlen() computes the string basically this way

    size_t strlen(const char *string)
{
    size_t count = 0;
    while (*string++ != '\0')
        count++;
    return count;
}

    there you can see how important the terminating '\0' is, and why you should not call strlen() in a loop for a string whose length doesn't change.

  4. If you return the address of a local variable, it will not work as you expect, because it wil be deallocated when the function returns, so you need to use dynamic allocation, for that there exists malloc() which makes it easy to do, so in your case it would be something like this

    char *reverseString(char *str)
{
    size_t index;
    char  *test;
    size_t length;

    if (str == NULL)
        return NULL;

    length = strlen(str);
    test   = malloc(1 + length);
    if (test == NULL)
        return NULL;
    /* perform string reversal */
    for (index = 0 ; index < length ; index++)
        test[index] = str[length - index - 1];
    /* add the `nul` terminator */
    test[length] = '\0';
    /* return the newly allocated and initialized data */
    return test;
}
Iharob Al Asimi
  • 52,653
  • 6
  • 59
  • 97