Please suggest what is wrong with this string reversal function?

Question

This code is compiling clean. But when I run this, it gives exception "Access violation writing location" at line 9.

void reverse(char *word)
{
int len = strlen(word);
len = len-1;
char * temp= word;
int i =0;
while (len >=0)
{
word[i] = temp[len];  //line9
++i;--len;
}
word[i] = '\0';
}

Answer 1

Be careful: not all strings in a C++ program are writable. Even if your code is good it can still crash when someone calls it with a string literal.

Answer 2

When len gets to 0, you access the location before the start of the string (temp[0-1]).

Answer 3

Have you stepped through this code in a debugger?

If not, what happens when i (increasing from 0) passes len (decreasing towards 0)?

Note that your two pointers word and temp have the same value - they are pointing to the same string.

Answer 4

Well, for one, when len == 0 len-1 will be a negative number. And that's pretty illegal. Second, it's quite possible that your pointer is pointing at an unreserved area of memory.

Answer 5

The function looks like it would not crash, but it won't work correctly and it will read from word[-1], which is not likely to cause a crash, but it is a problem. Your crashing problem is probably that you passed in a string literal that the compiler had put into a read-only data segment.

Something like this would crash on many operating systems.

char * word = "test";
reverse(word); // this will crash if "test" isn't in writable memory

There are also several problems with your algorithm. You have len = len-1 and later temp[len-1] which means that the last character will never be read, and when len==0, you will be reading from the first character before the word. Also, temp and word are both pointers, so they both point to the same memory, I think you meant to make a copy of word rather than just a copy of the pointer to word. You can make a copy of word with strdup. If you do that, and fix your off-by-one problem with len, then your function should work,

But that still won't fix the write crash, which is caused by code that you have not shown us.

Oh, and if you do use strdup be sure to call free to free temp before you leave the function.

Answer 6

Try this:

void reverse(char *word)
{
  size_t len = strlen(word);
  size_t i;

  for (i = 0; i < len / 2; i++)
    {
      char temp = word[i];
      word[i] = word[len - i - 1];
      word[len - i - 1] = temp;
    }
}

Answer 7

If you called that function as followed:

reverse("this is a test");

then with at least one compiler will pass in a read only string due to backwards compatibility with C where you can pass string literals as non-const char*.