Passing pointers through sc_fifo - issue when utilizing nb_read()

[Bryan]

I have googled and searched this forum, but I cannot find a hint yet to tell me what I'm doing wrong here. It is likely my lack of depth on handling references vs. pointers, but I haven't found a combination which will work.

In my model, I dynamically allocate an instance of a custom class in a producer function and write the pointer of that instance into an sc_fifo. In my consumer function, I have been using the sc_fifo_in_if read() function to pop the FIFO entry into a local variable (which is declared as a pointer to my custom class). The read() function is working just fine.

I am creating a new model for which I've found that using an nb_read() would work better (and a follow-on nb_write(), but I cannot get that far). But when I attempt to run this code, I run into a seg-fault as soon as I call the nb_read() function.

I don't know if there is a C++ limitation or perhaps my need to cast my pointer variable, but since the nb_read() function is utilizing a reference for the variable, I haven't been able to make any casting work.

Here is very simplistic pseudo-code of what I want to do:

class MyClass { 
  // define my class
};

sc_fifo<MyClass*>	fifo;

// in producer function
MyClass*	pItem = new MyClass();

fifo.write(pItem);

// In consumer function
MyClass* 	pOtherItem=NULL;

pOtherItem = fifo.read();	// works fine
fifo.nb_read(pOtherItem);	// seg-fault

 

I am doing this ultimately because the MyClass item is relatively large, thus copying those around through my model causes a performance hit. Passing around only a pointer to that item (and dynamically creating / destroying it as needed) allows faster runtime.

Am I missing something simple? Am I trying to do something not possible? Am I getting lucky (ie: not doing something correctly) when I just use the blocking read() function?

Appreciate any guidance here!

[David Black]

nb_read will return false if the fifo is empty and not update pOtherItem. You need to use blocking read for your case. The SystemC PoC should add [[nodiscard]] to the nb_ function calls, which would at least prevent this code from compiling without warning.

Suggestion, replace NULL with nullptr as well. nullptr was introduced into C++11 (12 years ago) and is technically superior to NULL. It is not your bug in this case, but using Modern C++ will help later.

 

 

[Bryan]

Thanx @David Black. While my pseudocode above did not show it, I do check the return value of the nb_read() function in my full code before attempting to use the pointer. Or so I intended... Now I get to fall on my sword.

Here is pseudocode of what I was actually doing:

// In consumer function
MyClass* 	pOtherItem=NULL;

if (fifo.nb_read(pOtherItem)) { 
  // process the data available at pOtherItem
}

delete pOtherItem;

The problem was the delete statement needed to be inside of the if protection. It was my intention for that, but I accidentally closed the if before the delete. And unfortunately the debugging dumps I added didn't catch that.

This is the second time I have hit an issue when replacing read() with nb_read(). It turns out that this time it was definitely my fault as shown above. The last time I could not determine the reason for the seg-fault. I was likely too quick to blame the nb_read() based on that. A few hours after posting for help above, I finally found the unprotected delete after adding many more debugging dumps to my code.

Thank you again for the help though! And I will look into replacing my NULL pointers as suggested.

[David Black]

With modern C++, a cleaner way to deal with pointers is to avoid using new & delete altogether. Instead, you need to consider ownership and use std::unique_ptr<T> or std::shared_ptr<T> as appropriate. The corresponding make_unique<T> and make_shared<T> mechanisms are also safer. Proper use of these will ensure that pointers automatically destroyed without double deletions. For more information see:

• https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rr-newdelete
•  https://en.cppreference.com/w/cpp/memory/unique_ptr/make_unique 

[Bryan]

@David Black I will look into this and attempt to learn. Since I have the model working now, it may be a significant change to go through and update to that newer style. However, if I have to make significant changes for some reason, it could make sense to do so.

Thank you again.