CODEWITHSUNDEEP
c++vector
ClaudeF
ClaudeF

Reputation: 13

getting segfault when I return a vector of pointer to structure

I was trying to split a string and insert the tokens in a vector of pointers to structure as follows:

#include <iostream>
#include <vector>
#include <regex>
#include <sstream>

using namespace std;

struct A {
    std::string pMessage;
};

std::vector<A*> splitQuery(A a) {
  std::vector<A*> split_queries;

  std::stringstream        ss(a.pMessage);
  std::string              item;

  while (std::getline(ss, item, ',')) {
    A inputPacket = {item};
    std::cout << item << std::endl;
    split_queries.push_back(&inputPacket);
  }

  return split_queries;
}

int main()
{
    A a = {"Hello,there"};
    std::vector<A *> split_queries = splitQuery(a);
    std::cout << split_queries.size() << std::endl;
    for (auto &s : split_queries) {
        std::cout << "elements " << s->pMessage << std::endl;
    }

    return 0;
}

The problem is that I am getting a segfault and I don't why exactly. The split is working correctly. But I don't know what is wrong.

Hello                                                                                                                                                             
there                                                                                                                                                             
2                                                                                                                                                                 
Segmentation fault (core dumped)

Update and correction

I actually updated the code by using Uniquer_ptr now it is dynanically created so it's on heap right? and therefore it wont be destroyed after push_back.

#include <iostream>
#include <vector>
#include <regex>
#include <sstream>
#include <memory>

using namespace std;

struct A {
    std::string pMessage;
};

std::vector<std::unique_ptr<A>> splitQuery(A a) {
  std::vector<std::unique_ptr<A>> split_queries;

  std::stringstream        ss(a.pMessage);
  std::string              item;

  while (std::getline(ss, item, ',')) {
    std::cout << item << std::endl;
    split_queries.push_back(std::unique_ptr<A>(new A({item})));
  }

  return split_queries;
}

int main()
{
    A a = {"Hello,there"};
    std::vector<std::unique_ptr<A>> split_queries = splitQuery(a);
    std::cout << split_queries.size() << std::endl;
    for (auto &s : split_queries) {
        std::cout << "elements " << s->pMessage << std::endl;
    }

    return 0;
}

Upvotes: 0

Views: 94

Answers (3)

Sheric
Sheric

Reputation: 416

A inputPacket = {item};

Creates an instance of A in local scope of while, so it will be deleted when the scope of while ends. (i.e.: Once when each loop finishes.) Therefore, when you're using the pointers later, They are already deleted.

You have to manage the lifetime of variable yourself, by manually reserving memory for them:

A *inputPacket = new A({item});

and avoiding to use pointer to a local object. However, in this case you'll have to manage the end of lifetime yourself, too. It means you'll have to delete the pointers somewhere. A nice idea to automate this is using unique_ptr Define your return type as:

std::vector<std::unique_ptr<A> >

then create unique pointers and push them into the vector:

split_queries.push_back(std::make_unique<A>({item}));

This way you'll be safer working with pointers.

Upvotes: 1

Jean-Marie H
Jean-Marie H

Reputation: 47

Your A inputPacket is allocated on the stack (local variable). When exiting your function split_queries(), the A instance is destroyed (destructor called) (moreover your are using only one instance of A declared in the loop). => you must allocate your A instance on the heap (A* inputPacket = new A(item); and do not forget to deallocate it in the main or use a vector of A (or share_ptr) instead of a vector of A* (which will copy each instance in the vector)

Upvotes: 0

Bathsheba
Bathsheba

Reputation: 234725

Your problem boils down to your storing a pointer to a variable with automatic storage duration, and that variable is destroyed despite the pointer to it being retained. Then line

A inputPacket = {item};

declares such a type, and this goes out of scope after the push_back statement.

As such then the behaviour of your code is undefined.

A possible fix is to use std::vector<A> as the type, at the expense of some value copies. This would work since your A lends itself well to being copied. If that's not acceptable, then use a vector of a smart pointer type such as std::unique_ptr<A> with refactoring around the construction of each instance of A.

Upvotes: 2

Related Questions