[PATCH 1/3] PATCHv1: Add generic function to retrieve HKEY string representation.
--- programs/reg/reg.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/programs/reg/reg.c b/programs/reg/reg.c index 8d510f7f7e..405cc2a8be 100644 --- a/programs/reg/reg.c +++ b/programs/reg/reg.c @@ -814,6 +814,75 @@ static int query_all(HKEY key, WCHAR *path, BOOL recurse) return 0; } +static const WCHAR HKCR_WSTR[] = {'H', 'K', 'E', 'Y', '_', 'C', 'L', 'A', 'S', 'S', 'E', 'S', '_', 'R', 'O', 'O', 'T', 0}; +static DWORD HKCR_WSTR_SZ = sizeof(HKCR_WSTR); +static const WCHAR HKLM_WSTR[] = {'H', 'K', 'E', 'Y', '_', 'L', 'O', 'C', 'A', 'L', '_', 'M', 'A', 'C', 'H', 'I', 'N', 'E', 0}; +static DWORD HKLM_WSTR_SZ = sizeof(HKLM_WSTR); +static const WCHAR HKCU_WSTR[] = {'H', 'K', 'E', 'Y', '_', 'C', 'U', 'R', 'R', 'E', 'N', 'T', '_', 'U', 'S', 'E', 'R', 0}; +static DWORD HKCU_WSTR_SZ = sizeof(HKCU_WSTR); +static const WCHAR HKU_WSTR[] = {'H', 'K', 'E', 'Y', '_', 'U', 'S', 'E', 'R', 'S', 0}; +static DWORD HKU_WSTR_SZ = sizeof(HKU_WSTR); + +/* + * Given hkey, return its string representation into buf, who is + * assumed to hold at least (buf_sz * sizeof(*buf)) bytes + * + * Return 0 if successful, other value otherwise + */ +static LONG hkey_to_wstring(HKEY hkey, WCHAR *buf, DWORD buf_sz) +{ + if (hkey == HKEY_LOCAL_MACHINE) + { + if (buf_sz >= HKLM_WSTR_SZ) + { + memcpy(buf, HKLM_WSTR, HKLM_WSTR_SZ); + return 0; + } + else + { + return -1; + } + } + else if (hkey == HKEY_CURRENT_USER) + { + if (buf_sz >= HKCU_WSTR_SZ) + { + memcpy(buf, HKCU_WSTR, HKCU_WSTR_SZ); + return 0; + } + else + { + return -1; + } + } + else if (hkey == HKEY_USERS) + { + if (buf_sz >= HKU_WSTR_SZ) + { + memcpy(buf, HKU_WSTR, HKU_WSTR_SZ); + return 0; + } + else + { + return -1; + } + } + else if (hkey == HKEY_CLASSES_ROOT) + { + if (buf_sz >= HKCR_WSTR_SZ) + { + memcpy(buf, HKCR_WSTR, HKCR_WSTR_SZ); + return 0; + } + else + { + return -1; + } + } + + return -2; +} + static int reg_query(HKEY root, WCHAR *path, WCHAR *key_name, WCHAR *value_name, BOOL value_empty, BOOL recurse) { -- 2.11.0.windows.1
--- programs/reg/reg.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/programs/reg/reg.c b/programs/reg/reg.c index 405cc2a8be..d50740b389 100644 --- a/programs/reg/reg.c +++ b/programs/reg/reg.c @@ -883,6 +883,74 @@ static LONG hkey_to_wstring(HKEY hkey, WCHAR *buf, DWORD buf_sz) return -2; } +typedef struct { + WCHAR *s; + DWORD sz; + DWORD base_sz; +} DisplayString; + +static int create_base_string(DisplayString *display, + HKEY root, WCHAR *remain, + WCHAR *value) +{ + WCHAR *ret, *pret; + WCHAR root_str[255]; + LONG st; + DWORD ret_sz; + + st = hkey_to_wstring(root, root_str, sizeof(root_str)); + if (st) { + return -1; + } + + display->base_sz = strlenW(root_str) + 1 + strlenW(remain) + 1; + + ret_sz = display->base_sz + strlenW(value) + 1; + ret = malloc(ret_sz * sizeof(*ret)); + if (ret == NULL) { + return -1; + } + display->s = ret; + display->sz = ret_sz; + + pret = ret; + memcpy(ret, root_str, + sizeof(*ret) * strlenW(root_str)); + pret += strlenW(root_str); + + pret[0] = '\\'; + pret += 1; + memcpy(pret, remain, strlenW(remain) * sizeof(*remain)); + pret += strlenW(remain); + + pret[0] = '\\'; + pret += 1; + memcpy(pret, value, strlenW(value) * sizeof(*value)); + pret += strlenW(value); + pret[0] = '\0'; + + return 0; +} + +static int resize_display(DisplayString *display, WCHAR* skey) +{ + DWORD skey_sz = strlenW(skey); + WCHAR *pdisplay; + + if(display->sz < (display->base_sz + skey_sz)) { + display->sz = display->base_sz + skey_sz; + display->s = realloc(display->s, sizeof(*display->s) * display->sz); + if(display->s == NULL) { + return -1; + } + } + pdisplay = display->s + display->base_sz; + memcpy(pdisplay, skey, skey_sz * sizeof(*skey)); + pdisplay[skey_sz] = '\0'; + + return 0; +} + static int reg_query(HKEY root, WCHAR *path, WCHAR *key_name, WCHAR *value_name, BOOL value_empty, BOOL recurse) { -- 2.11.0.windows.1
--- programs/reg/reg.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 126 insertions(+), 7 deletions(-) diff --git a/programs/reg/reg.c b/programs/reg/reg.c index d50740b389..e604e79ab7 100644 --- a/programs/reg/reg.c +++ b/programs/reg/reg.c @@ -887,11 +887,11 @@ typedef struct { WCHAR *s; DWORD sz; DWORD base_sz; -} DisplayString; +} DisplayString; static int create_base_string(DisplayString *display, HKEY root, WCHAR *remain, - WCHAR *value) + DWORD max_value_size) { WCHAR *ret, *pret; WCHAR root_str[255]; @@ -905,7 +905,7 @@ static int create_base_string(DisplayString *display, display->base_sz = strlenW(root_str) + 1 + strlenW(remain) + 1; - ret_sz = display->base_sz + strlenW(value) + 1; + ret_sz = display->base_sz + max_value_size + 1; ret = malloc(ret_sz * sizeof(*ret)); if (ret == NULL) { return -1; @@ -924,10 +924,6 @@ static int create_base_string(DisplayString *display, pret += strlenW(remain); pret[0] = '\\'; - pret += 1; - memcpy(pret, value, strlenW(value) * sizeof(*value)); - pret += strlenW(value); - pret[0] = '\0'; return 0; } @@ -951,6 +947,129 @@ static int resize_display(DisplayString *display, WCHAR* skey) return 0; } +const static WCHAR reg_type_str[][20] = { + {NULL}, + {'R', 'E', 'G', '_', 'S', 'Z', 0} +}; + +static int print_key_value(const HKEY hkey, const WCHAR* skey) +{ + static const WCHAR tabbed_ff[] = {' ', ' ', ' ', ' ', '%', 's', + ' ', ' ', ' ', ' ', '%', 's', + ' ', ' ', ' ', ' ', '%', 's', + '\n', 0}; + DWORD type; + BYTE data[256]; + DWORD data_sz = 256, wdata_sz; + WCHAR *data_as_wstr; + DWORD st; + + wdata_sz = data_sz; + st = RegQueryValueExW(hkey, skey, NULL, &type, data, &wdata_sz); + if (st != ERROR_SUCCESS) { + return st; + } + + data[wdata_sz] = '\0'; + data_as_wstr = (WCHAR*)data; + if (type < sizeof(reg_type_str)) { + reg_printfW(tabbed_ff, skey, reg_type_str[type], data_as_wstr); + return 0; + } else { + return 1; + } +} + +static int print_skeys(HKEY root, WCHAR *p, const HKEY hkey) +{ + DWORD st; + int i = 0; + DWORD nskeys, nvalues; + DWORD max_skey_sz, max_value_sz, max_value_data_sz, buf_sz; + WCHAR *skey; + DWORD skey_sz; + DisplayString display; + + static WCHAR empty_line[] = {'\n', 0}; + static const WCHAR ff[] = {'%', 's', '\n', 0}; + + st = RegQueryInfoKeyW( + hkey, // key handle + NULL, // buffer for class name + NULL, // size of class string + NULL, // reserved + &nskeys, // number of subkeys + &max_skey_sz, // longest subkey size + NULL, // longest class string + &nvalues, // number of values for this key + &max_value_sz, // longest value name + &max_value_data_sz, // longest value data + NULL, // security descriptor + NULL); // last write time + + if (st != ERROR_SUCCESS) { + return st; + } + + buf_sz = (max_value_sz > max_skey_sz) ? max_value_sz : + max_skey_sz; + buf_sz += 1; + skey = malloc(sizeof(*skey) * buf_sz); + if (skey==NULL) { + return 1; + } + reg_printfW(empty_line); + + if(create_base_string(&display, root, p, max_value_sz)) { + goto clean_skey; + } + + display.s[display.base_sz-1] = '\0'; + if (nvalues > 0) { + reg_printfW(ff, display.s); + } + display.s[display.base_sz-1] = '\\'; + + for (i = 0; i < nvalues; ++i) { + skey_sz = buf_sz; + skey[0] = '\0'; + st = RegEnumValueW(hkey, i, skey, &skey_sz, NULL, + NULL, NULL, NULL); + if (st != ERROR_SUCCESS) { + goto clean_display; + } + + st = print_key_value(hkey, skey); + if (st) { + goto clean_display; + } + } + + reg_printfW(empty_line); + for (i = 0; i < nskeys; ++i) { + skey_sz = buf_sz; + st = RegEnumKeyExW(hkey, i, skey, &skey_sz, NULL, + NULL, NULL, NULL); + if (st != ERROR_SUCCESS) { + goto clean_display; + } + memcpy(display.s + display.base_sz, skey, skey_sz * sizeof(*skey)); + display.s[display.base_sz + skey_sz] = '\0'; + reg_printfW(ff, display.s); + } + free(display.s); + free(skey); + + return 0; + +clean_display: + free(display.s); +clean_skey: + free(skey); + + return 1; +} + static int reg_query(HKEY root, WCHAR *path, WCHAR *key_name, WCHAR *value_name, BOOL value_empty, BOOL recurse) { -- 2.11.0.windows.1
Hi, and thanks for your submission. I've added some comments in-line. On 2018-01-02 15:14, Piyush Raj wrote:
--- programs/reg/reg.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+)
Your sign-off is missing. Perhaps also add a small explanation of what issue you're fixing or what feature you're implementing to clarify the goal of the patch(es). Ideally you'd also start your commit message with the name of the module ("reg: xxx"). Please see https://wiki.winehq.org/Submitting_Patches#The_commit_message
diff --git a/programs/reg/reg.c b/programs/reg/reg.c index 8d510f7f7e..405cc2a8be 100644 --- a/programs/reg/reg.c +++ b/programs/reg/reg.c @@ -814,6 +814,75 @@ static int query_all(HKEY key, WCHAR *path, BOOL recurse) return 0; }
+static const WCHAR HKCR_WSTR[] = {'H', 'K', 'E', 'Y', '_', 'C', 'L', 'A', 'S', 'S', 'E', 'S', '_', 'R', 'O', 'O', 'T', 0};
Please don't put spaces between the individual characters. All wide strings in Wine are formatted the same way ('A','B','C') so that it's possible to grep for strings. The naming of these variables also seems odd; all uppercase is usually used for macros.
+static DWORD HKCR_WSTR_SZ = sizeof(HKCR_WSTR); +static const WCHAR HKLM_WSTR[] = {'H', 'K', 'E', 'Y', '_', 'L', 'O', 'C', 'A', 'L', '_', 'M', 'A', 'C', 'H', 'I', 'N', 'E', 0}; +static DWORD HKLM_WSTR_SZ = sizeof(HKLM_WSTR); +static const WCHAR HKCU_WSTR[] = {'H', 'K', 'E', 'Y', '_', 'C', 'U', 'R', 'R', 'E', 'N', 'T', '_', 'U', 'S', 'E', 'R', 0}; +static DWORD HKCU_WSTR_SZ = sizeof(HKCU_WSTR); +static const WCHAR HKU_WSTR[] = {'H', 'K', 'E', 'Y', '_', 'U', 'S', 'E', 'R', 'S', 0}; +static DWORD HKU_WSTR_SZ = sizeof(HKU_WSTR); + +/* + * Given hkey, return its string representation into buf, who is + * assumed to hold at least (buf_sz * sizeof(*buf)) bytes
You compare buf_sz to HKLM_WSTR_SZ and friends, which are in bytes. Given how you call the function in patch 2, buf_sz really is in bytes, not characters.
+ * + * Return 0 if successful, other value otherwise + */ +static LONG hkey_to_wstring(HKEY hkey, WCHAR *buf, DWORD buf_sz)
This function isn't used, so you're introducing dead code. It's generally preferred to add code only when it is needed. In fact from what I can see, all of your code throughout the patch series seems to be unused. It would be better to introduce it along with the changes that make use of the new functions.
+{ + if (hkey == HKEY_LOCAL_MACHINE) + { + if (buf_sz >= HKLM_WSTR_SZ) + { + memcpy(buf, HKLM_WSTR, HKLM_WSTR_SZ);
I'd personally find this easier to read if it used sizeof directly, without the additional size variable, but that's a matter of taste I suppose. Best, Thomas
Hi Piyush, So we can help you further with your patches, could you let us know what your patches are supposed to do? Is there a bug in Wine's reg.exe utililty you are trying to fix? Or are you trying to implement some kind of helper function to improve the code logic? And could you tell us a litle bit about your programming experience as well? Unfortunately, I don't have time for an extended reply today. But your code base seems rather out-of-date. For example, the function reg_printfW() has not been included in Wine for a long time. I also noticed you are using git for Windows. That will make it difficult and inconvenient for you to test your own patches. -- Hugh McMaster
participants (3)
-
Hugh McMaster -
Piyush Raj -
Thomas Faber