30

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.

Improve this question
9
  • 1
    Segfault almost always means you're trying to dereference a null pointer. Run your code through GDB, find which line it's segfaulting on, and see which pointer is null (0x000000) there. Commented Oct 23, 2009 at 17:03
  • Also, why are you using two variables (i and j)? You can absolutely do this with counter. Commented Oct 23, 2009 at 17:04
  • Can you post the code that creates the char* and passes it to reverse()? Commented Oct 23, 2009 at 17:07
  • 3
    And don't use strlen() in a loop condition. Commented Oct 23, 2009 at 17:13
  • 3
    @Carl Norum: If you lok carefully, strlen is not used in the loop condition in the above code sample. Commented Oct 23, 2009 at 21:20

8 Answers 8

Reset to default
53

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");
Improve this answer
Sign up to request clarification or add additional context in comments.

3 Comments

This worked. Thanks. But can you please explain in more detail what the difference is between the two string initializations? I.e. why does the way I did create a string literal whereas the 'array' syntax creates a modifiable string?
@james String literals go in read-only storage. But by doing "char []s = ..." you're declaring an array and initializing it to a literal, rather than getting a pointer to a literal.
@james: See the additional text in my reply.
10

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);
Improve this answer

1 Comment

The literal "teststring" is located in read only memory somewhere and you are not allowed to write to it. This used to be common practice, ie a way of allocating some memory, but most modern systems won't allow it. JaredPar's example works because strdup allocates memory which you then own.
3

Are you testing this something like this?

int main() {
    char * str = "foobar";
    reverse(str);
    printf("%s\n", str);
}

This makes str a string literal and you probably won't be able to edit it (segfaults for me). If you define char * str = strdup(foobar) it should work fine (does for me).

Improve this answer

Comments

3

Your declaration is completely wrong:

char* s = "teststring";

"teststring" is stored in the code segment, which is read-only, like code. And, s is a pointer to "teststring", at the same time, you're trying to change the value of a read-only memory range. Thus, segmentation fault.

But with:

char s[] = "teststring";

s is initialized with "teststring", which of course is in the code segment, but there is an additional copy operation going on, to the stack in this case.

Improve this answer

Comments

1

See Question 1.32 in the C FAQ list:

What is the difference between these initializations?

char a[] = "string literal";
char *p  = "string literal";

My program crashes if I try to assign a new value to p[i].

Answer:

A string literal (the formal term for a double-quoted string in C source) can be used in two slightly different ways:

As the initializer for an array of char, as in the declaration of char a[], it specifies the initial values of the characters in that array (and, if necessary, its size).

Anywhere else, it turns into an unnamed, static array of characters, and this unnamed array may be stored in read-only memory, and which therefore cannot necessarily be modified. In an expression context, the array is converted at once to a pointer, as usual (see section 6), so the second declaration initializes p to point to the unnamed array's first element.

Some compilers have a switch controlling whether string literals are writable or not (for compiling old code), and some may have options to cause string literals to be formally treated as arrays of const char (for better error catching).

(emphasis mine)

See also Back to Basics by Joel.

Improve this answer

2 Comments

While this link may answer the question, it is better to include the essential parts of the answer here and provide the link for reference. Link-only answers can become invalid if the linked page changes. - From Review
@Sree It should be fixed now.
0

Which compiler and debugger are you using? Using gcc and gdb, I would compile the code with -g flag and then run it in gdb. When it segfaults, I would just do a backtrace (bt command in gdb) and see which is the offending line causing the problem. Additionally, I would just run the code step by step, while "watching" the pointer values in gdb and know where exactly is the problem.

Good luck.

Improve this answer

Comments

0

As some of the answers provided above, the string memory is read-only. However, some compilers provide an option to compile with writable strings. E.g. with gcc, 3.x versions supported -fwritable-strings but newer versions don't.

Improve this answer

Comments

-1

I think strlen can not work since s is not NULL terminated. So the behaviour of your for iteration is not the one you expect. Since the result of strlen will be superior than s length you will write in memory where you should not be.

In addition s points to a constant strings hold by a read only memory. You can not modify it. Try to init s by using the gets function as it is done in the strlen example

Improve this answer

1 Comment

How is s not null terminated? Literal strings are always null terminated.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.