CODEWITHSUNDEEP
c++string
AKS
AKS

Reputation: 1

Please suggest what is wrong with this string reversal function?

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';
}

Upvotes: 0

Views: 209

Answers (7)

MSN
MSN

Reputation: 54614

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*.

Upvotes: 0

Tuomas Pelkonen
Tuomas Pelkonen

Reputation: 7821

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;
    }
}

Upvotes: 2

John Knoeller
John Knoeller

Reputation: 34148

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.

Upvotes: 1

Zoli
Zoli

Reputation: 1137

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.

Upvotes: 0

CB Bailey
CB Bailey

Reputation: 792327

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.

Upvotes: 7

Donal Fellows
Donal Fellows

Reputation: 137667

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

Upvotes: 3

compie
compie

Reputation: 10536

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.

Upvotes: 4

Related Questions