New registry key adding patch

[Zimler Attila]

Hi,

I made the regedit program able to add new registry key (however, the 
key does not appear until next run of regedit in the browser tree - as I 
consulted it with Dimitrie, it will be handled later).

The diff is made against todays cvs tree (as 2003 december 31).

Attila

PS: Dimitrie: what is the next step to do?

diff -r -d -c -N wine/programs/regedit/edit.c wine.new_regkey/programs/regedit/edit.c
*** wine/programs/regedit/edit.c	2003-12-12 04:08:59.000000000 +0000
--- wine.new_regkey/programs/regedit/edit.c	2003-12-31 14:36:44.000000000 +0000
***************
*** 17,22 ****
--- 17,25 ----
   * License along with this library; if not, write to the Free Software
   * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
   */
+ /* Modified by Attila ZIMLER  on 28th of dec 2003.
+     - New key adding is now supported.
+ */

#define WIN32_LEAN_AND_MEAN     /* Exclude rarely-used stuff from Windows headers */

***************
*** 91,96 ****
--- 94,129 ----
      return FALSE;
  }

+ BOOL CreateKey(HKEY hKey)
+ {
+     LONG lRet;
+     HKEY retKey;
+     char* keyName = 0;
+     unsigned int keyNum = 1;
+          
+     /* If we have illegal parameter return with operation failure */
+     if (!hKey) return FALSE;
+     
+     /* 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");
+     lRet = RegOpenKey(hKey, keyName, &retKey);
+     while (lRet == ERROR_SUCCESS) {
+ 	    asprintf(&keyName, "new key %u", ++keyNum);
+ 	    lRet = RegOpenKey(hKey, keyName, &retKey);
+     }
+     
+     lRet = RegCreateKey(hKey, keyName, &retKey);
+     
+     free(keyName);
+     return lRet == ERROR_SUCCESS;
+ }
+ 
  BOOL ModifyValue(HWND hwnd, HKEY hKey, LPCTSTR valueName)
  {
      DWORD valueDataLen;
diff -r -d -c -N wine/programs/regedit/framewnd.c wine.new_regkey/programs/regedit/framewnd.c
*** wine/programs/regedit/framewnd.c	2003-12-12 04:08:59.000000000 +0000
--- wine.new_regkey/programs/regedit/framewnd.c	2003-12-31 14:38:34.000000000 +0000
***************
*** 469,474 ****
--- 469,478 ----
      case ID_EDIT_COPYKEYNAME:
          CopyKeyName(hWnd, _T(""));
          break;
+     case ID_EDIT_NEW_KEY:
+ 	if (CreateKey(hKey))
+ 	    RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, keyPath);
+ 	break;
      case ID_REGISTRY_PRINTERSETUP:
          /*PRINTDLG pd;*/
          /*PrintDlg(&pd);*/
diff -r -d -c -N wine/programs/regedit/main.h wine.new_regkey/programs/regedit/main.h
*** wine/programs/regedit/main.h	2003-12-12 04:08:59.000000000 +0000
--- wine.new_regkey/programs/regedit/main.h	2003-12-31 14:39:57.000000000 +0000
***************
*** 93,98 ****
--- 93,99 ----
  extern LPCTSTR GetItemPath(HWND hwndTV, HTREEITEM hItem, HKEY* phRootKey);

/* edit.c */
+ BOOL CreateKey(HKEY hKey);
  BOOL ModifyValue(HWND hwnd, HKEY hKey, LPCTSTR valueName);

#endif /* __MAIN_H__ */

[Dimitrie O. Paun]

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

Cool, here are a few comments on the patch:
> - /* Modified by Attila ZIMLER  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.

[Zimler Attila]

Dimitrie O. Paun wrote:
> On December 31, 2003 11:18 am, Zimler Attila wrote:
>
> Cool, here are a few comments on the patch:
> > - /* Modified by Attila ZIMLER  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.

ok, I will replace the code marked bad :) but I will send code only in 
next year ;)

Attila

[Zimler Attila]

Hi,
I tried to remove objectionable codes. Here is the patch again.

I used sprintf, because snprintf gives me a warning, that it is 
explicitly declared (stdio.h is included). This is working while "key 
name" string is not localized. After that this could introduce security 
bugs. Will it be localized? How to deal with situations like this?

Attila

diff -c -d -r -N wine/programs/regedit/edit.c wine.new_regkey/programs/regedit/edit.c
*** wine/programs/regedit/edit.c	2003-12-12 04:08:59.000000000 +0000
--- wine.new_regkey/programs/regedit/edit.c	2004-01-01 11:30:33.000000000 +0000
***************
*** 91,96 ****
--- 91,120 ----
      return FALSE;
  }

+ BOOL CreateKey(HKEY hKey)
+ {
+     LONG lRet;
+     HKEY retKey;
+     TCHAR keyName[32];
+     unsigned int keyNum = 1;
+          
+     /* If we have illegal parameter return with operation failure */
+     if (!hKey) return FALSE;
+     
+     /* try to find out a name for the newly create key.
+ 	We try it max 100 times. */
+     sprintf(keyName, "new key");
+     lRet = RegOpenKey(hKey, keyName, &retKey);
+     while (lRet == ERROR_SUCCESS && keyNum < 100) {
+ 	    sprintf(keyName, "new key %u", ++keyNum);
+ 	    lRet = RegOpenKey(hKey, keyName, &retKey);
+     }
+     if (lRet != ERROR_SUCCESS) return FALSE;
+     
+     lRet = RegCreateKey(hKey, keyName, &retKey);
+     return lRet == ERROR_SUCCESS;
+ }
+ 
  BOOL ModifyValue(HWND hwnd, HKEY hKey, LPCTSTR valueName)
  {
      DWORD valueDataLen;
diff -c -d -r -N wine/programs/regedit/framewnd.c wine.new_regkey/programs/regedit/framewnd.c
*** wine/programs/regedit/framewnd.c	2003-12-12 04:08:59.000000000 +0000
--- wine.new_regkey/programs/regedit/framewnd.c	2004-01-01 11:13:17.000000000 +0000
***************
*** 469,474 ****
--- 469,477 ----
      case ID_EDIT_COPYKEYNAME:
          CopyKeyName(hWnd, _T(""));
          break;
+     case ID_EDIT_NEW_KEY:
+ 	CreateKey(hKey);
+ 	break;
      case ID_REGISTRY_PRINTERSETUP:
          /*PRINTDLG pd;*/
          /*PrintDlg(&pd);*/
diff -c -d -r -N wine/programs/regedit/main.h wine.new_regkey/programs/regedit/main.h
*** wine/programs/regedit/main.h	2003-12-12 04:08:59.000000000 +0000
--- wine.new_regkey/programs/regedit/main.h	2004-01-01 11:04:41.000000000 +0000
***************
*** 93,98 ****
--- 93,99 ----
  extern LPCTSTR GetItemPath(HWND hwndTV, HTREEITEM hItem, HKEY* phRootKey);

/* edit.c */
+ BOOL CreateKey(HKEY hKey);
  BOOL ModifyValue(HWND hwnd, HKEY hKey, LPCTSTR valueName);

#endif /* __MAIN_H__ */

[Dimitrie O. Paun]

On January 1, 2004 06:41 am, Zimler Attila wrote:
> This is working while "key name" string is not localized. 
> After that this could introduce security bugs. 
> Will it be localized? How to deal with situations like this?

Well, we would need to allocate the buffer dynamically anyway.

Please use uniffied diffs (diff -u) when submitting patches.
> -
> /* try to find out a name for the newly create key.
> -
>   We try it max 100 times. */
> -
> sprintf(keyName, "new key");

You don't need this sprintf here
> -
> lRet = RegOpenKey(hKey, keyName, &retKey);
> -
> while (lRet == ERROR_SUCCESS && keyNum < 100) {
> -
>       sprintf(keyName, "new key %u", ++keyNum);
> -
>       lRet = RegOpenKey(hKey, keyName, &retKey);
> -
> }
> -
> if (lRet != ERROR_SUCCESS) return FALSE;

This condition is not right. We exit the loop when
lRet != ERROR_SUCCESS which means we always return FALSE.

[Zimler Attila]

Hi,
I give it a third try :)

Dimitrie O. Paun wrote:
> On January 1, 2004 06:41 am, Zimler Attila wrote:
> > This is working while "key name" string is not localized. 
> > After that this could introduce security bugs. 
> > Will it be localized? How to deal with situations like this?
>
> Well, we would need to allocate the buffer dynamically anyway.

I don't understand this. Probably because I don't understand how we 
translate the "key name" string, to the localized version, and because 
this I have no idea how big memory should we allocate.
> Please use uniffied diffs (diff -u) when submitting patches.

cvs diff -u is ok? (This is how I did it at this time)

Hope everything is ok (except localization) at this time.
Attila

Index: programs/regedit/edit.c
===================================================================
RCS file: /home/wine/wine/programs/regedit/edit.c,v
retrieving revision 1.3
diff -u -u -r1.3 edit.c
--- programs/regedit/edit.c	12 Dec 2003 04:08:59 -0000	1.3
+++ programs/regedit/edit.c	1 Jan 2004 12:06:32 -0000
@@ -91,6 +91,29 @@
     return FALSE;
 }

+BOOL CreateKey(HKEY hKey)
+{
+    LONG lRet;
+    HKEY retKey;
+    TCHAR keyName[32] = "new key";
+    unsigned int keyNum = 1;
+         
+    /* If we have illegal parameter return with operation failure */
+    if (!hKey) return FALSE;
+    
+    /* try to find out a name for the newly create key.
+	We try it max 100 times. */
+    lRet = RegOpenKey(hKey, keyName, &retKey);
+    while (lRet == ERROR_SUCCESS && keyNum < 100) {
+	    sprintf(keyName, "new key %u", ++keyNum);
+	    lRet = RegOpenKey(hKey, keyName, &retKey);
+    }
+    if (lRet == ERROR_SUCCESS) return FALSE;
+    
+    lRet = RegCreateKey(hKey, keyName, &retKey);
+    return lRet == ERROR_SUCCESS;
+}
+
 BOOL ModifyValue(HWND hwnd, HKEY hKey, LPCTSTR valueName)
 {
     DWORD valueDataLen;
Index: programs/regedit/framewnd.c
===================================================================
RCS file: /home/wine/wine/programs/regedit/framewnd.c,v
retrieving revision 1.5
diff -u -u -r1.5 framewnd.c
--- programs/regedit/framewnd.c	12 Dec 2003 04:08:59 -0000	1.5
+++ programs/regedit/framewnd.c	1 Jan 2004 12:06:32 -0000
@@ -469,6 +469,9 @@
     case ID_EDIT_COPYKEYNAME:
         CopyKeyName(hWnd, _T(""));
         break;
+    case ID_EDIT_NEW_KEY:
+	CreateKey(hKey);
+	break;
     case ID_REGISTRY_PRINTERSETUP:
         /*PRINTDLG pd;*/
         /*PrintDlg(&pd);*/
Index: programs/regedit/main.h
===================================================================
RCS file: /home/wine/wine/programs/regedit/main.h,v
retrieving revision 1.8
diff -u -u -r1.8 main.h
--- programs/regedit/main.h	12 Dec 2003 04:08:59 -0000	1.8
+++ programs/regedit/main.h	1 Jan 2004 12:06:32 -0000
@@ -93,6 +93,7 @@
 extern LPCTSTR GetItemPath(HWND hwndTV, HTREEITEM hItem, HKEY* phRootKey);

/* edit.c */
+BOOL CreateKey(HKEY hKey);
 BOOL ModifyValue(HWND hwnd, HKEY hKey, LPCTSTR valueName);

#endif /* __MAIN_H__ */

[Dimitrie O. Paun]

On January 1, 2004 08:16 am, Zimler Attila wrote:
> I don't understand this. Probably because I don't understand how we
> translate the "key name" string, to the localized version, and because
> this I have no idea how big memory should we allocate.

You use LoadString() to load the string (look at how we load
error strings in edit.c even though there it's simple since
the buffer is fixed size for now).
> cvs diff -u is ok? (This is how I did it at this time)

Yes, this is perfect.
> Hope everything is ok (except localization) at this time.

It looks like it. Please send it to wine-patches for inclusion
in the tree.

[Zimler Attila]

Dimitrie O. Paun wrote:
> You use LoadString() to load the string (look at how we load
> error strings in edit.c even though there it's simple since
> the buffer is fixed size for now).

Am I loaded correctly the string? Could you verify it for me? If yes, I 
will send the patch for including in the CVS tree. If yes, could you 
tell me also, which is the next task, I could do?

Attila

Index: programs/regedit/En.rc
===================================================================
RCS file: /home/wine/wine/programs/regedit/En.rc,v
retrieving revision 1.3
diff -u -u -r1.3 En.rc
--- programs/regedit/En.rc	3 Dec 2003 20:25:24 -0000	1.3
+++ programs/regedit/En.rc	1 Jan 2004 18:27:02 -0000
@@ -201,6 +201,7 @@
     IDS_BAD_VALUE           "Can't query value '%s'"
     IDS_UNSUPPORTED_TYPE    "Can't edit keys of this type (%ld)"
     IDS_TOO_BIG_VALUE       "Value is too big (%ld)"
+    IDS_NEWKEY              "new key"
 END

/*****************************************************************/
Index: programs/regedit/edit.c
===================================================================
RCS file: /home/wine/wine/programs/regedit/edit.c,v
retrieving revision 1.3
diff -u -u -r1.3 edit.c
--- programs/regedit/edit.c	12 Dec 2003 04:08:59 -0000	1.3
+++ programs/regedit/edit.c	1 Jan 2004 18:27:02 -0000
@@ -91,6 +91,40 @@
     return FALSE;
 }

+BOOL CreateKey(HKEY hKey)
+{
+    LONG lRet;
+    HKEY retKey;
+    TCHAR keyName[32];
+    static TCHAR newKey[28] = ""; /* should be max keyName len - 4 */
+    HINSTANCE hInstance;
+    unsigned int keyNum = 1;
+         
+    /* If we have illegal parameter return with operation failure */
+    if (!hKey) return FALSE;
+
+    /* Load localized "new key" string. -4 is because we need max 4 character
+	to numbering. */
+    if (newKey[0] == 0) {
+	hInstance = GetModuleHandle(0);
+	if (!LoadString(hInstance, IDS_NEWKEY, newKey, COUNT_OF(newKey)))
+    	    lstrcpy(newKey, "new key");
+    }
+    lstrcpy(keyName, newKey);
+
+    /* try to find out a name for the newly create key.
+	We try it max 100 times. */
+    lRet = RegOpenKey(hKey, keyName, &retKey);
+    while (lRet == ERROR_SUCCESS && keyNum < 100) {
+	    sprintf(keyName, "%s %u", newKey, ++keyNum);
+	    lRet = RegOpenKey(hKey, keyName, &retKey);
+    }
+    if (lRet == ERROR_SUCCESS) return FALSE;
+    
+    lRet = RegCreateKey(hKey, keyName, &retKey);
+    return lRet == ERROR_SUCCESS;
+}
+
 BOOL ModifyValue(HWND hwnd, HKEY hKey, LPCTSTR valueName)
 {
     DWORD valueDataLen;
Index: programs/regedit/framewnd.c
===================================================================
RCS file: /home/wine/wine/programs/regedit/framewnd.c,v
retrieving revision 1.5
diff -u -u -r1.5 framewnd.c
--- programs/regedit/framewnd.c	12 Dec 2003 04:08:59 -0000	1.5
+++ programs/regedit/framewnd.c	1 Jan 2004 18:27:02 -0000
@@ -469,6 +469,9 @@
     case ID_EDIT_COPYKEYNAME:
         CopyKeyName(hWnd, _T(""));
         break;
+    case ID_EDIT_NEW_KEY:
+	CreateKey(hKey);
+	break;
     case ID_REGISTRY_PRINTERSETUP:
         /*PRINTDLG pd;*/
         /*PrintDlg(&pd);*/
Index: programs/regedit/main.h
===================================================================
RCS file: /home/wine/wine/programs/regedit/main.h,v
retrieving revision 1.8
diff -u -u -r1.8 main.h
--- programs/regedit/main.h	12 Dec 2003 04:08:59 -0000	1.8
+++ programs/regedit/main.h	1 Jan 2004 18:27:02 -0000
@@ -93,6 +93,7 @@
 extern LPCTSTR GetItemPath(HWND hwndTV, HTREEITEM hItem, HKEY* phRootKey);

/* edit.c */
+BOOL CreateKey(HKEY hKey);
 BOOL ModifyValue(HWND hwnd, HKEY hKey, LPCTSTR valueName);

#endif /* __MAIN_H__ */
Index: programs/regedit/resource.h
===================================================================
RCS file: /home/wine/wine/programs/regedit/resource.h,v
retrieving revision 1.2
diff -u -u -r1.2 resource.h
--- programs/regedit/resource.h	3 Dec 2003 20:25:24 -0000	1.2
+++ programs/regedit/resource.h	1 Jan 2004 18:27:02 -0000
@@ -106,6 +106,7 @@
 #define IDS_BAD_VALUE			32837
 #define IDS_UNSUPPORTED_TYPE		32838
 #define IDS_TOO_BIG_VALUE		32839
+#define IDS_NEWKEY			32840

#define IDD_EDIT_STRING			2000
 #define IDC_VALUE_NAME			2001

[Dimitrie O. Paun]

On January 1, 2004 02:35 pm, Zimler Attila wrote:
> Am I loaded correctly the string? Could you verify it for me? If yes, I
> will send the patch for including in the CVS tree.

It seems so, yes. However, I'd change the "new key" string to
"New Key" to match the native one (it's also nicer).
> If yes, could you tell me also, which is the next task, I could do?

OK, I propose the following tasks (in this order):
  -- new value
  -- rename value
  -- rename key