Sivakami Subramaniam
Sivakami Subramaniam

Reputation: 453

what is the segmentation fault in this program

 #include<stdio.h>
 #include <stdlib.h>
 int main()
 {
         int n,small=0,large=0,s,l,temp;
         printf("this should work");
         scanf("%d",&n);
          //   printf("%d",n);//
         int a[n];
        for(int i=0;i<n;i++)
             {
              scanf("%d",&a[i]);
              }
             /*    for(int i=0;i<n;i++)
                          printf("%d",a[i]);*/
        small=a[0];
        large=a[n-1];
        for(int i=0;i<n;i++)
        {
               if(a[i]<small && i!=0)
               {  
                       small=a[i];
                       s=i;
                }
                if(a[i]>large && i!=n-1)
                { 
                        large=a[i];
                        l=i;
                 }
        }
        temp=a[s];
        a[s]=a[l];
        a[l]=a[s];
        for(int i=0;i<n;i++)
              printf("%d ",a[i]);
         return 0;
  }

This is a simple program to swap the largest and smallest number in an array and print the new array. When I tried to run this program I got a segmentation fault. Usually, a segmentation fault occurs when we try to access an out of bound memory location. So I added printf statements to find out where the error is. But none print statements were executed. what is the error here ?

Upvotes: 1

Views: 66

Answers (3)

Murtaza Chiba
Murtaza Chiba

Reputation: 41

You cannot declare an array based on a dynamic size, unless the compiler supports it and even then it is generally not portable.

int a[n]

you actually need to use malloc or calloc.

int *a;

a = (int *)malloc(n * sizeof(int)); or a = (int *)calloc(n, sizeof(int));

Upvotes: 0

Ayush
Ayush

Reputation: 759

You should initialize s and l to some value because when the if condition does not works their values will remain uninitialized garbage values. Hence, a[l] or a[s] will not work, since these indexes are undefined values. That is why you will get segmentation fault because you are accessing an undefined area of an array.

So, use random values within array range like s=0,l=0 to initialize the variables or you can add some flags to check if the conditions are working.

if (l != 0 && s != 0) {
    temp=a[s];
    a[s]=a[l];
    a[l]=a[s];
}

Also, I think you are swapping the values so in the last line a[l]=temp instead of a[l]=a[s].

ideone link

Upvotes: 2

paxdiablo
paxdiablo

Reputation: 881093

One problem is that you don't actually set s and l to anything unless you find an element that is smaller/larger than the current one.

That means (for example), if the first element is the smallest, s will be set to some arbitrary value and trying to index the array with it could be problematic.

To fix that, where to set small and large, you should also set:

s = 0;
l = n - 1;

In addition, your swap code is wrong and should be:

temp = a[s];
a[s] = a[l];
a[l] = temp;

Upvotes: 5

Related Questions