-1

Possible Duplicate:
How do you reverse a string in place in C or C++?
Why is this C code causing a segmentation fault?
Modifying value of char pointer in c produces segfault

Running a very simple code example

#include <stdlib.h>
#include <iostream>

char* last_char(char* s){
  char* last = s;
  while (*last) ++last;
  return last;
}

char* in_place_reverse(char* s) {
  char* left = s;
  char* right = last_char(s);
  char temp;

  while( left < right ) {
    temp = *left;
    *left = *right;
    *right = temp;

    left++;
    right--;
  }

  return s;
}

int main(){
  char * s = "letters\n";
  std::cout << in_place_reverse(s);
}

All the time I get

 Segmentation fault

But from my point of view I'm not doing anything illegal within the code. Please help me to determine what's wrong.

P.S. I compile with

g++ example.c
Community
  • 1
  • 1
meandre
  • 2,141
  • 2
  • 16
  • 21
  • 1
    You should run this code in the debugger. It will tell you which line caused the seg-fault, and you should be able to work backwards from there. – Oliver Charlesworth Jun 06 '12 at 17:50
  • Note that your function last_char returns the delimiting `\0`, so if your code will not segfault, then `std::cout` will output nothing because your first char in the string will be `\0` after reversion. – Nobody moving away from SE Jun 06 '12 at 17:54
  • [This code to reverse a string is really short](http://stackoverflow.com/a/6560310/176769). – karlphillip Jun 06 '12 at 17:59

2 Answers2

5

Two problems:

  1. You are trying to modify a string literal. This may work, it may not, or it may crash. This is invoking undefined behavior. Use char s[] = "letters\n" to make a mutable copy.
  2. last_char() in fact returns a pointer to the sentinel '\0' at the end of the string -- it points beyond the last character. Change return last to return last - 1. Otherwise you are going to move the sentinel around too, and that's almost certainly not what you want. (Note that this will return a pointer to garbage if the string is zero-length. You should fast-succeed in in_place_reverse() if *s == '\0' to avoid this complexity.)
cdhowie
  • 158,093
  • 24
  • 286
  • 300
  • are you sure about #2? i've tested it separately and it always returns last character. it would return '\0' if the cycle was `while (*last++);` – meandre Jun 06 '12 at 17:54
  • Yes. `last_char()` will return `last` only when `!*last`. – cdhowie Jun 06 '12 at 17:55
2

You are modifying a string literal and string literals are non-modifiable.

Use char s[] = "letters\n"; instead

ouah
  • 142,963
  • 15
  • 272
  • 331