String reverse function doesn't work

Question

I'm trying to create a function that reverses a char *. This is what I have so far:

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

char *reverse(char *x) {
     int len = strlen(x);
     char ans[len+1];
     int i;
     for (i = 0; i < len; i++) {
          ans[i] = x[len-i-1];
     }
     ans[i] = '\0';
     return ans;
}

int main() {
     char *a = reverse("hello");
     printf("%s\n", a);
}

It should print olleh, but for me, nothing prints out. Does anyone know where I messed up?

You should avoid reversing strings. Also [`std::reverse`](http://en.cppreference.com/w/cpp/algorithm/reverse) exists (you have a C++ tag on your question, after all), don't implement this yourself. — Robert Allan Hennigan Leahy, Aug 14 '14 at 04:29
As this code does (should) not compile in C++, I removed the tag. — chris, Aug 14 '14 at 04:36
Use `std::string`. And try with "now step live" or a phrase like this ;) — Jaffa, Aug 14 '14 at 04:36
returning the stack address(return ans;) will cause the undefined behavior. Change this ( char ans[len+1];) to global or heap memory and then try. — mahendiran.b, Aug 14 '14 at 04:36
If you want to preserve the original string then you must allocate new memory in the heap and return a pointer,I would just reverse the string pointed to buy 'x'(reverse in place using a temp variable) without creating 'ans' — tesseract, Aug 14 '14 at 04:40

score 2 · Accepted Answer · answered Aug 14 '14 at 04:27

You are returning ans, which is a pointer to an array of chars that reside in the stack when reverse() is called.

After reverse() returns, its stack frame is popped. Thus, the contents at the memory address stored in ans is not well defined when you try to print it out.

To remedy this, you can use heap memory instead by using malloc:

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

char *reverse(char *x) {
     int len = strlen(x);
     char *ans = malloc((len+1)*sizeof(char));
     int i;
     for (i = 0; i < len; i++) {
          ans[i] = x[len-i-1];
     }
     ans[i] = '\0';
     return ans;
}

int main() {
     char *a = reverse("hello");
     printf("%s\n", a);
     free(a);
}

In this case when reverse() returns, ans will continue to point to the char array in the heap. Just remember to free the array once you are done with it with free().

Jayesh Bhoi · Answer 2 · 2014-08-14T04:35:16.370

1

 char ans[len+1];

The content of above char array will no longer available after return from the function.Memory of above array is on stack. Try by allocating memory dynamically using malloc/calloc.

ex:

 char *ans = malloc(len+1); //Note: Sizeof char is always 1.

edited Aug 14 '14 at 04:35

answered Aug 14 '14 at 04:29

Jayesh Bhoi

24,694
15
58
73

I.e. by returning a pointer to it, and then dereferencing that pointer, one invokes undefined behaviour. – Robert Allan Hennigan Leahy Aug 14 '14 at 04:30

score -1 · Answer 3 · answered Aug 14 '14 at 04:36

-1

I'm not sure exactly what you want to accomplish but on unix systems, your code example should work. It is a bad idea though since you have returned memory from the stack which basically is not your memory to use and is volatile. Often you can immediately copy the return value to local memory already allocated. If you can, I'd suggest also you pass in the destination where you want the answer or simply return allocated memory.

answered Aug 14 '14 at 04:36

Brett Tiplitz

1

*on unix systems, your code example should work* - Unfortunately, [it doesn't](http://coliru.stacked-crooked.com/a/6bd9aa2f3838362e). Platform is not anywhere close to the only thing that influences the behaviour of undefined programs. – chris Aug 14 '14 at 04:44

String reverse function doesn't work

3 Answers3