Different disk serial number result from GetVolumeInformation()

Question

DWORD disk_serialINT[MAX_PATH + 1];
GetVolumeInformationA(NULL, NULL, NULL, disk_serialINT, NULL, NULL, NULL, NULL);
char* disk_serialANSI;
sprintf(disk_serialANSI, "%d", disk_serialINT);
std::string HDDserial = disk_serialANSI;

This is my piece of code where I get the hdd serial number, but the problem is that every time when the program executes the value is different. Can someone explain that?

SOLVED:

DWORD disk_serialINT;
GetVolumeInformationA(NULL, NULL, NULL, &disk_serialINT, NULL, NULL, NULL, NULL);
std::string HDDserial = std::to_string(disk_serialINT);

Thanks.

Answer 1

These two lines will give you undefined behavior:

char* disk_serialANSI;
sprintf(disk_serialANSI, "%d", disk_serialINT);

You declare a pointer variable, but you don't actually make it point anywhere. Uninitialized local variables have an indeterminate value (in practice it will be seemingly random), and by using that uninitialized pointer you don't know where the sprintf call will write.

---

Since you're programming in C++ there are a couple of solutions.

• The old-fashioned is to make disk_serialANSI an array of characters, big enough to hold the number (including string terminator). An alternative is to manually allocate memory for the pointer, and then free that memory again when you're done with it.

• Using std::ostringstream to format the data and get a std::string.

• Using std::to_string to convert to a string directly.

• Use Boost Lexical cast.

Answer 2

On top of what Joachim says, you are not passing the serial number correctly. You are expected to pass a pointer to a single value.

DWORD disk_serialINT;
GetVolumeInformationA(NULL, NULL, NULL, &disk_serialINT, NULL, NULL, NULL, NULL);

In your code this is compounded by what you do here:

sprintf(disk_serialANSI, "%d", disk_serialINT);

Ignoring the uninitialised variable disk_serialANSI, which Joachim's answer covers, you are passing a pointer to the "%d" format string. Once you change disk_serialINT to be a single value things will be better. However, you are passing an unsigned value to a single format string.

It's time to give up on these gnarly C formatting functions and use the C++ standard library to convert between integral values and text.

One final point to stress is that you must check the return value of calls to Win32 API. You've no idea whether or not the function call succeeded. You must not assume that it does. This is all covered in the documentation: https://msdn.microsoft.com/en-us/library/windows/desktop/aa364993.aspx

This program prints the serial number of the volume containing the current directory:

#include <Windows.h>
#include <iostream>

int main()
{
    DWORD disk_serialINT;
    if (!GetVolumeInformationA(NULL, NULL, NULL, &disk_serialINT, NULL,
        NULL, NULL, NULL))
    {
        std::cout << "Failed: " << GetLastError() << std::endl;
        return 1;
    }
    std::cout << "Current directory volume serial numnber: " << std::hex 
        << disk_serialINT << std::endl;

    return 0;
}

Answer 3

In case anyone comes looking at this code and the solution here is my take on it:

Problem #1

DWORD disk_serialINT[MAX_PATH + 1];

• Declaring an array of DWORD values when you only need one.
• Using MAX_PATH macro (and adding 1 to it) without understanding what it is used for.
• Not knowing types (naming a variable INT when DWORD type is unsigned).
• Not understanding problem domain (naming a variable disk_serial when it is actually a volume serial number).

Problem #2

GetVolumeInformationA(NULL, NULL, NULL, disk_serialINT, NULL, NULL, NULL, NULL);

• No error handling (not checking function return value for failure).
• Not understanding the difference between passing parameters by value and by reference (not passing the address of disk_serialINT variable).

Problem #3

char* disk_serialANSI;

• Not understanding the difference between array and a pointer.
• Not understanding the difference between static and dynamic allocation.

Problem #4

sprintf(disk_serialANSI, "%d", disk_serialINT);

• Using uninitialized variable disk_serialANSI.
• Using wrong variable type (either use an array of char, or keep the pointer but allocate memory and later free it).
• Not understanding problem domain (using wrong format for the volume serial number, should be %08lX).
• Not using secure version of sprintf.

Problem #5

std::string HDDserial = disk_serialANSI;

• Mixing C and C++ without a good reason.

In other words, all your typical beginner programmer mistakes. Hopefully they learned more in the meantime than just to find an answer for their problem on Stack Overflow.

There is already a C++ solution provided by David Heffeman above so here is the equivalent C solution for the sake of answer completeness:

#include <stdio.h>
#include <windows.h>

int main()
{
    const int VolumeSerialLength = 9; // 8 hex digits + zero termination
    char VolumeSerial[VolumeSerialLength] = { 0 };
    DWORD VolumeSerialNumber;

    if (GetVolumeInformationA(NULL, NULL, NULL, &VolumeSerialNumber, NULL, NULL, NULL, NULL) {
        sprintf_s(VolumeSerial, VolumeSerialLength, "%08lX", VolumeSerialNumber);
        printf("Volume serial number: %8.8s.\n", VolumeSerial);
        return 0;
    } else {
        printf("GetVolumeInformationA() error: %08lX\n", GetLastError());
        return 1;
    }
}