Trouble understanding the memory allocation of two seperate struct pointers of same type. Works in one example but not the other

Question

I'm having real difficulty understanding what is going on with memory allocation two specific scenarios that are producing different result but functionally appear identical to me when applying my naive wisdom. I created a simplified anologous code example below to demonstrate the issue I am encountering. I appreciate that that data is being passed to functions in an overaly complex way but please assume it is necassary unless fundamentally flawed.

In the first section of comment code. This works, no issue. In the second section of the commented code, the terminal flashes for a moment and the drops out with no errors or warnings.

I'd really appreciate an understanding of why this is occuring. Clearly I still have much to learn!

My thanks in advance,

Matminster.

#include <iostream>
#include <list>

struct item{
    int item_ID;
    std::list<int> related_item_IDs;
};

struct item * return_item_details(std::list<item> list_of_items, int item_ID) {
    
    for(std::list<item>::iterator it = list_of_items.begin(); it != list_of_items.end(); it++) {
        if(it->item_ID == item_ID) {
            return & * it;
        }
    }
    return NULL;
}

int find_shared_related_items(int item_1_ID,int item_2_ID, std::list<item> list_of_items) {
    int total_shared_items=0;
    
    //This works_________________________________________________________________
//    struct item * item_1_pointer = return_item_details(list_of_items, item_1_ID);
//  struct item * item_2_pointer = return_item_details(list_of_items, item_2_ID);
    //___________________________________________________________________________
    
    //But not this?________________________________________________________________
//  struct item * item_1_pointer = (struct item * ) malloc(sizeof(struct item * ));
//  item_1_pointer = return_item_details(list_of_items, item_1_ID);
//  struct item * item_2_pointer = (struct item * ) malloc(sizeof(struct item * ));
//  item_2_pointer = return_item_details(list_of_items, item_2_ID);
//  //_____________________________________________________________________________

    for(std::list<int>::iterator it = item_1_pointer->related_item_IDs.begin(); it != item_1_pointer->related_item_IDs.end(); it++) {
        for(std::list<int>::iterator it2 = item_2_pointer->related_item_IDs.begin(); it2 != item_2_pointer->related_item_IDs.end(); it2++) {
            if( * it == * it2) {
                total_shared_items++;
            }
        }
    }
    
    return total_shared_items;
}


int main() {
    
    item item_1; 
    item_1.item_ID = 1;
    
    item_1.related_item_IDs.push_back(3);
    item_1.related_item_IDs.push_back(4);
    item_1.related_item_IDs.push_back(5);
    item_1.related_item_IDs.push_back(6);
    
    item item_2; 
    item_2.item_ID = 2;
    
    item_2.related_item_IDs.push_back(5);
    item_2.related_item_IDs.push_back(6);
    item_2.related_item_IDs.push_back(7);
    item_2.related_item_IDs.push_back(8);
    
    std::list<item> list_of_items;
    list_of_items.push_back(item_1);
    list_of_items.push_back(item_2);
    
    std::cout << "The total number of shared items between item " << item_1.item_ID << " and item " << item_2.item_ID << " is: " << find_shared_related_items(item_1.item_ID, item_2.item_ID, list_of_items) << "\n";
    
    return 0;
}

Answer 1

The first version works by sheer misfortune. It's an Undefined Behaviour.

In return_item_details you accept std::list by value, which means the list and all its elements are copied and will be destroyed once the function finishes. You then return address of one of those temporary elements and try to use it (but the element doesn't exist anymore).

You can change the parameter to be passed by reference:

struct item * return_item_details(std::list<item>& list_of_items, int item_ID) {
//                                ~~~~~~~~~~~~~~~^

Or use std::find_if from <algorithm> library instead:

auto item_1_iter = std::find_if(list_of_items.begin(), list_of_items.end(), 
                                [item_1_ID](auto& listElem){return listElem.item_ID == item_1_ID;});

---

Couple of extra notes:

1. Your second version did the same thing as the first, but it added a memory leak. You call malloc and immediately overwrite the pointer to the newly allocated memory, losing it.
2. Don't use malloc in C++ (unless you are ready to use placement new afterwards). The C++ way to allocate memory is using new and delete.
3. Avoid using new and delete in modern C++. We have smart pointers and containers to do that for us. In C++11 there are few places where you need new, in C++14 it's already a bit of a bad smell.
4. struct item * is a C-ism. You don't need to repeat struct keyword in C++, item* item_1_pointer = nullptr; will work just fine.
5. auto is a very handy mechanism introduced in C++11, it may allow you to skip typing out std::list<item>::iterator (and any other type than compiler can deduce from context) explicitly:

for(auto it = item_1_pointer->related_item_IDs.begin(); it != item_1_pointer->related_item_IDs.end(); it++) {
    for(auto it2 = item_2_pointer->related_item_IDs.begin(); it2 != item_2_pointer->related_item_IDs.end(); it2++) {
        if( * it == * it2) {
            total_shared_items++;
        }
    }