GetComputerName() works on win7 but fails on xp

Question

I am trying to use GetComputerName() to retrieve the box's name. The code works great on windows 7, however, when i test on windows xp the code does not work (result is UNKNOWN). Any idea why?

int GetBoxName(BoxInfo &box_info)
{
    int Ret;    
    DWORD dwLen;
    const char* szUnk = "UNKNOWN\0";

    // Get the size of the box name then allocate memory.
    Ret = GetComputerName(NULL, &dwLen);
    box_info.BoxName = new char[dwLen]; 

    // Attempt to retrieve the box name.
    if((Ret = GetComputerName(box_info.BoxName, &dwLen) == 0))
    {       
        delete[] box_info.BoxName;
        box_info.BoxName = new char[strlen(szUnk)];
        box_info.BoxName = (char*)szUnk;
        return 1;       
    }   

    return 0;
}

Answer 1

@Ben has given a good account of errors that you made. I'd like to show you how you would typically call GetComputerName. You are making it harder than it needs to be. The key information is the excerpt from the documentation:

The buffer size should be large enough to contain MAX_COMPUTERNAME_LENGTH + 1 characters.

You have a hard upper bound on the size of the buffer. You can therefore use a fixed size buffer, and only make a single call to GetComputerName.

std::string getComputerName()
{
    char buffer[MAX_COMPUTERNAME_LENGTH + 1];
    DWORD len = MAX_COMPUTERNAME_LENGTH + 1;
    if (GetComputerName(buffer, &len))
        return std::string(buffer, len);
    return "UNKNOWN";
}

Were you compiling for Unicode it would be:

std::wstring getComputerName()
{
    wchar_t buffer[MAX_COMPUTERNAME_LENGTH + 1];
    DWORD len = MAX_COMPUTERNAME_LENGTH + 1;
    if (GetComputerName(buffer, &len))
        return std::wstring(buffer, len);
    return L"UNKNOWN";
}

If you want to cater for the possibility of the computer name being longer than MAX_COMPUTERNAME_LENGTH then you can write it like this:

std::string getComputerName()
{
    char buffer[MAX_COMPUTERNAME_LENGTH + 1];
    DWORD len = MAX_COMPUTERNAME_LENGTH + 1;
    if (GetComputerName(buffer, &len))
    {
        return std::string(buffer, len);
    }
    if (GetLastError() == ERROR_BUFFER_OVERFLOW)
    {
        std::vector<char> name(len);
        if (GetComputerName(&name[0], &len))
        {
            return std::string(&name[0], len);
        }
    }
    return "UNKNOWN";
}

I don't know whether or not this can even happen. The docs hint that it can, although if it does happen then it renders MAX_COMPUTERNAME_LENGTH somewhat of a misnomer. If we pass a value of len that is less than MAX_COMPUTERNAME_LENGTH+1 then the function succeeds if the name fits. It does not automatically fail with ERROR_BUFFER_OVERFLOW. Of course, if the name returned by this function can never exceed MAX_COMPUTERNAME_LENGTH then the second version is rather paranoid.

---

FWIW, the code in your updated answer is still badly broken. You simply must not pass NULL for the first parameter to GetComputerName. The documentation could not be much clearer.

Answer 2

This makes no sense at all:

box_info.BoxName = new char[strlen(szUnk)];
box_info.BoxName = (char*)szUnk;

You allocate memory, then immediately lose track of it. And you are directing a non-const pointer to a string literal. And the amount of memory allocated here doesn't include space for the terminating NUL byte, so you would overrun the buffer.

Perhaps you wanted the second line to be

strcpy(box_info.BoxName, szUnk);

And why aren't you using a smart pointer to automatically deallocate the memory when needed, for example std::string or std::unique_ptr<char[]> ?

---

Finally, the documentation says

The buffer size should be large enough to contain MAX_COMPUTERNAME_LENGTH + 1 characters.

That is a pretty plain requirement. Makes you wonder the point of

If the buffer is too small, the function fails and GetLastError returns ERROR_BUFFER_OVERFLOW. The lpnSize parameter specifies the size of the buffer required, including the terminating null character.

This latter behavior appears to only be true for the Unicode version. Stanly Roark left a comment on the MSDN page that:

The ANSI version does not return the required length

I have to maintain an old application that uses ANSI. I have noticed that, while the Unicode version returns the required buffer length, the ANSI version does not.