-2

I'm working on DirectX and I have a function that gives a predefined array of custom vertices, it is

CUSTOMVERTEX* createSampleTriangle()
{
    CUSTOMVERTEX verts[] = 
    {
        {320.0f, 50.0f, 0.5f, 1.0f, D3DCOLOR_ARGB (0, 255, 0, 0), }, 
        {250.0f, 400.0f, 0.5f, 1.0f, D3DCOLOR_ARGB (0, 0, 255, 0), }, 
        {50.0f, 400.0f, 0.5f, 1.0f, D3DCOLOR_ARGB (0, 0, 0, 255), }
    };

    return verts;
}

Now, I pick up the result like this:

CUSTOMVERTEX *v = createSampleTriangle();

but as I do line by line debugging I see only one vertex under v, even though verts shows 3 vertices under it. The program doesn't crash, but then again DirectX just doesn't render when something goes wrong, it just skips the function.

any idea why this happens? Does the function not return a pointer with allocated memory that is after that handled by v?

user3079666
  • 1,159
  • 10
  • 23

2 Answers2

3

You're returning a pointer to a local automatic object that has ceased existing when you return.

That's Undefined Behavior, of the kind known as a dangling pointer.

Make that object static, or allocate it dynamically, or something. I would go for using a std::vector.

Cheers and hth. - Alf
  • 142,714
  • 15
  • 209
  • 331
  • So I only pickup a single vector because of random behavior? Allocating it dynamically would be by using malloc()? – user3079666 May 21 '15 at 16:29
  • @user3079666: Yes to random behavior. Re dynamic allocation, a good way to do that is to use `std::vector`. A not so good but sometimes acceptable way is to use C++ `new[]`. An ungood way, except in the rare cases where you need the efficiency of `realloc`, is to use `malloc`. – Cheers and hth. - Alf May 21 '15 at 16:33
  • Ok, final one, if I use a class, that shouldn't have any chance of failure, right? – user3079666 May 21 '15 at 16:35
  • @user3079666: Oh, in that case just make it `static` and `const`. – Cheers and hth. - Alf May 21 '15 at 16:35
  • Making something static and returning a pointer to it is generally a bad way to approach the situation. There are times where it is justified, but you would know it if that were the case. The cleanest way to do this is to create a `std::vector` and return that. If you get to a point where you need better efficiency, then you should start exploring other options but I doubt it'll be an issue for you now. – RyanP May 21 '15 at 16:39
  • @RyanP: Uhm, bollocks. Do not introduce needless complication, runtime inefficiency and header dependencies. The function wrapper is a Meyers' singleton that ensures that there will be not be any static initialization order fiasco problem. I get the impression from the "generally" that you're applying a rule of thumb without understanding its rationale. Please don't do that when you get expert advice directly. – Cheers and hth. - Alf May 21 '15 at 17:04
  • re the above commentary, my "in that case" referred to a comment that the data is not modified. that comment seems to have disappeared. – Cheers and hth. - Alf May 21 '15 at 17:08
2

The problem as I see here, verts is having automatic storage duration and local to the function createSampleTriangle(). Returning the address of it and therefore using it later will invoke undefined behaviour.

To elaborate, verts only exist in the stack allocated for createSampleTriangle(). Once this function has finished execution, then there is no existence of verts. So, using the returned value (pointer) from the createSampleTriangle() in the caller function will be wrong and result in UB.

Natasha Dutta
  • 3,242
  • 21
  • 23