Terminal showing "segmentation fault(core dumped)" in C, Please check what is wrong in my code?

Question

The code is about that takes the addresses of three double variables as arguments and that moves the value of the smallest variable into the first variable, the middle value to the second variable, and the largest value into the third variable.

#include<stdio.h>
void swapy(double *x, double *y, double *z);

int main()
{
    double no1,no2,no3;

    printf("Enter three numbers\n");
    printf("no1 = ");
    scanf("%lf",&no1);
    printf("no2 = ");
    scanf("%lf",&no2);
    printf("no3 = ");
    scanf("%lf",&no3);
    swapy(&no1,&no2,&no3);
    printf("The numbers will be printed in increasing order!\n");
    printf("no1 = %lf\n",no1);
    printf("no2 = %lf\n",no2);
    printf("no3 = %lf\n",no3);

}

void swapy(double *x, double *y, double *z)
{

     double *max; 
     double *mid; 
     double *min;   

     if(*x>*y && *x>*z) {

        *max = *x;
     }
     else if(*y>*x && *y>*z) {

        *max = *y;
     }
     else {

        *max = *z;
     }



     if(*x<*y && *x<*z) {

        *min = *x;
     }
     else if(*y<*x && *y<*z) {

        *min = *y;
     }
     else {

        *min = *z;
     }

    if(*x != *max && *x != *min) {

        *mid = *x;
     }
     else if(*y != *max && *y != *min) {

        *mid = *y;
     }
     else {

        *mid = *z;
     }

     *x = *min;
     *y = *mid;
     *z = *max;

     return;
}

enter image description here

the code is about that takes the addresses of three double variables as arguments and that moves the value of the smallest variable into the first variable, the middle value to the second variable, and the largest value into the third variable.

`max` is uninitialized, and when you dereference it with `*max` you invoke undefined behavior. Pointers must point to something before you can dereference them. Same for `min` and `mid`. Those 3 don't need to be pointers anyway, just use `double min, max, mid;` — yano, Jan 27 '18 at 18:18
Welcome to Stack Overflow, please take the [tour] and read [ask]. Also don't post pictures of code and output. Copy&paste the output here in your question. — Pablo, Jan 27 '18 at 18:25
@AashutoshSahni just use automatic storage.. `double max = *x;`. No reason to use pointers here. — yano, Jan 27 '18 at 18:28
also, in general, be wary of using `double`s for math and comparisons. You can get some unexpected results,.. https://stackoverflow.com/questions/588004/is-floating-point-math-broken — yano, Jan 27 '18 at 18:35
A picture of text - genius! SO is quite capable or rendering _actual_ text. — Clifford, Jan 27 '18 at 18:37

score 1 · Accepted Answer · answered Jan 27 '18 at 18:28

Your problem is clear here: max, mid and and min are uninitialized. Try:

void swapy(double *x, double *y, double *z)
{

     double max; 
     double mid; 
     double min;   

     if( (*x>*y) && (*x>*z)) {

        max = *x;
     }
     else if((*y>*x) && (*y>*z)) {

        max = *y;
     }
     else {

        max = *z;
     }



     if((*x<*y) && (*x<*z)) {

        min = *x;
     }
     else if((*y<*x) && (*y<*z)) {

        min = *y;
     }
     else {

        min = *z;
     }

    if((*x != max) && (*x != min)) {

        mid = *x;
     }
     else if((*y != max) && (*y != min)) {

        mid = *y;
     }
     else {

        mid = *z;
     }

     *x = max;
     *y = min;
     *z = mid;
}

you can notice I added brackets in your if conditions. This is a good habit to get, else it could lead to unwanted behavior sometimes.

Achal · Answer 2 · 2018-01-27T18:31:42.670

0

max, min and mid are Un-initilized pointer and then you are performing *max = *x; which is causing segmentation fault.

First assign max,min and min valid address and then do further operation.

You can allocate 8 bytes to each dynamically and then you can store values.

double *max=malloc(sizeof(double));
 double *mid=malloc(sizeof(double));
 double *min=malloc(sizeof(double));

complete function

void swapy(double *x, double *y, double *z)
{

        double *max=malloc(sizeof(double)); 
        double *mid=malloc(sizeof(double)); 
        double *min=malloc(sizeof(double));   
        if(*x>*y && *x>*z) {
                *max = *x;
        }
        else if(*y>*x && *y>*z) {
                *max = *y;
        }
        else {
                *max = *z;
        }
        if(*x<*y && *x<*z) {
                *min = *x;
        }
        else if(*y<*x && *y<*z) {
                *min = *y;
        }
        else {
                *min = *z;
        }
        if(*x != *max && *x != *min) {
                *mid = *x;
        }
        else if(*y != *max && *y != *min) {
                *mid = *y;
        }
        else {
                *mid = *z;
        }
        *x = *max;
        *y = *min;
        *z = *mid;

        // return;//wrong .. return type mentioned as void 
}

edited Jan 27 '18 at 18:31

answered Jan 27 '18 at 18:22

Achal

11,821
2
15
37

your malloc lines are wrong, need to allocate for sizeof double, not sizeof int. Also may as well just have max, mid and min be doubles and skip malloc entirely. – afic Jan 27 '18 at 18:25
@yano mistakenly did. Got it. – Achal Jan 27 '18 at 18:26
1

It is quite inefficient to `malloc` memory for a single (not an array) variable anyway. If you just use automatic variables, the allocation has no cost. – Weather Vane Jan 27 '18 at 18:27
@WeatherVane Yes it's not optimal solution as I just modified what OP asked. There can be simplest and efficient way for the same task. – Achal Jan 27 '18 at 18:29
I would still clean up with `free`,, although a couple of days ago I got into a bit of a discussion with an SO user with much higher rep than me who relies exclusively on the OS to cleanup.. so by their standards your code is fine. – yano Jan 27 '18 at 18:31

Terminal showing "segmentation fault(core dumped)" in C, Please check what is wrong in my code?

2 Answers2