what is the segmentation fault in this program

Question

 #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 ?

1) `small=a[0]; large=a[n-1];` --> `small=a[0]; s=0; large=a[n-1]; l=n-1;` 2) `a[l]=a[s];` --> `a[l]=temp;` — BLUEPIXY, Mar 12 '17 at 04:24
You can use the C debugger, gdb, to find the line where the segmentation fault occurs. — Ben Frankel, Mar 12 '17 at 04:36
Please turn your compiler's warning to maximum so you can easily get information of possible problem points, like the ones here — Sami Kuhmonen, Mar 12 '17 at 06:04

score 5 · Answer 1 · answered Mar 12 '17 at 04:34

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;

score 2 · Answer 2 · answered Mar 12 '17 at 04:35

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

Murtaza Chiba · Answer 3 · 2017-03-12T18:32:54.440

0

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

edited Mar 12 '17 at 18:32

answered Mar 12 '17 at 04:47

Murtaza Chiba

41
3

1

You can if the compiler supports variable-length arrays. Also don't cast the return value of malloc, ever. This is not an answer to the problem – Sami Kuhmonen Mar 12 '17 at 06:02
The explicit cast is not necessary, but helps with code readability and prevents side effects due to change of the type of 'a' in the future. Variable-length arrays is **not** a standard option and needs to be explicity enabled. – Murtaza Chiba Mar 12 '17 at 17:46
No, it hides problems, adds clutter and is usually considered bad practice. http://stackoverflow.com/a/605858/1806780 – Sami Kuhmonen Mar 12 '17 at 17:48
The link suggests using the type of the dereferenced pointer variable. IMHO that is also an issue if you change the type and are not handling the change correctly. Clutter is less of a worry than having unknown side effects. – Murtaza Chiba Mar 12 '17 at 17:58

what is the segmentation fault in this program

3 Answers3