-2

When I was writing this program, was getting segmentation fault

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

but when I allocated some memory, no segmentation fault was there

void swap(int *a,int *b){
    int *temp;
    temp = malloc(sizeof(int));
    if (temp == NULL)
      return;
    *temp=*a;
    *a=*b;
    *b=*temp;
    free(temp);
}

But what is the cause behind this?

Ananya
  • 35
  • 1
  • 6

3 Answers3

3

Your code should be:

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

In other words, during your swap, you really want to hold an integer value versus a pointer to an integer.

By not having memory to back the temp pointer you had and the fact that it's uninitialized means it's pointing to some random location, which is badness. Once you dereference it to save the value behind a, you hit the segfault. In this version, we're setting aside stack space to store an integer (not a pointer to one, but an actual integer). That means memory is set aside to hold the value, so we can copy data into it and out of it without issue (as long as you don't try to read or write more than what was set aside).

BTW, it's probably a good idea to turn on extra warning by the compiler. If you're using clang or gcc, I highly recommend adding -Wall and -Wextra to your compile command line. And you should consider learning to use a debugger where you can single step through and see what is actually happening.

John Szakmeister
  • 44,691
  • 9
  • 89
  • 79
2

You just need a temporary memory location of type int. The easy way to get this is to declare temp of type int:

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

What you did in the first version was to use temp uninitialized, which is one of the numerous way a C program can be incorrect. Assigning to *temp reads the value of temp in order to write at the location it points to, but temp was not set to any value in particular. Anything can happen.

In the second version, you set the value of temp before using it, and you even set it to the address of a valid memory location to store an int, so that makes everything work:

    temp = malloc(sizeof(int));
    if (temp == NULL)
      return;

Although if you decide to write your function this way, it can fail, so it should be able to indicate to the caller somehow when it failed. One way to do this would be to make it return a success code instead of void:

// returns -1 for failure, 0 for success
int swap(int *a,int *b){
    int *temp;
    temp = malloc(sizeof(int));
    if (temp == NULL)
      return -1;
    *temp=*a;
    *a=*b;
    *b=*temp;
    free(temp);
    return 0;
}
Pascal Cuoq
  • 79,187
  • 7
  • 161
  • 281
0

Segmentation error means your program has attempted to access an area of memory that it is not allowed to access. There is many reason why there might come segmentation in your above code. I think due to not using & sign while calling the swap function.

you should use swap(&arg1,&arg2); to call to swap function.

But in second code you use dynamic memory allocation which will allocate memory to the area in RAM where memory is free. So there is no segmentation error.

  • 1
    I think the OP is using `&` when they call the `swap` function, they're just not initializing `temp` in the first example. – S.S. Anne Aug 31 '19 at 15:24