Member Avatar for MarounMaroun

Hello folks,
I have this header file:

class Agent {
private:

    struct AgentStruct {
        std::string agentName;
        double pID;
        double mID;
        AgentStruct *nextAgent;
    } *AgentP;

public:

    Agent(std::string);
    void SetNextAgent(AgentStruct*);
    Agent* GetNextAgent();
    void SendMessage();
    void ReceiveMessage();

};

and I have the implementation here:

/*
 * The constructor we are required to implement.
 */
Agent::Agent(std::string name) {
      AgentP->nextAgent=NULL;
      AgentP->agentName=name;
      AgentP->pID=setPID();
      AgentP->mID=AgentP->pID;
}

I get a segmentation fault.
Any ideas?

thanks!

  • 4 Contributors
  • 5 Replies
  • 191 Views
  • 13 Hours Discussion Span
  • Latest Post Latest Post by mike_2000_17

Dani AI

Generated

The crash came from dereferencing an uninitialized pointer: AgentP was declared but not owning a valid AgentStruct, so any AgentP->... access hit garbage. 's allocation fixes the immediate segfault; 's suggestion to keep the struct as a value member eliminates heap management entirely; 's prompt ("why pointers?") is the right design question — prefer value semantics or standard containers unless a pointer-based layout is required.

If the heap is used, ownership must be handled correctly (Rule of Three/Five). With a raw pointer, implement destructor and safe copy/assignment to avoid leaks and double-free. Example (copy-and-swap idiom for clarity):

~Agent() { delete AgentP; }

Agent::Agent(const Agent& other)
  : AgentP(other.AgentP ? new AgentStruct(*other.AgentP) : nullptr)
{}

Agent& Agent::operator=(Agent rhs) // copy-by-value then swap
{
  std::swap(AgentP, rhs.AgentP);
  return *this;
}

Better modern approach: prefer RAII. Use std::unique_ptr (or std::shared_ptr if shared ownership is needed) and fix the API/type mismatch in the thread: AgentStruct::nextAgent is a AgentStruct* while GetNextAgent() returns Agent* — make those consistent (either have nextAgent be Agent* and return Agent*, or expose AgentStruct* throughout). Example sketch using unique_ptr:

std::unique_ptr<AgentStruct> AgentP; // member

Agent::Agent(const std::string& name)
  : AgentP(std::make_unique<AgentStruct>())
{
  AgentP->agentName = name;
  AgentP->nextAgent = nullptr;
}

Other practical notes: prefer const std::string& in the constructor to avoid extra copies; consider using std::list/std::vector or an intrusive list of Agent objects instead of hand-rolled pointers; confirm setPID() is defined and initialized before use; debug memory issues with tools like Valgrind or AddressSanitizer if the crash persists.

Recommended Answers

All 5 Replies

Member Avatar for LordNemrod

You have never allocated memory for AgentP member.
This should fix the problem

Agent::Agent(std::string name)
:AgentP(new AgentStruct())
 {
      AgentP->nextAgent=NULL;
      AgentP->agentName=name;
      AgentP->pID=setPID();
      AgentP->mID=AgentP->pID;
}
Member Avatar for MarounMaroun

Thanks a lot! :)

Member Avatar for mrnutty

Why are you using pointers?

Member Avatar for MarounMaroun

how can I do it without pointers?

Member Avatar for mike_2000_17

>>how can I do it without pointers?

Like this:

class Agent {
private:

    struct AgentStruct {
        std::string agentName;
        double pID;
        double mID;
        AgentStruct *nextAgent;
    } AgentP; //notice no * star sign.

public:

    Agent(std::string);
    void SetNextAgent(const AgentStruct&); //notice pass-by-reference
    Agent* GetNextAgent();
    void SendMessage();
    void ReceiveMessage();

};

//...
/*
 * The constructor we are required to implement.
 */
Agent::Agent(std::string name) { //no need to create AgentP
      AgentP.nextAgent=NULL; //notice dot instead of arrow.
      AgentP.agentName=name; //in C++0x, this could also all be in the initialization list instead.
      AgentP.pID=setPID();
      AgentP.mID=AgentP.pID;
}

Of course, if there is another reason (that you didn't mention) why you need to use a pointer for AgentP, it is fine to do it, but remember to delete it in the destructor of the Agent class (or better, use a smart pointer).

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.