Hi,
+#define MAX_NAME_BUFFER 255 +#define MAX_VALUE_BUFFER 16383
I don't like this. Ideally the code should query the lengths from the Registry APIs and allocate appropriate buffers. If the Reg* functions guarantee a maximum size use defines from the public headers.
+static const WCHAR fmt_value[] = {' ',' ',' ','%','s',' ','%','s',' ','%','s','\n',0};
This is used in print_value() only, you can move it into this function.
+static const WCHAR no_name[] = { 'n','o','_','n','a','m','e',0 };
Is this used for the default value? Is "no_name" the right choice? Does this need localization?
- hkey = get_rootkey(key_name);
- subkey_name = strchrW(key_name, '\');
- if (NULL == hkey || NULL == subkey_name)
- {
reg_printfW(fmt_err, key_name);return 1;- }
Are you sure that this is the right behavior? I haven't checked, but I can imagine that I can e.g. have a string value directly in HKEY_CURRENT_USER, in which case subkey_name would be NULL.
- value_buffer = HeapAlloc(GetProcessHeap(), 0, MAX_VALUE_BUFFER);
Please check for allocation errors. If you do stay with the fixed size buffer, do you need HeapAlloc at all? Can't you just put this on the stack?
On a related note, passing value_buffer to the two helper functions but not its length, and setting value_len in those functions is ugly. This way the helper functions depend on an implementation detail of their caller. Yeah, the #define makes it a bit better, but it's still not pretty.
- run_reg_exe("reg Query HKCU\" KEY_BASE " /ve", &r);
- ok(r == REG_EXIT_SUCCESS, "got exit code %d, expected 0\n", r);
- run_reg_exe("reg Query HKCU\" KEY_BASE " /v test0", &r);
- ok(r == REG_EXIT_SUCCESS, "got exit code %d, expected 0\n", r);
- run_reg_exe("reg Query HKCU\" KEY_BASE " /s", &r);
- ok(r == REG_EXIT_SUCCESS, "got exit code %d, expected 0\n", r);
- run_reg_exe("reg delete HKCU\" KEY_BASE " /f", &r);
- ok(r == REG_EXIT_SUCCESS, "got exit code %d, expected 0\n", r);
Some tests for error cases would be helpful as well, including all the error checks you have in your code.
I expect scripts to parse the output of this command. Please add some tests that check the output. There's infrastructure for reading command output in other program tests that you can use.
The following are style issues. I'm pointing them out while I'm at it, please fix them as well.
+static void query_sub(HKEY hkey, WCHAR* key_name, WCHAR* value_buffer)
(Style) WCHAR *key_name please. "WCHAR*" is more common in C++.
- HKEY hsubkey = NULL;
- HKEY hkey = NULL;
(Style) Please drop the h from these names.
- ret = RegOpenKeyExW( hkey, subkey_name+1, 0, KEY_READ, &hsubkey );
(Style) inconsistent space after parenthesis.
if (ret == ERROR_SUCCESS )
Same
- static const WCHAR* reg_type[] = {
type_none,type_sz,
(Style) { placement