How dangerous is returning a initialized pointer in C++?

Started by
21 comments, last by Paul C Skertich 9 years, 9 months ago

A quick question on how danagerous this would actually be...

In a static library i have this function that will be the starting base for my engine.


_declspec(dllexport) struct EngineAPI {

public: 
EngineAPI(); 
~EngineAPI();

EngineAPI *Create();

private:

};
//-- EngineAPI.cpp
EngineAPI *EngineAPI::Create() {
EngineAPI *engine = new EngineAPI();
return engine;
}


//-- In Dynamic Library to be interpoled in C#.
EngineAPI *gEngine;
void CreateEngine() {
gEngine = gEngine->Create();
if(gEngine == nullptr) {
//-- we have problem
}
else {
//-- We are good to go.
 }
}

I just want to know because it's working for me but on this other question\answer forum they literally jumped on me saying that code was dangerous and absolutely horrendous. I'm just wanting to know why they would consider that?

Game Engine's WIP Videos - http://www.youtube.com/sicgames88
SIC Games @ GitHub - https://github.com/SICGames?tab=repositories
Simple D2D1 Font Wrapper for D3D11 - https://github.com/SICGames/D2DFontX
Advertisement

Okay, I don't quite understand why you are doing things this way. You declare a global pointer, then use it to initialize the same global pointer. Why? if gEngine is not allocated, then calling ::Create is just going to crash.

I use C++ myself, but this is how I would do it instead.


void CreateEngine()
{
   gEngine = new EngineAPI();

   /* Do error checking here */
}

What you are doing appears to be quite unnecessary. Is there a particular reason you want your engine this way?

Shogun.

EDIT: One more detail. If you are returning an initialized pointer from C++ to C#, that is a bad idea. C++ has it's own code routines for initializing a pointer via "new", whereas C# is undoubtedly different, so calling delete on a C++ pointer in C# is potentially dangerous. Think of it this way, would you use malloc() for a C pointer, then deallocate it with delete in C++? That is a VERY bad idea, and I would not recommend it.

Okay, that would make a lot of sense! Well inside the API I wanted to have the engineAPI do most most of the handling. It's not it's crashing - it's just the correctedness of good programming is why the people on that q/a forum was getting hysterical about. I get what you're saying. For instance I wanted everything to be based from the engine or to be branching from the engine.

So, I believe from going back and forth to the static library to the dynamic library for interloping to C# I'm getting a bit mixed up. An API\SDK are just tools either interfaces, abstract classes and a like - am I correct maybe not on the terminology but the basis of it.

Maybe my brain isn't making sense lol! You know how NVIDIA creates a physics scene I don't recall using a new keyword. I switched to bullet physics because I can't compile the nvidia physics library along with my static library file then import the static library into my dynamic library.

Yeah you're right it doesn't make much sense why i would be calling the Create() function because it's just like saying the new keyword like you mentioned.

Game Engine's WIP Videos - http://www.youtube.com/sicgames88
SIC Games @ GitHub - https://github.com/SICGames?tab=repositories
Simple D2D1 Font Wrapper for D3D11 - https://github.com/SICGames/D2DFontX


if gEngine is not allocated, then calling ::Create is just going to crash.

Well, the C++ standard might say that it is undefined behavior (I'm not sure), but it shouldn't crash since ::Create (in its current form) doesn't reference any object state. Of course, there's no reason to do it this way, as you pointed out.

Your Create function doesn't check for a previously allocated pointer. You need to check for a valid pointer and return that if it exists, otherwise multiple calls will result in memory leaks, or worse.


for( int i=0; i < 1000; i++) EngineAPI::Create();

Please don't PM me with questions. Post them in the forums for everyone's benefit, and I can embarrass myself publicly.

You don't forget how to play when you grow old; you grow old when you forget how to play.

I thought I just ask because the stackexchange site wasn't so happy about my question. Instead of the Create() function I just am using the EngineAPI *engine = new APIEngine(). approach. It's not too bad just have to remember to place a delete engine when I'm done. Which I've already have done when the editor closes. When it gets down to the creating the game finally - I'll make sure I check every possibly memory leak. I know how it is to open a program then to close it - then to reopen the program again and it hangs a bit or suddenly just start spitting out exceptions.

That's one more thing I've been working on keeping sure everything is leak free and code is readible so i can understand it when I'm looking back at the code for any improvements.

Game Engine's WIP Videos - http://www.youtube.com/sicgames88
SIC Games @ GitHub - https://github.com/SICGames?tab=repositories
Simple D2D1 Font Wrapper for D3D11 - https://github.com/SICGames/D2DFontX

Also what came to mind is if I stuck with the whole Create function and when I need to dispose it - there will possibly be a memory leak most definately. As Buckeye stated. It's always to stay safe than sorry later on when testing on other platforms.

Game Engine's WIP Videos - http://www.youtube.com/sicgames88
SIC Games @ GitHub - https://github.com/SICGames?tab=repositories
Simple D2D1 Font Wrapper for D3D11 - https://github.com/SICGames/D2DFontX

You wouldn't need to call delete if you would do:


auto engine = std::make_unique<APIEngine>();

You wouldn't need to call delete if you would do:


auto engine = std::make_unique<APIEngine>();

I haven't got to smart pointers yet but still I would need the engine pointer for later referencing until the program closes.

Game Engine's WIP Videos - http://www.youtube.com/sicgames88
SIC Games @ GitHub - https://github.com/SICGames?tab=repositories
Simple D2D1 Font Wrapper for D3D11 - https://github.com/SICGames/D2DFontX

You wouldn't need to call delete if you would do:


auto engine = std::make_unique<APIEngine>();

I haven't got to smart pointers yet but still I would need the engine pointer for later referencing until the program closes.

What I'm trying to do is create a proper framework for the Engine's API. The Engine API is at the parent or root node for instance and all it's counter parts are in the children node. Graphics API is under the Engine's API under the Graphic's API goes in the model rendering and so on and so on. A neat looking heiarchy tree. My framework now is going through re-working.

Game Engine's WIP Videos - http://www.youtube.com/sicgames88
SIC Games @ GitHub - https://github.com/SICGames?tab=repositories
Simple D2D1 Font Wrapper for D3D11 - https://github.com/SICGames/D2DFontX

This topic is closed to new replies.

Advertisement