views:

1027

answers:

3

Ok, I'm learning C, and I'm trying to reverse a string in place using pointers. (I know you can use an array, this is more about learning about pointers.)

I keep getting segfaults when trying to run the code below. Gcc seems not to like the *end = *begin; line. Why is that?

especially since my code is nearly identical to the non-evil c fn already discussed


#include <stdio.h>
#include <string.h>

void my_strrev(char* begin){
    char temp;
    char* end;
    end = begin + strlen(begin)-1;

    while(end>begin){
        temp = *end;
        *end = *begin;
        *begin = temp;
        end--;
        begin++;
    } 
}

main(){
    char *string = "foobar";
    my_strrev(string);
    printf("%s", string);
}
+6  A: 

One problem lies with the parameter you pass to the function:

char *string = "foobar";

This is a static string allocated in the read-only portion. When you try to overwrite it with

*end = *begin;

you'll get the segfault.

Try with

char string[] = "foobar";

and you should notice a difference.

The key point is that in the first case the string exists in the read-only segment and just a pointer to it is used while in the second case an array of chars with the proper size is reserved on the stack and the static string (which always exists) is copied into it. After that you're free to modify the content of the array.

Remo.D
Something like `char *string = strdup( "foobar" );` would work.
x4u
Thanks a lot Remo.D. That's it!
brice
Now to read about Read-only memory...
brice
@x4u. Yes but you should free the memory returned by strdup() when done. Not important in this case, I admit, but still I wouldn't suggest using strdup() unless it's the only sensible option.
Remo.D
@brice: yw :) What do you mean about reading from the read only memory?
Remo.D
@Remo.D: I had no idea that char str[] and char *str would be different. I had the impression that they would create exactly the same object in memory, so I'm going to have to get my google-foo on and learn some more! There's a good discussion here: http://stackoverflow.com/questions/1704407/what-is-the-difference-between-char-s-and-char-s-in-c
brice
Just to be perfectly clear, `char *s = "blah"` allocates storage for 5 characters somewhere that may or may not be read-only. In most cases the storage is in a read-only segment of memory alongside the machine code for the program instructions. On the other hand, `char s[] = "blah";` allocates a modifiable array of 5 characters on the stack (i.e., local storage). This is always modifiable. Take a look at (http://stackoverflow.com/questions/2036096/literal-string-initializer-for-a-character-array) for more details about literal strings.
D.Shawley
brice
+1  A: 

Change char *string = "foobar"; to char string[] = "foobar";. The problem is that a char * points to read only memory which you then try to modify causing a segmentation fault.

Kyle Lutz
+2  A: 

In your code you have the following:

*end--;
*begin++;

It is only pure luck that this does the correct thing (actually, the reason is operator precedence). It looks like you intended the code to actually do

(*end)--;
(*begin)++;

Which is entirely wrong. The way you have it, the operations happen as

  • decrement end and then dereference it
  • increment begin and then dereference it

In both cases the dereference is superfluous and should be removed. You probably intended the behavior to be

end--;
begin++;

These are the things that drive a developer batty because they are so hard to track down.

ezpz
Thanks for catching that. Edited it out.
brice