CODEWITHSUNDEEP
cpointerssegmentation-faultswap
guyOnJupiter
guyOnJupiter

Reputation: 11

C program unexpectedly crashes after giving correct output

I am trying to write a simple C program to reverse the string. The code is below:

void swap(char* a, char* b){
    char* temp;

    *temp  = *a;
    *a = *b;
    *b = *temp;
}


char* reverseString(char* str){
    int length = strlen(str);
    int i=0;

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

    return str;
}

int main(){
    char str[] = "Hello World";
    reverseString(str);
    printf("%s\n",str);

    return 0;
}

It does print the correct result but then it gives out a SEGMENTATION FAULT. It happens at the "return 0" statement of the main function.

Could you help me figure out why there is a SEG FAULT happening.

Thanks.

Upvotes: 1

Views: 92

Answers (3)

nitimalh
nitimalh

Reputation: 961

In the swap function, either use int temp; 

void swap(char *a, char *b)
{
    char temp;
    temp = *a;
    *a = *b;
    *b = temp;
}

or set int *temp = NULL so that it is not pointing to unknown memory.

void swap(char *a, char *b)
{
   char *temp = NULL;
   *temp = *a;
   *a = *b;
   *b = *temp;
}

Upvotes: 0

Paul Roub
Paul Roub

Reputation: 36438

In your swap() function, ask yourself:

void swap(char* a, char* b){
  char* temp;

Q: Where is temp pointing right now?
A: Unknown, possibly someplace dangerous.

*temp = *a;

And yet we just wrote something to that spot.

Instead, use a char:

void swap(char* a, char* b){
  char temp;

  temp = *a;
  *a = *b;
  *b = temp;
}

Upvotes: 7

Iharob Al Asimi
Iharob Al Asimi

Reputation: 53006

You invoke undefined behavior, that's why the code appears to be working, but it's not really working correctly, you have a problem here

void swap(char* a, char* b) {
    char *temp;

    *temp  = *a;
    *a = *b;
    *b = *temp;
}

you declared temp as a char pointer and then you dereference it while it's an invalid pointer, the right way to do this would be

void swap(char* a, char* b) {
    char temp;

    temp  = *a;
    *a = *b;
    *b = temp;
}

and you can do this too

void swap(char* a, char* b) {
    char temp[1];

    *temp  = *a;
    *a = *b;
    *b = *temp;
}

which doesn't make much sense, but would work, or you could even do it like this

void swap(char* a, char* b) {
    char *temp;

    temp = malloc(1);
    if (temp != NULL)
    {
        *temp  = *a;
        *a = *b;
        *b = *temp;

        free(temp);
    }
}

which makes way less sense but also works, the point is to show you that to use the indirection operator *, the pointer must be valid.

So the reason your program was crashing was undefined behavior caused by an invalid pointer dereference.

Upvotes: 2

Related Questions