CODEWITHSUNDEEP
c++winapiregistry
Jakub Cieślak
Jakub Cieślak

Reputation: 65

RegEnumValueA returns 87 ("Invalid Parameter")

I am currently implementing functions and classes from Borland C++ Builder 5 in Visual Studio 2022. One of the classes is used to handle Windows registry IO, and one of its methods is supposed to return a list of values which the current key contains.

I am using Windows' RegEnumValueA function which, after passing correct arguments, always returns 87 – which stands for "Invalid Parameter".

The method looks as follows:

void __fastcall TRegistry::GetValueNames(TStrings* Strings)
{
    HKEY hKey                                     = m_CurrentKey;
    DWORD dwIndex                                 = 0;
    CHAR cValueName[TREG_MAX_VALUES_BUF_SIZE]     = {};
    LPSTR lpValueName                             = cValueName;
    DWORD dwValueBufSize                          = TREG_MAX_VALUES_BUF_SIZE;
    LPDWORD lpcchValueName                        = &dwValueBufSize;
    LPDWORD lpType                                = NULL;
    BYTE cData[TREG_MAX_VALUES_BUF_SIZE]          = {};
    LPBYTE lpData                                 = cData;
    LPDWORD lpcbData                              = NULL;

    long res = RegEnumValueA(hKey, dwIndex, lpValueName, lpcchValueName, NULL, lpType, lpData, lpcbData);
    while (res != ERROR_NO_MORE_ITEMS)
    {
        if (res != ERROR_SUCCESS)
        {
            throw(Exception(AnsiString(res)));
        }
        else
        {
            ++dwIndex;
            res = RegEnumValueA(hKey, dwIndex, lpValueName, lpcchValueName, NULL, lpType, lpData, lpcbData);
        }
    }
}

As you can see, all the parameters are set correctly. My suspicion is that the NULL passed after lpcchValueName is causing this problem, since I've seen some people having the same issue after looking it up. Unfortunately, these were problems from years ago and were related to system-specific issues on e.g. Windows NT. The call to this method looks as follows:

int main()
{
    TRegistry* treg = new TRegistry; // Create a TRegistry object
    if (treg->OpenKey(AnsiString("TRegistryTest"), false)) // Open the TRegistryTest key
    {
        if (treg->OpenKey(AnsiString("subkey1"), true)) // Open the subkey1 key
        {
            TStringList ts; 
            treg->GetValueNames(&ts); // Write the value names into a TStringList
        }
    }
    delete treg;
}

TStringList is essentially a container which stores AnsiString values, which in turn are basically glorified std::strings. I expected the RegEnumValueA function to exit with code 0 as long as there are registry values left to read - in this case, there are 4 values in total in TRegistryTest/subkey1.

Changing TREG_MAX_VALUES_BUF_SIZE does not influence the result at all - it's currently set to a value of 200.

Upvotes: 2

Views: 223

Answers (2)

Remy Lebeau
Remy Lebeau

Reputation: 596497

If you look at C++Builder 5's source code for its TRegistry::GetValueNames() method, you would see that there are two big differences between its approach versus your approach:

  • BCB5 first calls RegQueryInfoKeyA() to retrieve the number of values in the key, and the length of the longest value name in the key. It then allocates a buffer of that length, and then runs a for loop calling RegEnumValueA() to retrieve the name for each index. But it ignores the return value, which means it could fail if the Registry key is modified mid-loop.

    Whereas you are (rightly so) calling RegEnumValueA() in a while loop until it reaches the end of the list or fails. But, you are using a fixed-length buffer to receive the names. 200 should be OK in practice for most systems, but you should use a dynamically allocated buffer and pay attention to RegEnumValueA()'s return value so you can know when it tells you that it needs additional buffer space that you can then allocate.

  • BCB5 sets the lpType, lpData, and lpcbData parameters of RegEnumValueA() to NULL, since it does not need those data.

    Whereas you are setting the lpData parameter to the address of a local buffer, which is wrong (as other comments/answers have already pointed out), and unnecessary anyway since all you really want is each value's name and not its content.

Upvotes: 0

Adrian Mole
Adrian Mole

Reputation: 51845

Your lpcbData parameter, which you have set to NULL is invalid. This should be the address of a DWORD that specifies the size (in bytes) of the buffer pointed to by the lpData parameter (i.e. the size of the cData array).

From the documentation:

[in, out, optional] lpcbData

A pointer to a variable that specifies the size of the buffer pointed to by the lpData parameter, in bytes. When the function returns, the variable receives the number of bytes stored in the buffer.

This parameter can be NULL only if lpData is NULL.

Also, note that, on success, the values in the variables pointed to by that lpcbData argument and by lpcchValueName (i.e. dwValueBufSize) will be modified to contain the actual sizes of the data/value returned. So, you should reset those before each call. For lpcchValueName, you would use a line like dwValueBufSize = TREG_MAX_VALUES_BUF_SIZE;, with similar code for the lpcbData target, depending on what you call that variable. (And I'm not sure it's an especially good idea to use the same size variable for both, as your comment seems to suggest.)

Upvotes: 3

Related Questions