Whats wrong with this c++ reverse string function

Question

void reverse (char s[]){
int len = strlen(s);
int j = len - 1;
for (int i = 0; i < j; i++,j--){
    cout << s[i];
    char ch = s[i];
    s[i] = s[j]; //error line - giving exception, cannot write to the memory
    s[j] = ch;
    }
}

I am using Visual Studion 2008 and i can't understand whats the problem here .. :s .. I am out of C++ practice :$ .

Answer 1

The problem is that it uses C-style strings instead of C++ style strings. In particular, you are apparently trying to write to a constant string literal:

char const* str = "I cannot be written to";

C++ allows to omit the const here for backwards compatibility but the literal is still constant.

Finally, C++ already has a reverse function:

#include <algorithm>
#include <iostream>
#include <string>

int main() {
    std::string str = "Hello world";
    std::reverse(str.begin(), str.end());
    std::cout << str << std::endl;
}

Answer 2

I'd guess the problem is with how you're calling it, probably with a string literal, something like:

reverse("This is a string");

or: char *string = "This is a string"; reverse(string);

or some other minor variation. In any case, you're trying to write to a string literal, which gives undefined behavior.

Since you're apparently using C++, I'd consider using an std::string instead:

void reverse(std::string &s) { 
    int j=s.length()-1;
    for (int i=0; i<j; i++, j--) {
        // ..
    }
}

Answer 3

Are you calling this on a const char* or a string literal?

Are you trying to reverse

reverse("foo");

or

char *s = "foo";
reverse(s);

You'll need to create a new string from the non-writeable one and reverse that instead. You can use strdup.

char *s = strdup("foo");
reverse(s);
free(s, strlen(s));

Also note that your question is tagged c++, so you should probably be using std::string.