Why is this code giving me segmentation error?

Question

I'm new to C, trying to get an user entered array into ascending order and print it. I'm trying to use selection sort for this.My devcpp crashes after accepting 1st input and when I tried to run it on an online c compiler, its giving segmentation error. Can someone tell me why is this happening here? TIA

#include<stdio.h>

void swap()
{
    int *x, *y;
    int temp = *x;
    *x = *y;
    *y = temp;
}

void ss(int A[], int n)
{
    int i, j, min;
    for(i=0; i<n-1;i++)
    {
        min = i;
        for(j=i+1; j<n; j++)
        {
            if (A[j]< A[min])
            {
                min = j;
            }
            swap(&A[min], &A[i]);
        }
    }
}

void print(int A[], int n)
{
    int i;
    for(i=0; i<n; i++)
    {
        printf("%d",A[i]);
    }
}

int main()
{
    int A[4], i, n;
    printf("Enter the elements");
    scanf("%d", &A[i]);
    n=4;
    ss(A,n);    
    printf("Sorted array \n");
    print(A,n);
    return 0;

}

I want the user entered array in ascending order.

Look closely at the prototype and definition of `swap` and how it is used. — P.W, Feb 07 '19 at 05:09

score 1 · Answer 1 · answered Feb 07 '19 at 05:14

Your swap() function has no parameters, and the pointers x and y are not pointing to anything. Your segmentation fault is coming from statements like:

int temp = *x;

Because *x has no determined value, you didn't assign an address to x of any existing value in your program, yet your trying to indirect it here.

Instead, put parameters in your swap function:

void swap(int* x, int* y)
{
    int temp = *x;
    *x = *y;
    *y = temp;
}

And then in the outer scope, pass proper, existing values on the stack's addresses to this swap function.

paxdiablo · Answer 2 · 2019-02-07T05:22:56.557

Well, this is wrong, for a start:

void swap()
{
    int *x, *y;
    int temp = *x;
    *x = *y;
    *y = temp;
}

This creates two brand new pointers within the function, with arbitrary values, and then attempts to dereference them, something that's undefined behaviour.

Since you're passing two pointers to the swap function with swap(&A[min], &A[i]), you should receive those in the parameter list so that you can operate on them:

void swap(int *x, int *y) {
    int temp = *x;
    *x = *y;
    *y = temp;
}

And, though it's not a bug, you may want to consider using more descriptive names than A or ss (e.g., dataArray and SelectionSort).

This will tend to make your code much more readable and therefore maintainable if you have to, for example, come back and modify it twelve months in the future.

You will also need a loop in your main function to get the four values. At the moment, you get only one and with an arbitrary index i which may cause you undefined behaviour again.

Since you've already done similar loops in the other two functions, I'll assume you can handle this task without me giving you the code.

score 1 · Answer 3 · answered Feb 07 '19 at 05:58

There are quite a few errors buddy. I'm listing them out below:

In the main(), check how you are accepting the array values from the user. i is undefined and you are trying to insert into just A[i] without using a for loop or without defining i either. That's where you get your seg fault.

for(i = 0; i < 4; i++) scanf("%d", &A[i]);
Another mistake is in the way you have defined the swap() function . Since you have called swap function as swap(&A[min], &A[j]);, i.e., passing addresses as parameters, your function too needs to have pointers to these addresses. should be something like this: void swap(int* x, int* y).
As a result of the change we made in the 2nd point above, you need to remove this line ==> int *x, *y;, which declares the 2 pointers again inside the swap function.

here is the code which should work:

#include<stdio.h>

void swap(int* x, int* y)
{
    int temp = *x;
    *x = *y;
    *y = temp;
}

void ss(int A[], int n)
{
    int i, j, min;
    for(i=0; i<n-1;i++)
    {
        min = i;
        for(j=i+1; j<n; j++)
        {
            if (A[j]< A[min])
            {
                min = j;
            }
            swap(&A[min], &A[j]);
        }
    }
}

void print(int A[], int n)
{
    int i;
    for(i=0; i<n; i++)
    {
        printf("%d\t",A[i]);
    }
}

int main()
{
    int A[4], i, n;
    printf("Enter the elements:\n");
    for(i = 0; i < 4; i++)
      scanf("%d", &A[i]);
    n=4;
    ss(A,n);    
    printf("Sorted array \n");
    print(A,n);
    return 0;
}

Which would give the output as follows:

Enter the elements: 1 2 3 4 Sorted array 4 3 2 1

Hey man, I tried this. Thanks! Now the thing is, it is working fine for all positive numbers but when I give zero or any negative number, its not working fine. Any idea why this might be happening? — NewToCoding, Feb 07 '19 at 12:22
it works.. kindly cross-check your input.. it should work with negatives. If still not working post your output as part of the question itself and notify me. — Harshith Rai, Feb 08 '19 at 03:33

Why is this code giving me segmentation error?

3 Answers3