Why is this string reversal C code causing a segmentation fault? [duplicate]

Question

I am trying to write code to reverse a string in place (I'm just trying to get better at C programming and pointer manipulation), but I cannot figure out why I am getting a segmentation fault:

#include <string.h>  void reverse(char *s);  int main() {     char* s = "teststring";     reverse(s);      return 0; }  void reverse(char *s) {     int i, j;     char temp;      for (i=0,j = (strlen(s)-1); i < j; i++, j--) {         temp = *(s+i);     //line 1         *(s+i) = *(s+j);   //line 2         *(s+j) = temp;     //line 3     } } 

It's lines 2 and 3 that are causing the segmentation fault. I understand that there may be better ways to do this, but I am interested in finding out what specifically in my code is causing the segmentation fault.

Update: I have included the calling function as requested.

Answer 1

There's no way to say from just that code. Most likely, you are passing in a pointer that points to invalid memory, non-modifiable memory or some other kind of memory that just can't be processed the way you process it here.

How do you call your function?

Added: You are passing in a pointer to a string literal. String literals are non-modifiable. You can't reverse a string literal.

Pass in a pointer to a modifiable string instead

char s[] = "teststring"; reverse(s);  

This has been explained to death here already. "teststring" is a string literal. The string literal itself is a non-modifiable object. In practice compilers might (and will) put it in read-only memory. When you initialize a pointer like that

char *s = "teststring"; 

the pointer points directly at the beginning of the string literal. Any attempts to modify what s is pointing to are deemed to fail in general case. You can read it, but you can't write into it. For this reason it is highly recommended to point to string literals with pointer-to-const variables only

const char *s = "teststring"; 

But when you declare your s as

char s[] = "teststring"; 

you get a completely independent array s located in ordinary modifiable memory, which is just initialized with string literal. This means that that independent modifiable array s will get its initial value copied from the string literal. After that your s array and the string literal continue to exist as completely independent objects. The literal is still non-modifiable, while your s array is modifiable.

Basically, the latter declaration is functionally equivalent to

char s[11]; strcpy(s, "teststring"); 

Answer 2

You code could be segfaulting for a number of reasons. Here are the ones that come to mind

1. s is NULL
2. s points to a const string which is held in read only memory
3. s is not NULL terminated

I think #2 is the most likely. Can you show us the call site of reverse?

EDIT

Based on your sample #2 is definitely the answer. A string literal in C/C++ is not modifiable. The proper type is actually const char* and not char*. What you need to do is pass a modifiable string into that buffer.

Quick example:

char* pStr = strdup("foobar"); reverse(pStr); free(pStr);