0

Originally, my lab was passing three argument: addFractionJesseR(*lFrac, *rFrac, **resFrac); but I just found out I can't pass three arguments. I had to change it to **resFrac = addFractionJesseR(*lFrac, *rFrac); and now I'm having problems compiling. I know my pointers and double pointers are out of scope somewhere but I just can't spot where. the debugger points to the second line as the problem:

FractionJesseR& FractionJesseR::operator=(const FractionJesseR& arg) {
  num = arg.num;
  denom = arg.denom;
  return *this;
}

which is called by:

FractionJesseR& addMenu(FractionJesseR* lFrac, FractionJesseR* rFrac) {
  int option;
  FractionJesseR** resFrac = new FractionJesseR*();
......
    case 2:
      cout << "Calling add() --\n\n";
      **resFrac = addFractionJesseR(*lFrac, *rFrac);
      break;
    ......

**resFrac = addFractionJesseR(*lFrac, *rFrac); was originally addFractionJesseR(*lFrac, *rFrac, **resFrac);

which is called by:

void displayMenu() {
  int option;
  FractionJesseR *lFrac = nullptr;
  FractionJesseR *rFrac = nullptr;
  FractionJesseR *resFrac = nullptr;
......
    case 2:
      cout << "  Adding Option --\n\n";
      if (lFrac == nullptr && rFrac == nullptr) {
        cout << "    Not a proper call as no Fractions are available!\n\n";
      }
      else {
        *resFrac = addMenu(lFrac, rFrac);
      }
      break;

*resFrac = addMenu(lFrac, rFrac) was originally addMenu(lFrac, rFrac, &resFrac)

(yes, I did call delete on all my pointers, I'm still new to Stack Overflow and learning to only put up relevant snippets of code) I need help pointing me in the right direction. I think my pointers go out of scope somewhere in addMenu or displayMenu... maybe I'm dereferencing a double pointer wrong?

Any help would be greatly appreciated!

edit:

FractionJesseR& addFractionJesseR(FractionJesseR& lFrac, FractionJesseR& rFrac) {
  int n = 0;
  int d = 0;
  FractionJesseR *resFrac = nullptr;

 // Adding the fractions
 n = (&lFrac)->getNum() * (&rFrac)->getDenom() + (&lFrac)->getDenom() *
    (&rFrac)->getNum();
  d = (&lFrac)->getDenom() * (&rFrac)->getDenom();

  resFrac = new FractionJesseR(n / gcd(n, d), d / gcd(n, d));

  if (d < 0) {
    d = -d;
    n = -n;
  }

  return *resFrac;
}
JRags
  • 13
  • 5
  • You have problems with compiling, but your debugger is telling you something? I got lost. – luk32 May 11 '15 at 01:01
  • when I try to add two fractions, the program crashes. this points to null and the debugger is unable to read memory for num and denom. I think I'm not initializing resFrac as a double pointer correctly. – JRags May 11 '15 at 01:06
  • So it does compile? Problems with compiling can be 1. warnings, 2. errors in which case there is nothing to debug. If you get warnings you should straight them out as well, they are often important things. Especially when it comes to pointers. – luk32 May 11 '15 at 01:10
  • The source or at least the definition of `addFractionJesseR()` would be helpful. – uesp May 11 '15 at 01:11
  • @luk32 I'm able to run it, initialize my fractions and print them out fine. when I go to add, it crashes. no warnings show up. – JRags May 11 '15 at 01:17
  • @uesp I added addFractionJesseR() for you – JRags May 11 '15 at 01:17
  • @JRags Umm... probably not relevant, but why are you doing `(&lFrac)->` O_ó ? Generally your code is a great example, why c++ people put so much pressure to avoid using pointers... they don't seem to be needed in your code and you most probably ran into problem due to bad resource management =/ – luk32 May 11 '15 at 01:18
  • @luk32 hmm... it's what my professor did in class - I simply used his logic for the addition part and expanded on that for subtract, multiply and divide... I'm new-ish to c++! – JRags May 11 '15 at 01:22
  • I suggest you click on the "double-pointer" tag and read the tag wiki. – Keith Thompson May 11 '15 at 01:23
  • Apologies, thanks @KeithThompson – JRags May 11 '15 at 01:25
  • @JRags Ok, my knowledge doesn't reach far enough to explain precisely what is going on, but maybe some expert will be able if this is right or wrong. IMO the problematic part is dereferencing the pointer and returning the object via reference. It is extremenly bizzare and I doubt it is legal, but I am not sure =). So maybe that is the answer or someone will debunk it. – luk32 May 11 '15 at 01:33
  • Aw, and I was hoping you knew it all! No worries, thanks for your help though. I know this can be done with single pointers but professor insists on double pointers. I'm still struggling with it! – JRags May 11 '15 at 01:36
  • @JRags Without seeing all of your source code, I can not tell when and where you are deleting your pointers and if you are trying to access memory after its been deleted or if there are errors in other places. I am not able to run your code sample within my own debugger to be able to try and help you. – Francis Cugler May 11 '15 at 02:15
  • there are no other errors in place. @isanae has the right idea of what my problem is. The source code is a fairly long and consists of 5 files. How would I put all that up here? – JRags May 13 '15 at 01:10

1 Answers1

1

You're having issues with memory management.

I assume you are using some version of Visual Studio. The debugger will usually mark the next line to be executed, so you're getting a crash on

num = arg.num;

This happens because this is null, since

FractionJesseR** resFrac = new FractionJesseR*();

allocates a pointer on the free store (which is unusual) and initializes it with 0 (because of the brackets). This:

**resFrac

first dereferences resFrac, giving a null pointer, which you then dereference again. Dereferencing a null pointer is undefined behaviour. In your case, it causes a crash in the assignment operator on the first statement.

The obvious solution is to stop using pointers and manual memory management. At best, use objects:

FractionJesseR resFrac
// ...
resFrac = addFractionJesseR(*lFrac, *rFrac);

At worst, use smart pointers:

auto resFrac = std::make_unique<FractionJesseR>();
// ...
*resFrac = addFractionJesseR(*lFrac, *rFrac);

What you don't want to do (unless you are forced to because this is an assignment, in which case I'd question the motivation):

auto resFrac = new FractionJesseR;
// ...
*resFrac = addFractionJesseR(*lFrac, *rFrac);
// ...
delete resFrac;

addFractionJesseR() is returning a reference to a value that was allocated on the free store. Where are you going to delete it? Every new must be matched with a delete.

If you really want an example of manual memory management, you should not mix pointers and references:

FractionJesseR* addFractionJesseR(FractionJesseR* a, FractionJesseR* b)
{
    auto n = a->getNum() * b->getDenom() + a->getDenom() * b->getNum();
    auto d = a->getDenom() * b->getDenom();

    if (d < 0)
    {
        d = -d;
        n = -n;
    }

    return new FractionJesseR(n / gcd(n, d), d / gcd(n, d));
}

FractionJesseR* addMenu(FractionJesseR* a, FractionJesseR* b)
{
    // ...
    FractionJesseR* resFrac = addFractionJesseR(a, b);
    // ...
    return resFrac;
}

void displayMenu()
{
    // ...
    FractionJesseR *resFrac = addMenu(lFrac, rFrac);
    // ...
    delete resFrac;
}

Notice that the pointer allocated in addFractionJesseR() is returned to addMenu(), which returns it to displayMenu(), which deletes it.

isanae
  • 3,253
  • 1
  • 22
  • 47
  • Many thanks!!! However, professor doesn't allow for anything outside of what he teaches. We're expected to do manual memory management. So, my mistake was in: FractionJesseR** resFrac = new FractionJesseR(n, d); ? or num, denom? – JRags May 13 '15 at 01:04
  • @JRags Without seeing more code, my guess is that this should be `auto resFrac = new FractionJesseR;`, followed by `*resFrac = addFractionJesseR(...);`. Don't forget to delete it! – isanae May 13 '15 at 02:56