Debug CRT memory allocation

[tblaxland]

EDIT: Posts moved here from the OBSP Development Thread

I am trying to get it to work in release build atm. The MFD doesn't show in the menu when built as release, so I think it's gt something to do with not initializing variables.

But it shows when built as Debug? Very strange. Sounds like a problem around oapiRegisterMFDMode. One clue is that the debug runtime libraries poison the memory with non-null values when they allocate memory, yet the runtime libraries don't (because it is faster) so most often a freshly allocated piece of memory will be filled with zeros.

 

[Kozak]

One clue is that the debug runtime libraries poison the memory with non-null values when they allocate memory, yet the runtime libraries don't (because it is faster) so most often a freshly allocated piece of memory will be filled with zeros.

tblaxland, it's just the opposite. The memory is usually filled with random values, and debug library does fill it with zeroes, while release does not.

 

[tblaxland]

tblaxland, it's just the opposite. The memory is usually filled with random values, and debug library does fill it with zeroes, while release does not.

I beg to differ. See the magic numbers 0xCCCCCCCC and 0xCDCDCDCD on this page:
http://msdn.microsoft.com/en-us/library/aa260966(VS.60).aspx
So if you are using the debug runtime library, this declaration will give myVessel a value of 0xCCCCCCCC upon creation:

VESSEL *myVessel;

It is handy because if you get a CTD with an exception for accessing memory at 0xCCCCCCCC or 0xCDCDCDCD, you know for sure you have an uninitialised pointer.

On the other hand, there are no guarantees what values the release runtime library will give you in the above declaration but it is often zero, and often enough that it will hide badly written code.

 

[Kozak]

I beg to differ. See the magic numbers 0xCCCCCCCC and 0xCDCDCDCD on this page:
http://msdn.microsoft.com/en-us/libr...66(VS.60).aspx
So if you are using the debug runtime library, this declaration will give myVessel a value of 0xCCCCCCCC upon creation:

1.In debug mode values are always filled with something (and the memory allocated by new operator will be filled with zeroes). In release mode they're just left what was in that place.
2.You're confusing stack variables (which can be pointers, in which case they are filled with magic numbers) and allocated memory, which you were talking about.

 

[RisingFury]

Yea, I think in release mode you just inherit the crap that was left in the memory by the program before you...

 

[computerex]

As far as I know DarkWanderer is correct, only in debug mode memory is explicitly filled with magic numbers in order to aid in debugging. Otherwise the programmer is expected to initialize it.

http://en.wikipedia.org/wiki/Magic_number_(programming)#Magic_debug_values

 

[tblaxland]

1.In debug mode values are always filled with something (and the memory allocated by new operator will be filled with zeroes).

Nope. Try this:

int *i;
i = new int;
assert((*i)==0xCDCDCDCD); // true for debug CRT

See here:
http://www.nobugs.org/developer/win32/debug_crt_heap.html#table

In release mode they're just left what was in that place.

I agree.

2.You're confusing stack variables (which can be pointers, in which case they are filled with magic numbers) and allocated memory, which you were talking about.

See code snippet above. i sits on the stack, is initialised to 0xCCCCCCC, changes to the value returned by new and is pointing to allocated heap memory. The allocated portion of heap memory has 0xCDCDCDCD in it (for the debug runtime, anyway).

 

[Kozak]

Well, I admit I'm wrong. Just misunderstood tblaxland's original post.

 

[escapetomsfate]

So, is initialising all pointers (even all variables?) to NULL a good idea? Or declare them as new (data type) and delete when finished with them?

Now I know to build in release from the start.

 

[computerex]

Why build in release mode when you are debugging? You won't be able to use the debugger to it's full extent (unless you modify the release settings to generate debug information, etc).

 

[dbeachy1]

^ +1 to that. The whole point of the debug CRT libraries setting unused (including freed) memory blocks to 0xCDCDCDCD is so that the program will crash consistently if uninitalized or already-freed memory is used as a pointer during development. [And if the uninitialized memory is used as an integer or some other non-pointer value 0xCDCDCDCD will quite probably show up as a bug during testing.] So it's a good idea to always do your initial development testing using debug libraries so that you can catch memory errors during development (not to mention being able to use the debugger more easily, as CX said). Of course, you should also do testing with release builds as well before going to beta testing, but using debug libraries during development can help catch memory management errors that may be very difficult to reproduce and track down using a release build.

So, is initialising all pointers (even all variables?) to NULL a good idea? Or declare them as new (data type) and delete when finished with them?

Well, except for static or global variables that you want initialized to zero, you must always initialize your variables and pointers before you use them. However, there is no point in setting a variable to NULL and then immediately setting it again (that would be redundant and optimized out by the compiler anyway). As for when to use 'new' and 'delete' vs. just creating local or class variables, that depends on what the variable is supposed to do and how long it needs to live. One way is not "better" than the other -- both are necessary depending on what the variable does. A good C++ book is the best way to learn about variable scopes.

 

[escapetomsfate]

I'm not debugging, atm it all runs fine - in debug build. It needs to be built in release so that it can be used by everyone.

Besides, if there is a problem, I should eliminate it anyway.

 

[Hielor]

So, is initialising all pointers (even all variables?) to NULL a good idea? Or declare them as new (data type) and delete when finished with them?

Unless you're going to be immediately setting the value to something else, there is no disadvantage at all to always initializing your variables.

Also make sure you always initialize strings, to at the very least have a null character as the first character.

Now I know to build in release from the start.

You won't be able to do debugging if you build in release mode...

 

[tblaxland]

I'm not debugging, atm it all runs fine - in debug build. It needs to be built in release so that it can be used by everyone.

Besides, if there is a problem, I should eliminate it anyway.

If the MFD is not showing up in the list, it suggests that there is a problem with your call to oapiRegisterMFDMode. Can you post the code relevant to that function call here?

 

[escapetomsfate]

If the MFD is not showing up in the list, it suggests that there is a problem with your call to oapiRegisterMFDMode. Can you post the code relevant to that function call here?

DLLCLBK void opcDLLInit (HINSTANCE hDLL)
{
    LOG = oapiOpenFile ("\\OBSP\\OBSP.txt",FILE_OUT,CONFIG);
    WriteLog("**OBSP INTIATED**");

        static char *name = "Weapons Management";   // MFD mode name
    MFDMODESPEC spec;
    spec.name = name;
    spec.key = OAPI_KEY_T;                // MFD mode selection key
    spec.msgproc = WeaponMFD::MsgProc;  // MFD mode callback function

    // Register the new MFD mode with Orbiter
    //if(ShowMFD == true) <- For now I won't use it
        g_MFDmode = oapiRegisterMFDMode (spec);
    
}

I've been playing around with the code a bit. Now, the release version crashes orbiter shortly after load (OpenRenderViewport?)

The debugger says:

        7C911689  mov         ecx,dword ptr [ecx]    CXX0013: Error: missing operator

EDIT: Could this section be the problem?

WriteLog("Beggining Weapon Config file Parse... \n");
    char OrbiterPath[_MAX_PATH];
    _getcwd(OrbiterPath,_MAX_PATH);

I checked the log after the crash - it ends on "Beggining Weapon Config file Parse..."! I think this must be the problem.

I've tried initializing it like this:

char *OrbiterPath=NULL;
    _getcwd(OrbiterPath,_MAX_PATH);

and like this:

char OrbiterPath = new char[_MAX_PATH];

But those just produce the same result.

EDIT2: I've found the problem. Really.

It's not the fact that I'm initialising it wrong, it's that I'm using OrbiterPath in a stringstream.

    char *OrbiterPath=NULL;
    _getcwd(OrbiterPath,_MAX_PATH);

stringstream WeaponDir;
WeaponDir << OrbiterPath << "\\Config\\OBSP\\Weapons";

When I comment out the line where the variables are copied into WeaponDir, it runs fine (obviously there are no weapons loaded, but it works).

So how do I copy a char* into a stringstream safely?

 

[computerex]

What you are doing is you are getting the current directory in some random location in memory. You did not allocate any memory for your path to be stored in.

Try this:

char path[256];
_getcwd(path, 255);

 

[escapetomsfate]

I've fixed the initialization problem, I just can't use the char* in a stringstream.

I've tried making a std::string to append() the char to, but no luck.

 

[Hielor]

I've fixed the initialization problem, I just can't use the char* in a stringstream.

I've tried making a std::string to append() the char to, but no luck.

Check the return value of _getcwd to make sure that the function is actually succeeding.

There should be nothing wrong with using a char * in a stringstream, especially since the literal string you have there is a char*.

Do (something like) this:

char path[256];
memset(path, 0, sizeof(path));
if (_getcwd(path, 256) == NULL)
{
  WriteLog("Error getting working directory!");
} else {
  // do stuff here
}

This is bad:

    char *OrbiterPath=NULL;
    _getcwd(OrbiterPath,_MAX_PATH);

stringstream WeaponDir;
WeaponDir << OrbiterPath << [URL="file://\\Config\\OBSP\\Weapons"]\\Config\\OBSP\\Weapons[/URL];

What you're doing there is attempting to copy the working directory into a null pointer, and then trying to put that null pointer into the stringstream. You need to use the char[] like I have above or like computerex showed.

 

[escapetomsfate]

Check the return value of _getcwd to make sure that the function is actually succeeding.

There should be nothing wrong with using a char * in a stringstream, especially since the literal string you have there is a char*.

Do (something like) this:

char path[256];
memset(path, 0, sizeof(path));
if (_getcwd(path, 256) == NULL)
{
  WriteLog("Error getting working directory!");
} else {
  // do stuff here
}

This is bad:

    char *OrbiterPath=NULL;
    _getcwd(OrbiterPath,_MAX_PATH);

stringstream WeaponDir;
WeaponDir << OrbiterPath << [URL="file://%5C%5CConfig%5C%5COBSP%5C%5CWeapons"]\\Config\\OBSP\\Weapons[/URL];

What you're doing there is attempting to copy the working directory into a null pointer, and then trying to put that null pointer into the stringstream. You need to use the char[] like I have above or like computerex showed.

I tried that code you gave me, with a log message to indicate success. It still crashed, but the success message appeared in the log. So it has found the path, saved it to a valid char* pointer and it can't append it to a std::string or stringstream

 

[Hielor]

I tried that code you gave me, with a log message to indicate success. It still crashed, but the success message appeared in the log. So it has found the path, saved it to a valid char* pointer and it can't append it to a std::string or stringstream

Can you post the code as you currently have it, please?

Also, are you printing the path to the log after you've fetched it?