CODEWITHSUNDEEP
cstringpointersreverse
brice
brice

Reputation: 25039

Segmentation fault occurring when modifying a string using pointers?

Context

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

Problem

I keep getting segmentation faults 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 function already discussed in another question

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

Upvotes: 11

Views: 42623

Answers (8)

Reza
Reza

Reputation: 3929

Below, you can see my code for this problem:

#include <string>
#include <iostream>

char* strRev(char* str)
{
    char *first,*last;

    if (!str || !*str)
        return str;

    size_t len = strlen(str);
    for (first = str, last = &str[len] - 1; first < last ; first++, last--)
    {
        str[len] = *first;
        *first = *last;
        *last = str[len];
    }
    str[len] = '\0';
    return str;
}

int main()
{
    char test[13] = "A new string";
    std::cout << strRev(test) << std::endl;
    return 0;
}

Upvotes: 0

dushshantha
dushshantha

Reputation: 447

Here's my version of in-place C string reversal.

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

int main (int argc, const char * argv[])
{
    char str[] = "foobar";
    printf("String:%s\n", str);
    int len = (int)strlen(str);
    printf("Lenth of str: %d\n" , len);
    int i = 0, j = len - 1;
    while(i < j){
        char temp = str[i];
        str[i] = str[j];
        str[j] = temp;
        i++;
        j--;
    }

    printf("Reverse of String:%s\n", str);
    return 0;
}

Upvotes: 0

user720694
user720694

Reputation: 2075

You can also utilize the null character at the end of a string in order to swap characters within a string, thereby avoiding the use of any extra space. Here is the code:

#include <stdio.h>

void reverse(char *str){    
    int length=0,i=0;

    while(str[i++]!='\0')
        length++;

    for(i=0;i<length/2;i++){
        str[length]=str[i];
        str[i]=str[length-i-1];
        str[length-i-1]=str[length];
    }

    str[length]='\0';
}

int main(int argc, char *argv[]){

    reverse(argv[1]);

    return 0;
}

Upvotes: 5

Simon Peverett
Simon Peverett

Reputation: 4207

This makes for a small(ish) recursive function and works by storing the values on the way down the stack and incrementing the pointer to the start of the string (*s) on the way back up (return).

Clever looking code but awful in terms of stack usage.

#include <stdio.h>

char *reverse_r(char val, char *s, char *n)
{
    if (*n)
        s = reverse_r(*n, s, n+1);
   *s = val;
   return s+1;
}

int main(int argc, char *argv[])
{
    char *aString;

    if (argc < 2)
    {
        printf("Usage: RSIP <string>\n");
        return 0;
    }

    aString = argv[1];
    printf("String to reverse: %s\n", aString );

    reverse_r(*aString, aString, aString+1); 
    printf("Reversed String:   %s\n", aString );

    return 0;
}

Upvotes: 0

nmd
nmd

Reputation: 843

This would be in place and using pointers

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

 void reve(char *s)
 {
    for(char *end = s + (strlen(s) - 1); end > s ; --end, ++s)
    {
        (*s) ^= (*end);
        (*end) ^= (*s);
        (*s) ^= (*end);
    }
 }

int main(void)
{
    char *c = malloc(sizeof(char *) * 250);
    scanf("%s", c);
    reve(c);
    printf("\nReverse String %s", c);
}

Upvotes: 3

ezpz
ezpz

Reputation: 12047

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.

Upvotes: 4

Remo.D
Remo.D

Reputation: 16522

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.

Upvotes: 22

Kyle Lutz
Kyle Lutz

Reputation: 8036

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.

Upvotes: 2

Related Questions