-3
typedef struct _route_timer_info_t {
    task_timer * withdraw_timer;
    std::vector<sockaddr_union> mcast_addr_list;
    uint32_t route_block_id;
    uint32_t bgp_device_id;
}route_timer_info_t;

I have the above structure in my Code. I have taken a pointer to the above structure and allocated memory from heap.

route_timer_info_t *route_timer=(route_timer_info_t *)malloc(sizeof(route_timer_info_t));

Now to populate the vector, I am doing

route_timer->mcast_addr_list.push_back(some_item)

In the first run size of the vector is coming correct - as many, I am pushing, but when in 2nd run when the whole structure is allocated memory,the size of vector is coming some big garbage value. What could be going wrong? I have tried clearing the vector as well but no help.

dbush
  • 205,898
  • 23
  • 218
  • 273
  • 15
    That struct cannot be in C code because C doesn't have `std::vector` (or namespaces or templates even). – Kevin Jun 13 '18 at 18:50
  • 2
    Do not `malloc` a `vector` or anything containing a `vector`. `vector`, and the majority of C++ standard library classes, requires its constructor to be run to properly be properly initialized. `malloc` knows not what a constructor is and thus cannot run the constructor. This leaves the `vector` in an unusable state. – user4581301 Jun 13 '18 at 18:50
  • 1
    You can't allocate c++ classes via `malloc` as simple as that. – Voo Jun 13 '18 at 18:50
  • You should execute the Ctor to get it working correctly – JVApen Jun 13 '18 at 18:51
  • 1
    @JVApen That's still UB isn't it? You'd have to malloc the place and then use placement new, no? – Voo Jun 13 '18 at 18:51
  • Yes, indeed. Also Dtor will be needed. I prefer using new (or even better: std::make_unique) – JVApen Jun 13 '18 at 18:54
  • On Ubuntu 17.10 (64 bit), each instance of "route_timer_info_t" is about 40 bytes (regardless of how many elements in the std::vector). Are you sure you want to put this small of an object onto the heap? (using new, of course) – 2785528 Jun 13 '18 at 19:20
  • @DOUGLASO.MOEN Idea of using heap is because I need to access the structure in some other function after a timer expires ( max 25 secs). – Saurabh Suman Jun 13 '18 at 19:24

3 Answers3

0

Because you used malloc to allocate the memory instead of new, the constructor for std::vector was never called, and thus it was never properly initialized.

Always use new:

route_timer_info_t *route_timer = new route_timer_info_t;
dbush
  • 205,898
  • 23
  • 218
  • 273
  • 2
    That really looks like C code `typedef struct`, but it can't be since it has `std::vector`! I can't make sense of this question. – SergeyA Jun 13 '18 at 18:59
  • @SaurabhSuman Glad I could help. Feel free to [accept this answer](https://stackoverflow.com/help/accepted-answer) if you found it useful. – dbush Jun 13 '18 at 19:03
  • If the `struct` at all needs to be `malloc`'d you can malloc it and then make the address into a vector using the `new` operator (the version that takes an address also as parameter). – Ajay Brahmakshatriya Jun 13 '18 at 19:16
0

Malloc only allocates a blob of memory, this is uninitiated. For C code, this is often a member. In C++, there is the concept of a constructor.

Each class and struct gets this by default and it should be optimized in the best way by the compiler. Calling it happens implicitly when constructing the instance on the stack or by using the new operator.

In some special cases, you like to allocate memory in front and construct later. (std::vector does so) You can do so:

auto validPtr = new (&route_timer->mcast_addr_list)   std::vector<sockaddr_union>();

Much better is off course to simply call new directly:

new route_timer_info_t();

To ensure that all members are initialized to a known value, I would recommend to initialize all members explicitly:

struct _route_timer_info_t {
    task_timer * withdraw_timer{nullptr};
    std::vector<sockaddr_union> mcast_addr_list{};
    uint32_t route_block_id{0};
    uint32_t bgp_device_id{0};
};

To make the story complete, from C++14, it is not recommend any more to call new directly, instead you can create a unique pointer directly via:

 std::make_unique<route_timer_info_t>();

In all of these cases, including your original example, the struct ain't compatible with C.

JVApen
  • 11,008
  • 5
  • 31
  • 67
0

As mentioned in the other answers, the main problem is because the object is not "constructed". The mentioned solutions to call the new operator on the struct itself are valid. But for some reason if the struct needs to be malloc'd or it is not in your control, you can construct the object explicitly at that memory using new operator. See the third form here.

You can construct the vector as -

new (&(route_timer->mcast_addr_list)) std::vector<sockaddr_union>;

You can now use the vector as if it was actually created using the new operator.

Ajay Brahmakshatriya
  • 8,993
  • 3
  • 26
  • 49
  • How do I call the destructor for this? I've had a hard time figuring it out... – Superziyi Apr 23 '20 at 02:16
  • @Superziyi this answer describes how the destructor for a custom allocated object can be called without free'ing (and the memory free'd later) - https://stackoverflow.com/a/14187068/2858773 – Ajay Brahmakshatriya Apr 23 '20 at 03:02
  • So in this case you could call it as - `route_timer->mcast_addr_list.~std::vector();` – Ajay Brahmakshatriya Apr 23 '20 at 03:04
  • yeah I always get `'~' in destructor name should be after nested name specifier`/`expected class-name before ‘::’ token` error if I do that. Somehow calling `route_timer->mcast_addr_list.vector::~vector()` compiles instead. – Superziyi Apr 23 '20 at 03:19
  • 1
    @Superziyi so sorry about that. You can also actually skip the name space entirely and the class name as - `route_timer->mcast_addr_list.~vector()` – Ajay Brahmakshatriya Apr 23 '20 at 03:41
  • Thanks! Yeah it's interesting so ~vector() is actually the destructor name that doesn't need std namespace – Superziyi Apr 23 '20 at 04:17