C++ uninitialized local variable

Solution 1:

You are making the mistake that many make when it comes to functions that require pointer arguments.

When a function requires a pointer as an argument, it doesn't mean you blindly declare a pointer and pass it to the function. What the function is asking for is an address-of an existing, valid, entity.

DWORD major, minor, build;
GetOSVersion(&major, &minor, &build);

The DWORD's above are valid, and all that is done is pass the address of these variables to the function.

The other mistake that is related to this (not a bug, as it will give the desired results, but still a "mistake") is to declare a pointer, have it point to somewhere valid, and then pass it to the function. In other words:

PDWORD major, minor, build;
major = new DWORD;
minor = new DWORD;
build = new DWORD;
GetOSVersion(major, minor, build);
delete major;
delete minor;
delete build;

or

PDWORD pmajor, pminor, pbuild;
DWORD major, minor, build;
pmajor = &major;
pminor = &pminor;
pbuild = &build;
GetOSVersion(pmajor, pminor, pbuild);

I have seen code written in this manner. This indicates that the programmer does not have a clear understanding of what it means when a function requires a pointer as an argument. The programmer erroneously believes that a pointer must be declared, have it point somewhere valid, and then pass this pointer.

Yes, you get the results without crashing, but it is a waste of time to call the allocator (new / delete), or if not calling the allocator, create unnecessary pointer variables which unnecessarily obfuscates the code.

So the easiest way is the first example above. Just declare non-pointer types, and just pass the address.

Solution 2:

the problem is there:

DWORD major;
DWORD minor;
DWORD build;
GetOSVersion(&major, &minor, &build);

Fix:

VOID GetOSVersion(PDWORD major, PDWORD minor, PDWORD build)
{
    OSVERSIONINFO osver = {};
    osver.dwOSVersionInfoSize = sizeof(OSVERSIONINFO);
    GetVersionEx(&osver);
    if(major)
    *major = osver.dwMajorVersion;
    if(minor)
    *minor = osver.dwMinorVersion;
    if(build)
    *build = osver.dwBuildNumber;
}

DWORD major = 0;
DWORD minor = 0;
DWORD build = 0;
GetOSVersion(&major, &minor, &build);

PDWORD is pointer to DWORD. All three parameters are output parameters. In C/C++, it's a common usage: if you want to return more then one value from a function, you need pass pointer (or reference too in case of c++) to a variable:

int var = 0;
if(some_function_that_can_fail_and_write_result(&var))
 ;//do something

In your case you are passing uninitialized pointer to a function by value. It's the same as:

void foo(int parameter);
// ...
int a;
foo(a);

You have a lot of ways:

Pass uninitialized pointer by reference:

VOID GetOSVersion(PDWORD& major, PDWORD&, PDWORD&)
{
//...
major = new DWORD(osver.dwMajorVersion);

}
// usage:
PDWORD major;
GetOSVersion(major, ....);

//...
delete major;

Pass all parameters by reference:

VOID GetOSVersion(DWORD& major, DWORD&, DWORD&)
{
//...
major = osver.dwMajorVersion;

}
// usage:
DWORD major = 0;
GetOSVersion(major, ....);

Use your version of GetOSVersion(), but with the fix in this answer on the beginning

Solution 3:

You probably meant to have your variable declarations and to call your function like this

DWORD major;
DWORD minor;
DWORD build;
GetOSVersion(&major, &minor, &build);

You use pointers to reference the output parameters, thus these need to point them to valid memory addresses. You can refer to those variables to get a valid pointer using the 'address-of' (&) operator as shown above.

With c++ you can use by reference parameters, which will make things a bit clearer

VOID GetOSVersion(DWORD& major, DWORD& minor, DWORD& build) {
    OSVERSIONINFO osver;
    ZeroMemory(&osver, sizeof(OSVERSIONINFO));
    osver.dwOSVersionInfoSize = sizeof(OSVERSIONINFO);
    GetVersionEx(&osver);
    // Note there's no check needed if the pointers are valid!
    major = osver.dwMajorVersion;
    minor = osver.dwMinorVersion;
    build = osver.dwBuildNumber;
}

DWORD major;
DWORD minor;
DWORD build;
GetOSVersion(major, minor, build);

No need to call the new() allocator (and bother with correct dynamic memory allocation management) with any of the above samples in 1st place.