0

Trying to implement the add function in dynamic list, recursively. The code is self-explanatory:

struct list {
    int value;
    list* next = NULL; // is this valid?
};

list head;

void add(list* cur, int value) {
    if (cur->next) {
        add(cur->next, value);
        return;
    }
    cur->next = (list*)(malloc(sizeof(list)));
    cur->next->value = value;
    cur->next->next = NULL; // withouth this line, the program errors out after two calls to the function add
}

int main() {
    for (int i = 0; i < 50; i++) 
        add(&head, i);
}

After seeing the debugger, I realized that calling malloc wasn't initiating "next" with NULL as specified in the defininition of the list struct.

Alpha_B
  • 3
  • 3
  • 4
    Don't use `malloc`. It just gives you memory, it doesn't create an object. Since it doesn't create an object, it doesn't perform any initialization you might expect. Use `std::make_unique()` and `std::unique_ptr next;` instead. – François Andrieux Dec 28 '22 at 16:07
  • This looks like C code, not C++. Also, what's your question? Is your question about the contents of memory you allocate? – Nicol Bolas Dec 28 '22 at 16:08
  • 3
    `malloc` doesn't know anything about C++ classes and can therefore not call constructors or default initializers – UnholySheep Dec 28 '22 at 16:08
  • 2
    `NULL` is a C backward compatibility macro in C++. In C++ use `nullptr` for null pointers. – Some programmer dude Dec 28 '22 at 16:08
  • 2
    And generally, every time you need to do a C-style cast in your C++ code (like that `(list*)` needed for the `malloc` call) you should take it as a sign that you're doing something wrong. – Some programmer dude Dec 28 '22 at 16:09
  • 3
    I am in the camp where I think recommending a smart pointer for a linked list means you've never tried it yourself. Because if you had, you wouldn't be recommending it. Raw pointers are still plenty fine in C++, and data structures are one of the arguments. – sweenish Dec 28 '22 at 16:13
  • Why all this manual memory management? Use containers and smart pointers. Don't manually `new`/`delete` (and certainly not `malloc`/`free`) in modern C++. – Jesper Juhl Dec 28 '22 at 16:14
  • *"The code is self-explanatory:"* -- even if that is the case, please add an explanation. Search engines have trouble analyzing code to see if its explanation matches a search term (and the point of this site is to have questions and answers that can be found by searches). – JaMiT Dec 28 '22 at 16:17
  • @sweenish "observing"/"non-owning" raw pointers are obviously fine. Raw *owning* pointers however are usually trouble. – Jesper Juhl Dec 28 '22 at 16:18
  • Debugger's the right tool and this is the right question to ask, but you should have looked it up in your textbook first. If you don't have a textbook, seriously consider getting one. C++ is too complicated a language to not have [a good book](https://stackoverflow.com/q/388242/4581301) leading you through at least the initial "See Dick run!" phases of learning the language. – user4581301 Dec 28 '22 at 16:19
  • 1
    *"calling malloc wasn't initiating"* -- right. That's even in the [documentation for malloc](https://en.cppreference.com/w/cpp/memory/c/malloc); it produces **uninitialized** storage. Quick allocation of memory with no attempt to clear out existing garbage has always been the purpose of `malloc`. Why did you expect something else? – JaMiT Dec 28 '22 at 16:20
  • 1
    As for lists and other container-like structures, my recommendation is always to keep it all separate: Make a separate list structure, a separate node structure, and separate data structure. It might be a little more work, but it will make your code much more flexible, easier to debug and test, and also easier to maintain. – Some programmer dude Dec 28 '22 at 16:21
  • 1
    @JesperJuhl Have you ever tried to implement a linked list with smart pointers? I have. It's better to just stick with raw pointers. You might get away with a singly linked list, but I can just about guarantee you'll be pulling your hair out with a doubly linked list. Unless you think it's fine to slather shared pointers all over code, that is. If you're just trying to explain why we have smart pointers, there was no need to tag me in it; I know what they're for. – sweenish Dec 28 '22 at 16:22
  • @sweenish Yes, I have. – Jesper Juhl Dec 28 '22 at 16:25
  • [What is a debugger and how can it help me diagnose problems?](https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems) – Jesper Juhl Dec 28 '22 at 16:25
  • 1
    `malloc` does not call constructors, it's a C function and was not designed to work with C++. `new` is a C++ operator and does call constructors. So prefer `new` over `malloc` in a C++ program. – john Dec 28 '22 at 16:31
  • Question is how did you get to a place where you are trying to use `malloc` in a C++ program? You clearly have some programming experience. Maybe time to revaluate your sources of information. – john Dec 28 '22 at 16:33
  • @JesperJuhl Raw owning pointers are fine if they are completely contained in a class whose purpose is to manage them. `std::unique_ptr` is an example of that exception. For a linked list, it would fall into that exception to the general rule. – François Andrieux Dec 28 '22 at 16:34
  • @FrançoisAndrieux Agreed, but "completely contained" can be a high bar to meet. – Jesper Juhl Dec 28 '22 at 16:35
  • 1
    The real kicker of the smart pointer linked list is it turns an iterative destruction process into a potentially stack-popping recursive one. Good learning experience, though. – user4581301 Dec 28 '22 at 16:37
  • @user4581301 In this case, it turns a non-existent destructive process into a stack-popping recursive one. – François Andrieux Dec 28 '22 at 16:39
  • 1
    Given that he's using recursion on the insertion side in this case, using recursion during destruction isn't quite as horrible a thing as you'd normally expect. – Jerry Coffin Dec 28 '22 at 16:43

1 Answers1

0

As noted in comments, malloc does not initialize anything. It simply grabs a chunk of memory big enough for a list struct. Now, that might mean that struct has next set to NULL, or it might not.

That is why you're having to explicitly initialize it to NULL as that prevents undefined behavior when accessing that member.

If you use new to handle dynamic memory allocation, then the next member is initialized to NULL.

    cur->next = new list;

Including this use of malloc, your code is very C-ish, and there are numerous improvements C++ will allow you to make.

Chris
  • 26,361
  • 5
  • 21
  • 42