1

I get a bad feeling about this code 

widget* GetNewWidget()
{
   widget* theWidget = (widget *) malloc(sizeof(widget));
   return theWidget;
}

Firstly, one should never cast the result of malloc() (nor, I suspect, use it in C++ (?)).

Secondly, won't theWidget be allocated on the stack?

If so, won't the caller trying to access after this function returns be undefined behaviour?

Can someone point to an authoritative URL explaining this?

[Update] I am thinking of this question Can a local variable's memory be accessed outside its scope?

Community
  • 1
  • 1
Mawg says reinstate Monica
  • 38,334
  • 103
  • 306
  • 551

2 Answers2

1

On the stack you just allocated a pointer, it's not related to the object itself. :)

I never use malloc (it's a C thing, you shouldn't use it in C++), thus i am not sure, but i hardly believe it's undefined behaviour.

If you would write this: widget* theWidget = new widget(); it should work correctly.

Even better if you use smart pointers if you have C++11

std::unique_ptr<widget> GetNewWidget()
{
   std::unique_ptr<widget> theWidget(std::make_unique<widget>());
   return theWidget;
}

Or in this case you can write even smaller code, like this:

std::unique_ptr<widget> GetNewWidget()
{
   return std::make_unique<widget>();
}

The above version will clean out the memory as soon as unique pointer go out of scope. (unless you move it to another unique_ptr) It's worth some time to read about memory management in C++11.

Melkon
  • 418
  • 3
  • 12
  • Thanks . I am looking at someone else's code and wondering whether it is undefined behaviour as it stands. Alas, we are restricted to C++03. It is still not clear to me, even with your answer, if it would be undefined (or dangerous) behaviour to access that return after return. – Mawg says reinstate Monica Sep 01 '15 at 07:29
  • 1
    You allocate a memory on the heap for your object, it's not related to the stack. On the stack you just have a pointer. As long as you use heap allocated object you shouldn't care about stacks, it's defined. You can have several pointer to the same heap object. This would be problematic only if you would allocate the object on the stack and return a pointer to it, so like: `widget myWidget; return &myWidget;` This would crash as soon as you try to use this reference cause you would have a reference to a deleted object. In heap you don't have such a problem as long as you don't free it. – Melkon Sep 01 '15 at 07:32
  • I think that I am missing your point, sorry. I do understand that the actual memory here is on the heap. However, where I fear a problem is that the pointer to it is on the stack (variable theWidget ). Won't that pointer go out of scope when the function reruns, leaving the allocated memory with nothing reliably pointing to it? – Mawg says reinstate Monica Sep 01 '15 at 07:35
  • 1
    That's why you return the pointer. If you call this function like this: `widget* widgetptr = GetNewWidget();` then you have a copy of the pointer to the heap allocated object, which you can use. It's absolutely okay. If you just call this function without storing the return value, so for some unknown reason you write something like: `if(GetNewWidget()) {blablabla}` then you would have a memory leak, but it's still defined and won't crash. – Melkon Sep 01 '15 at 07:37
  • It's the scenario i wrote that would crash... in the code you wrote you don't return a pointer to a local variable, you return a pointer to a heap object... – Melkon Sep 01 '15 at 07:46
  • Agreed - BUT the pointer itself is on the stack and goes out of scope at the end of the function - surely the caller cannot be guaranteed access to it after the function returns. – Mawg says reinstate Monica Sep 01 '15 at 07:49
  • It would be undefined behaviour if someone tried to use the `malloc`'d space as if it contained a real `widget`, and widget is not trivially constructible – M.M Sep 01 '15 at 08:27
  • 1
    @Mawg the caller accesses a copy of the pointer, not the original pointer. This function returns by value which means that the caller receives a copy of the expression being `return`ed. It is no different to `int x() { return 5; }` – M.M Sep 01 '15 at 08:28
1

In summary: this code is perfectly fine

Returning a pointer is like returning an int: the very act of returning creates a bitwise copy.

Step, by step, the code works as follows:

  1. malloc(sizeof(widget));

    Allocates a block of memory on the heap[1], starting at some address (let's call it a), and sizeof(widget) bytes long.

  2. widget* theWidget = (widget *) malloc(sizeof(widget));

    Stores the address a on the stack[2] in the variable theWidget. If malloc allocated a block at address0x00001248, then theWidget now contains the value 0x00001248, as if it were an integer.

  3. return theWidget;

    Now causes the value of a to be returned, i.e., the value 0x00001248 gets written to wherever the return value is expected.

At no point is the address of theWidget used. Hence, there is no risk of accessing a dangling pointer to theWidget. Note that if your code would return &theWidget;, there would have been an issue.

[1] Or it might fail, and return NULL

[2] Or it might keep it in a register

Community
  • 1
  • 1
Thierry
  • 1,099
  • 9
  • 19