c++: An int value in a struct is changing value for no apparent reason

Question

I'm trying to learn c++ after learning java. I've been told that the structs are a lot like classes with instance variable in java. The "whole" value in the fraction struct seems to randomly change from one to zero in the isDivisible function. Why is this happening? please don't spare the details. Thank you for your help

#include <stdio.h>
#include <math.h>
#include <iostream>
using namespace std;

#define LARGEST(a,b) ((a > b) ? 1 : 0)

struct fraction {
    int numerator;
    int denominator;
    int whole;
};

fraction* wholeNumber(fraction * frac){
    if (LARGEST((frac->numerator), (frac->denominator)) == 1){
        double w = (((double) (frac->numerator)) / ((double) (frac->denominator)));
        int whole = floor(w);
        struct fraction now = { (frac->numerator) - whole*(frac->denominator),
                (frac->denominator), whole};
        return &now;
    }

    return frac;
}

int toCheck[4] = {2,3,5,7}; 

int isDivisible(fraction * frac){
    double newNum = (double) frac->numerator;
    double newDen = (double) frac->denominator;
    // The value of whole seems to change right about here after the two doubles are declared
    for(int i = 0; i < 4; ++i){
        if (( (newNum / toCheck[i]) == (frac->numerator / toCheck[i]) ) ? true : false &&
                ((newDen / toCheck[i]) == (frac->numerator / toCheck[i]) ) ? true : false)
            return toCheck[i];
    }
    return 0;
}

fraction* simplifier(fraction * frac){
    frac = wholeNumber(frac);
    while(isDivisible(frac) != 0){
        int factor = isDivisible(frac);
        frac->numerator = frac->numerator / factor;
        frac->denominator = frac->denominator / factor;
    }

    return frac;
}

int main (int argc, char ** argv){
    struct fraction frac = {55, 50, 0}; // fraction declared, 0 55/50
    struct fraction * final = simplifier(&frac);

    printf("%d %d/%d\n", final->whole, final->numerator, final->denominator);

    return 0;
}

`if (LARGEST((frac->numerator), (frac->denominator)) == 1` - ***Whyyyy???*** I'm crying. Why not `if (frac->numerator > frac->denominator)`? Seriously! — , Aug 29 '13 at 17:54
I'm just trying to use different things in my code to see how it all works. This isn't meant to be used for anything other than my learning — user2726232, Aug 29 '13 at 17:56
@user2726232 It's horrible even for learning. If/while you are learning (and later too), your goal should be simplicity and readability, not being "clever". — , Aug 29 '13 at 17:56
@Dukeling (no real difference) but struct is perfectly appropriate for a structure with all public members. — Justin Meiners, Aug 29 '13 at 18:10
@Dukeling that makes little to no sense at all, since structs and classes are the same entities in C++. — R. Martinho Fernandes, Aug 29 '13 at 18:11
It's not a dupe at all. This is a "Debug my code" question. The other question can only be found if you know what you are talking about. — Puppy, Aug 29 '13 at 18:38

Carl Norum · Accepted Answer · 2013-08-29T18:00:22.067

5

Did you get this warning (or something like it) when compiling?

example.cpp:20:17: warning: address of stack memory associated with local
      variable 'now' returned [-Wreturn-stack-address]
        return &now;
                ^~~

Returning a pointer to a stack variable is bad news. If your compiler didn't warn you, you need to turn on some more warning flags (or get a better compiler - my quotation above comes from clang, which has pretty great error messages and is probably a good choice for a beginner because of that).

The long and short of your problem is that now goes out of scope when the function ends. That means accessing the pointer you return causes undefined behaviour.

You might want to use a lot fewer pointers in your program - if you're using C++ you have access to references, but even besides that, C and C++ allow structures to be passed around by value, which might make your life easier when learning.

edited Aug 29 '13 at 18:00

answered Aug 29 '13 at 17:54

Carl Norum

219,201
40
422
469

I didn't get any compiler warnings. I am very new to c and c++ and printf is just what I've seen used. – user2726232 Aug 29 '13 at 17:57
What compiler are you using ? You have to use `C++` IO streams for input/output operations. Avoid mixing `C, C++` code in a program. In this case, you clearly can. – Mahesh Aug 29 '13 at 17:59
To be honest, I'm not even sure, it came as a download from the online tutorial I am taking (I seem to be a bit in over my head). If I was to change compilers, would you have a recommendation? – user2726232 Aug 29 '13 at 18:05
I would recommend [clang](http://clang.llvm.org). – Carl Norum Aug 29 '13 at 18:10
@user2726232, Make sure your warnings are turned on as well (I like having at least `-Wall -Wextra -pedantic-errors`). – chris Aug 29 '13 at 18:16
Both GCC & clang warned for me with no special flags. – Carl Norum Aug 29 '13 at 18:17
All the above recommendations are great, but if you prefer to ignore the compiler till you get to grips with the language; use [codepad.org](http://codepad.org/4469d6Lu); ( as you can see, the compiler used by codepad gives you enough information to catch such errors, early ) – ϹοδεMεδιϲ Aug 29 '13 at 21:59

score 1 · Answer 2 · answered Aug 29 '13 at 17:54

1

wholeFunction returns address of a local variable.

 struct fraction now = { (frac->numerator) - whole*(frac->denominator),
            (frac->denominator), whole};
 return &now;

The above leads to undefined behavior.

answered Aug 29 '13 at 17:54

Mahesh

34,573
20
89
115

No, it doesn't return a reference. It returns a pointer. – Aug 29 '13 at 17:54

score 0 · Answer 3 · answered Aug 29 '13 at 18:20

C++ is a language that has value semantics and that gives the language a very different feel to Java or C# which have reference semantics. You can have references in C++ but they are explicit. There were a number of issues with your code and at the heart of the issues was this concept of value semantics. Rather than go through each individually I refactored your code to be more idiomatic C++. It's by no means perfect but it has removed your error and will hopefully provide you with some insights on how to do things in C++.

struct fraction 
{
    int numerator;
    int denominator;
    int whole;
};

fraction wholeNumber(const fraction& frac)
{
    if (frac.numerator > frac.denominator)
    {
        double w = ((double)frac.numerator) / ((double)frac.denominator);
        int whole = floor(w);
        fraction now = { frac.numerator - whole * frac.denominator, frac.denominator, whole};

        return now;
    }

    return frac;
}

int toCheck[4] = {2,3,5,7}; 

int isDivisible(const fraction& frac)
{
    double newNum = (double) frac.numerator;
    double newDen = (double) frac.denominator;

    for(int i = 0; i < 4; ++i)
    {
        if ( (newNum / toCheck[i]) == (frac.numerator / toCheck[i]) &&
                (newDen / toCheck[i]) == (frac.numerator / toCheck[i]))
            return toCheck[i];
    }
    return 0;
}

fraction simplifier(const fraction& frac)
{
    fraction new_frac = wholeNumber(frac);
    int factor = isDivisible(new_frac);
    while(factor)
    {
        new_frac.numerator = new_frac.numerator / factor;
        new_frac.denominator = new_frac.denominator / factor;
        int factor = isDivisible(new_frac);
    }

    return new_frac;
}

int main (int argc, char ** argv){
    struct fraction frac = {55, 50, 0}; // fraction declared, 0 55/50
    struct fraction final = simplifier(frac);

    std::cout << final.whole << " " << final.numerator << "/" << final.denominator;

    return 0;
}

c++: An int value in a struct is changing value for no apparent reason

3 Answers3