2

I'm trying to port over some code that uses pointers, to a different location with the different sections of the code in separate functions. However, I keep getting undefined behaviour from the pointers and was not able to understand why.

A sample of how my code is currently is as follows. I am using TAEF

In TestClass.h

class TestClass{
   CurrentData* data1;
   CurrentData* data2;
};

In TestClass.cpp

// initialize pointer
TestClass::MethodSetup()
{
    data1 = new CurrentData();
    data2 = new CurrentData();
}

// actual test function
TestClass::Test1()
{
   SetupData();
   Verify(data1);
   Verify(data2);
}

// supplementary functions
TestClass::SetupData()
{
   SetupStep(&data1);
   SetupStep(&data2);
}

TestClass::SetupStep(CurrentData** p_Data)
{
   CurrentDataRequest req;
   csref<CurrentDataResponse> resp;
   // send request and get response
   *p_Data = (*resp)->m_Data;
}

TestClass::Verify(CurrentData* cd)
{
   VERIFY_IS_EQUAL(cd->val, 0);   
}

// cleanup
TestClass::MethodCleanup()
{
   delete data1;
   delete data2;
}

I have only made the wrappers SetupData() and Verify(), not the code inside them. SetupStep() is auxiliary both here and in the original. In the original version of the code, the code blocks inside each wrapper would execute in the same scope with a single pointer CurrentData* cd defined before testing begins. I expected that my code would behave identically to the original code and pass tests as expected, but I observed that the value of members of the cd pointer is pointing to change unexpectedly i.e. undefined behaviour.

Another problem is I want to keep the pointers as is as there are multiple data members to maintain and with unique pointers, it seems not recommended to pass them to functions as references, as seen in a highly upvoted comment to the first reply here

Borisonekenobi
  • 469
  • 4
  • 15
Ian Storm
  • 21
  • 1
  • Please try to create a [mre] to show us. – Some programmer dude Aug 08 '23 at 13:40
  • 2
    As for your problem, what happens with `resp` when `SetupStep` returns? More specifically, what happens with `(*resp)->m_Data`? Will the data that the `m_Data` is pointing to still stay valid? Will it be destructed? Will the saved pointers still be pointing to valid objects? – Some programmer dude Aug 08 '23 at 13:43
  • I'm sorry about the example, it's hard for me to create a minimal reproducible example as this is production code with lot of nested helper functions and servers. I have checked the m_Data and data1/data2 locally inside SetupStep. They match immediately after assignment, but beyond the function SetupStep, specifically in Verify, and occasionally inside Setup as well, the value of data1/data2 changes to something random. So, I thought I did something wrong with using the pointers in this way for undefined behavior. – Ian Storm Aug 08 '23 at 13:54
  • What is `csref`? What is `CurrentDataResponse`? What does the `CurrentDataResponse` destructor do? – Some programmer dude Aug 08 '23 at 13:58
  • Since I can't copy-and-paste compile-and-run your code, I can only suggest: **never** use raw pointers as owning pointers. For *dynamic* objects, always use a smart pointer (e.g., `std::unique_ptr`) to track/transfer *ownership*. Consider `std::shared_ptr` only as a last resort. Careful distinguish between **value** types, **entity** types, and **service** types. – Eljay Aug 08 '23 at 14:05
  • *...not recommended to pass unique_ptr to functions as references...* Correct. Either `void foo(std::unique_ptr);` to explicitly pass ownership, or `void foo(Quux const&);` to reference the object without regard to whether it is owned by a unique_ptr, or a vector, or a member variable, or an automatic (stack) variable, or a static variable. Only do `void foo(Quux const*);` if the parameter could be null, and it is **not** transferring ownership. – Eljay Aug 08 '23 at 14:13
  • There are zero [member pointers](https://en.cppreference.com/w/cpp/language/pointer#Pointers_to_members) in the entire snippet. The motivation behind most of the code is unclear. (1) `new CurrentData();`: Why raw pointers? They are not needed here. I would stick with RAII principles and keep `CurrentData` or `std::unique_ptr` as `TestClass` members. (2) `*p_Data = (*resp)->m_Data;` Here you assign the `*p_Data` pointer (pointing at `data1` or `data2`) to a pointer taken from `csref`. Will it stay valid after `csref` goes out of scope? – Andrej Podzimek Aug 13 '23 at 22:25

0 Answers0