New registry key adding patch

Dimitrie O. Paun dpaun at rogers.com
Wed Dec 31 12:12:17 CST 2003


On December 31, 2003 11:18 am, Zimler Attila wrote:

Cool, here are a few comments on the patch:

> + /* Modified by Attila ZIMLER <hijaszu at hlfslinux.hu> on 28th of dec 2003.
> +     - New key adding is now supported.
> + */

Please don't add such stuff in the headers, this goes automatically in
the CVS log, where it belongs. Otherwise we risk cluttering up the files.

> +     /* try to find out a name for the newly create key */
> +     /* TODO: warning is issued because asprintf's implicit declaration,
> +       I tried to declare it as
> +       extern "C" ssize_t asprintf(char**, const char**, ...)
> +       which is the format of this function, but it seems not helping.
> +       Don't know why :(
> +       */
> +     asprintf(&keyName, "new key");

Please don't use GNU extensions like asprintf. We can with a
local variable:
    TCHAR keyName[32];

> +     lRet = RegOpenKey(hKey, keyName, &retKey);
> +     while (lRet == ERROR_SUCCESS) {
> +           asprintf(&keyName, "new key %u", ++keyNum);
> +           lRet = RegOpenKey(hKey, keyName, &retKey);
> +     }

I'd add a counter to the loop, and exit after, say, 100 tries.

> +     case ID_EDIT_NEW_KEY:
> +       if (CreateKey(hKey))
> +           RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, keyPath);
> +       break;

As I said, no need to refresh the ListView here. Maybe you should just
create a dummy RefreshTreeView() function that just prints a warning
for now.

-- 
Dimi.




More information about the wine-devel mailing list