villianNoob
villianNoob

Reputation: 31

Floating point exception (core dumped) when calculating GCD

I'm writing a program to reduce a fraction to the lowest term. Here is my program:

#include <stdio.h>
int main(){
    int m,x,n,gcd;
    printf("Enter a fraction: ");
    scanf("%d/%d",&n,&m);

    if(m==0)printf("Error");
    else
        for(;;){
           x=m%n;
           if(x==0){
              gcd=n;
              m/=gcd;
              n/=gcd;
              printf("In lowest terms: %d/%d",n,m);
            }else
                m=n;
                n=x;
         }

    return 0;
}

I use Euclid's algorithm to calculate the GCD. When executed, it reports

Floating point exception (core dumped)

What's wrong with my code?

Upvotes: 3

Views: 412

Answers (2)

villianNoob
villianNoob

Reputation: 31

@paddy Yes, I dint't see it through. Anyway my code has been flawed from the beginning. I use for(;;) to create loop but I tell it to stop at else. It is completely flawed because the loop will execute again and create an infinte loop. Additional, I intended to assign gcd to n, but before that n was assigned to x (that case,after else, the loop executed). Therefore if x is 0 then m/=gcd and n/=gcd may have caused core dumped. This is a stupid code that I shouldn't have asked. So I thought again and rewrote the code.

#include <stdio.h>
    int main(){
    int m,x,n,gcd;
    int M,N;

    printf("Enter a fraction: ");
    scanf("%d/%d",&n,&m);
    M = m;
    N = n;

     if(m==0)printf("Error");
        else
        do{
           x=m%n;
           m=n;
           n=x;
             }while(x!=0);
    gcd=m;
    M/=gcd;
    N/=gcd;

    printf("In lowest terms: %d/%d\n",N,M);

    return 0;
}

Because after the loop, m and n has been modified so m/=gcd and n/=gcd can't return accurate result. So I assign n to N,m to M so that the value I entered won't change. Assigning gcd to m is a bit tricky, but it's not wrong because before that m was assign to n. so I think this trick would have same effect. I use do while loop because I can set the "termination point", so there will be no infinite loop.

Upvotes: 0

paddy
paddy

Reputation: 63481

You have fallen victim to block scope here:

if(x==0){
    // ...
}else
    m=n;
    n=x;

This will be executed as:

if(x==0){
    // ...
}else{
    m=n;
}
n=x;

That means when x is zero, then n gets set to zero at the end of the loop. When you come around again to calculate m % n, you have a problem. Or if for some reason that gives a value without crashing your program, then gcd = n will end up giving you divide-by-zero when you subsequently divide m and n by gcd.

If you want more than one statement to be part of the else branch, then you need to enclose them in braces as follows:

if(x==0){
    // ...
}else{
    m=n;
    n=x;
}

In fact, it's good practice to always use the braces, even when you only have one statement. At my workplace this is even in our coding standards, and part of the code review process.

Upvotes: 5

Related Questions