Home > Enterprise >  Memory leak in C (Valgrind)
Memory leak in C (Valgrind)

Time:10-10

I implement the stack with a minimum. In this program, I get an error from valgrind. Something is wrong with the push() and main() functions. When I add delete st; to the push() function, I get even more errors. I check it through valgrind ./a.out. Sorry for the long code. I also wrote the rest of the functions for stack. But there is no error in them, I left those in the code where there may be an error.

#include <cstring>
#include <iostream>

struct Stack {
  int data;
  int min;
  Stack* next;
};

void Push(Stack** top, int n) {
  Stack* st = new Stack();
  st->data = n;
  if (*top == NULL) {
    *top = st;
    (**top).min = n;
  } else {
    st->min = ((n <= (**top).min) ? n : (**top).min);
    st->next = *top;
    *top = st;
  }
  std::cout << "ok" << std::endl;
}

void Pop(Stack** top) {
  if (*top != NULL) {
    std::cout << (**top).data << std::endl;
    *top = (*top)->next;
  } else {
    std::cout << "error" << std::endl;
  }
}

int main() {
  Stack* top = nullptr;
  int m;
  std::cin >> m;
  std::string str;
  for (int i = 0; i < m;   i) {
    std::cin >> str;
    if (str == "push") {
      int value;
      std::cin >> value;
      Push(&top, value);
    }
    if (str == "pop") {
      Pop(&top);
    }
  }
  delete top;
}

CodePudding user response:

When you just delete top, you destruct it (in your case it's nothing, but you can distract yourself for reading about destructors if interested) and free the dynamic memory allocated for top. However, you actually want to also delete top->next, top->next->next (if present) etc. A hotfix:

while (top) { // same as "while (top != nullptr) {"
    Stack* next = top->next; // we can't use `top` after we `delete` it, save `next` beforehand
    delete top;
    top = next;
}

Now, about more general things. The course teaches you some really old C (almost just plain C; even C here is bad though). At the very least, your whole Push() can be replaced (thanks to lvalue references (Type&), std::min and aggregate initialization) with:

void push(Stack*& top, int n) {
    top = new Stack{n, std::min(n, top ? top->min : n), top};
    std::cout << "ok\n";
}

I'm new to C programming. I used to write in Python

Good job. Sadly, such teaching shows C as something too old and horrifying.

Edit

here's a new in Push, so there should most likely be a delete in Pop

That's right (thanks to @molbdnilo). You should delete popped elements instead of just leaking them.

  • Related