2

I have a unique_ptr member on a class that points to a struct.

class ExampleClass {
    std::unique_ptr<StateStruct> _character_state;
}

I don't understand how to acquire the memory for the struct and set the unique_ptr.

In my constructor I have:

ExampleClass::ExampleClass {
    std::unique_ptr<StateStruct> _character_state(static_cast<StateStruct*>(malloc(sizeof(StateStruct))));
    _character_state->state_member_int_value = 4 // _character_state is empty
}

What am I doing wrong?

dbush
  • 205,898
  • 23
  • 218
  • 273
adelphospro
  • 165
  • 1
  • 2
  • 11
  • 9
    `malloc()` rather using `new` seriously?? – πάντα ῥεῖ Aug 04 '15 at 21:14
  • 1
    Your struct is probably not initialized correctly because of this `malloc` nonsense you do. You also most likely do not even need a pointer here. – Baum mit Augen Aug 04 '15 at 21:15
  • 2
    I particularly like the juxtaposition of malloc() with a C++ style cast. – Jonathan Potter Aug 04 '15 at 21:18
  • 3
    Err.. no one noticed that he is creating a *local* `_character_state` that hides the member `_character_state`? This has noting to do with the `malloc`. – sbabbi Aug 04 '15 at 21:27
  • Haha... yes I am still learning.... In my defense, this represented a moment of extreme desperation. I am working with a code base that is over 20 years old. @sbabbi, can you explain what you mean by creating a local state that hides the member? – adelphospro Aug 04 '15 at 23:59
  • @adelphospro, in your example ctor you put `typename varname` in the first line without assigning it to the local `ExampleClass::_char_state`. It will be treated as a local var even though it shares the same name. – learnvst Aug 05 '15 at 13:38
  • @learnvst Ah yes I see. Thank you. It is my first time working with smart pointers and class inheritance in C++ (from a C, then Ruby background. Working with an old c codebase). – adelphospro Aug 05 '15 at 14:43

3 Answers3

2
ExampleClass::ExampleClass() : _character_state( new StateStruct() ) {

}

... or if you want to transfer ownership at some point later (you could do this in the constructor also, but does not convey what you are trying to do as clearly)

_character_state.reset( new StateStruct() );

... or in the interests of completeness, you can assign a fresh unique_ptr to your variable if you enjoy typing

_character_state = std::unique_ptr<someObject>(new someObject());
learnvst
  • 15,455
  • 16
  • 74
  • 121
  • 3
    Personally, I'd prefer to see `make_unique` instead of `new`. – Chris Drew Aug 04 '15 at 21:31
  • @Chris Drew Because the question is tagged C++11 and not C++14 http://stackoverflow.com/questions/7038357/make-unique-and-perfect-forwarding Note: I had to look up the details on that first before forming my douchy reply ;) – learnvst Aug 04 '15 at 21:34
1

Well don't use malloc.

std::unique_ptr<StateStruct> _character_state(static_cast<StateStruct*>(malloc(sizeof(StateStruct))));
                                                                        ^^^^^^

The unique_ptr release the memory with a call to delete (not free).

Also you are creating a local variable inside the constructor (not initializing the member variable). Prefer to initialize member variables inside the initializer list not in the body of the constructor.

ExampleClass::ExampleClass {
    _character_state(new StateStruct)
{
    // Should you not move this to the constructor
    // if StateStruct
    _character_state->state_member_int_value = 4
}
Martin York
  • 257,169
  • 86
  • 333
  • 562
  • To be frankly you can make it call `free()` but I do not see any reason to use malloc/free – Slava Aug 04 '15 at 21:25
0

What am I doing wrong?

First of all your syntax is wrong, you are missing brackets. Second, inside your constructor you are creating local variable _character_state which would hide member variable and leave it uninitialized. So proper syntax is:

ExampleClass::ExampleClass() :
    _character_state( std::make_unique<StateStruct>() )
{
    _character_state->state_member_int_value = 4 // _character_state is empty
}

If you, for watever reason, have to create StateStruct with malloc(), you would need to supply custom deleter, that calls free().

If you do not really need malloc() you probably should initialize state_member_int_value inside StateStruct own constructor.

Slava
  • 43,454
  • 1
  • 47
  • 90