class constructor building struct at the same memory location on every instantiation but other members in their own memory locations?

Question

I have a class that when instantiated I build two standard struct members. my instantiation is independent and is in its own memory space every time I build one as is the name member, but the struct's built in the constructor are being given the same memory address every time and thus write over the previous instantiations struct? The pointer nor the struct is static any where so I don't see why this is occurring. If any one can spot my error please point it out.

-------------signal.h--------------
prama once

struct GenParams
{
 float a;
 float b;
};


class signal {
public:
signal();
~signal();
std::string sigName;
GenParams* mGP;  //not static so every instance should have its own mGP I set to private as well but made no difference.
};
--------------------siganl.cpp------------------

#include signal.h

signal::signal()
{  
   GenParams GP; //every instance should have its own member GP at a unique mem location
    // but instead GP is created at the same mem location
   mGP = &GP;  // mGP points to the same mem location every time the constructor is called

   //fill struct with default values
   GP.a = 0.0;
   GP.b = 5.0;
}
signal::~signal()
{ }

--------------------interface.h--------------------
#include signal.h

class interface
{
public:
interface();
~interface();

std::string interName;
std::vector<signal*> sigList;
interface* iPtr;
unsigned int sigNum;

void newSignal();

--------------------------------interface.cpp----------------------

#include interface.h

interface::interface() : iPtr(this)
{  sigNum = 0;}

interface::~interface()
{
  for (auto i : sigList)
      delete i;
}

interface::newSignal()
{
   signal* s = new signal;
 //after this line returns the values are stored correctly in memory of struct

//during this line which does not reference the struct or the signal yet I see the structs
// memory get deallocated and go red as if something is writing to it and the values are gone
   std::string newSName = this->interName + "sig" std::to_string(sigNum + 1);

//Here some values go back into the struct but they are bogus vals
   s->sigName = newSName;

   sigList.push_back(s);
//after adding to the list the sig instance has a name correctly written by the line above
//and a pointer it it but the pointer to the struct member mGP all instances point to the 
//same memory location?? I don't understand how this is happening.
   sigNum++;
  
}

If you want every object to have its own `GenParams` then change `GenParams* mGP;` to `GenParams mGP;` so the class has its own object, not a pointer to one. — NathanOliver, Aug 11 '22 at 15:57
Don't use pointers, especially good advice when you have a shaky understanding of them. There is no obvious reason why `mGP` has to be a pointer, and the problem described would go away if it were not a pointer. So change it to be not a pointer, and solve any remaining problems after that. — john, Aug 11 '22 at 16:04
@NathanOliver Ok I will take your advice thanks for the quick responses. so no pointer ok then do you recommend writing tons of getXmember functions in order to reference those values from other files and objects because thats why I was using the pointers so I could point to one before an instance was created. — MacLCM, Aug 11 '22 at 17:37

Vlad from Moscow · Accepted Answer · 2022-08-11T16:13:49.663

5

This constructor invokes undefined behavior

signal::signal()
{  
   GenParams GP; //every instance should have its own member GP at a unique mem location
    // but instead GP is created at the same mem location
   mGP = &GP;  // mGP points to the same mem location every time the constructor is called

   //fill struct with default values
   GP.a = 0.0;
   GP.b = 5.0;
}

because the data member mGP

mGP = &GP;

is set to the address of the local variable GP with automatic storage diration

GenParams GP;

that will not be alive after exiting the constructor.

So the data member mGP will have an invalid value.

You should to allocate an object of the type GenParams dynamically or to have a data member of this type.

Pay attention to that in this line

 std::string newSName = this->interName + "sig" std::to_string(sigNum + 1);

there is a typo. It seems you mean

 std::string newSName = this->interName + "sig" + std::to_string(sigNum + 1);

edited Aug 11 '22 at 16:13

answered Aug 11 '22 at 16:00

Vlad from Moscow

301,070
26
186
335

yes there was a typo in that naming line, not present in real code. But thank you for the response I see your point about the structs duration, weird to me that it kept using the same exact mem address though. – MacLCM Aug 11 '22 at 17:43
also I did not create the structs as class members because some of the struct's were going to be instantiated multiple times, so GenParams would only ever be once and that could be a class member but lets say struct e could be added 4 or 5 times, so class member didn't make since to me. in this case should I just keep a vector of the struct's belonging to the instance? – MacLCM Aug 11 '22 at 17:51
@MacLCM As an alternative you could have a vector. – Vlad from Moscow Aug 11 '22 at 18:00

class constructor building struct at the same memory location on every instantiation but other members in their own memory locations?

1 Answers1