0

I have a function:

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

And I wanted to invoke it like this:

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

I get an error for all the three arguments:

uninitialized local variable

In my head it went like: I declare major, minor, build, and they get filled in the function. Space is already allocated for them during the first three lines of the invoking code.

I am surely missing something here. Could somebody please explain this for me?

Improve this question
1
  • 2
    You pass all the three variables major, minor and build as non-reference to the function, without initialising them. Since the function deals with the values of those variables this doesn't make much sense. Commented Jun 29, 2014 at 0:42

5 Answers 5

Reset to default
5

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.

Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

+1 If any here, I think this answer explains best, what the probable misconception of the OP is!
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

Improve this answer

7 Comments

Your answer could be greatly improved by showing a simple sample how to initialize those variables properly.
Yes, I have tried that, the compiler says: "Cannot convert parameter 1 from PDWORD to PDWORD"
@user3718333 Is that really the error you get from the compiler? That seems strange. Can you paste the exact code?
@David Schwartz That is what I got when invoking with the code sample in the comment's first grey box. The code was exactly that + the function
@user2451677 You're going into a worse direction. Pointer references and new() aren't necessary here.
|
2

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.

Improve this answer

Comments

0

Those are pointers. They are not pointing at any memory you have allocated. They do not get "filled" in the function, they get used to access (uninitialized) memory.

Improve this answer

Comments

-1

You are probably not getting an error but a warning (but you might have configured your complier to treat warnings as errors).

If executed, your program will segfault because you are writing to the memory pointed to by them but as they are uninitialized they contain invalid/random addresses.

Possible solution

PDWORD major = new DWORD;
PDWORD minor = new DWORD;
PDWORD build = new DWORD;

assuming PDWORD is defined to be *DWORD.

Do not forget the deletes!

edit: Actually its much more sensible to allocate these on the stack – see user2451677’s answer.

Improve this answer

3 Comments

Thanks @wonce. I before posting i tried allocating them as new PDWORD and now I get the point. Really a complete answer, thanks a lot!
-1 No, No, No that's veeeery bad advice! And certainly shouldn't be accepted! Think twice!
That is a bad solution. It gives the impression that pointer arguments means that you must actually declare pointers. You are needlessly calling the allocator.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.