windows/storage.c: factor out low-level Registry access.

All the fiddly business where you have to check that a thing exists,
make sure of its type, find its size, allocate some memory, and then
read it again properly (or, alternatively, loop round dealing with
ERROR_MORE_DATA) just doesn't belong at every call site. It's crying
out to be moved out into some separate utility functions that present
a more ergonomic API, so that the code that decides _which_ Registry
entries to read and what to do with them can concentrate on that.

So I've written a fresh set of registry API wrappers in windows/utils,
and simplified windows/storage.c as a result. The jump-list handling
code in particular is almost legible now!

windows/CMakeLists.txt

@@ -26,7 +26,7 @@ add_sources_from_current_dir(utils
   utils/open_for_write_would_lose_data.c
   utils/pgp_fingerprints_msgbox.c
   utils/platform_get_x_display.c
-  utils/registry_get_string.c
+  utils/registry.c
   utils/request_file.c
   utils/screenshot.c
   utils/security.c

windows/help.c

@@ -149,7 +149,7 @@ static bool find_chm_from_installation(void)
     };
 
     for (size_t i = 0; i < lenof(reg_paths); i++) {
-        char *filename = registry_get_string(
+        char *filename = get_reg_sz_simple(
             HKEY_LOCAL_MACHINE, reg_paths[i], NULL);
 
         if (filename) {

windows/platform.h

@@ -716,7 +716,20 @@ char *get_jumplist_registry_entries(void);
 #define CLIPUI_DEFAULT_INS CLIPUI_EXPLICIT
 
 /* In utils */
-char *registry_get_string(HKEY root, const char *path, const char *leaf);
+HKEY open_regkey_fn(bool create, HKEY base, const char *path, ...);
+#define open_regkey(create, base, ...) \
+    open_regkey_fn(create, base, __VA_ARGS__, (const char *)NULL)
+void close_regkey(HKEY key);
+void del_regkey(HKEY key, const char *name);
+char *enum_regkey(HKEY key, int index);
+bool get_reg_dword(HKEY key, const char *name, DWORD *out);
+bool put_reg_dword(HKEY key, const char *name, DWORD value);
+char *get_reg_sz(HKEY key, const char *name);
+bool put_reg_sz(HKEY key, const char *name, const char *str);
+strbuf *get_reg_multi_sz(HKEY key, const char *name);
+bool put_reg_multi_sz(HKEY key, const char *name, strbuf *str);
+
+char *get_reg_sz_simple(HKEY key, const char *name, const char *leaf);
 
 /* In cliloop.c */
 typedef bool (*cliloop_pre_t)(void *vctx, const HANDLE **extra_handles,

windows/storage.c

@@ -33,28 +33,16 @@ struct settings_w {
 
 settings_w *open_settings_w(const char *sessionname, char **errmsg)
 {
-    HKEY subkey1, sesskey;
-    int ret;
-    strbuf *sb;
-
     *errmsg = NULL;
 
     if (!sessionname || !*sessionname)
         sessionname = "Default Settings";
 
-    sb = strbuf_new();
+    strbuf *sb = strbuf_new();
     escape_registry_key(sessionname, sb);
 
-    ret = RegCreateKey(HKEY_CURRENT_USER, puttystr, &subkey1);
+    HKEY sesskey = open_regkey(true, HKEY_CURRENT_USER, puttystr, sb->s);
-    if (ret != ERROR_SUCCESS) {
+    if (!sesskey) {
-        strbuf_free(sb);
-        *errmsg = dupprintf("Unable to create registry key\n"
-                            "HKEY_CURRENT_USER\\%s", puttystr);
-        return NULL;
-    }
-    ret = RegCreateKey(subkey1, sb->s, &sesskey);
-    RegCloseKey(subkey1);
-    if (ret != ERROR_SUCCESS) {
         *errmsg = dupprintf("Unable to create registry key\n"
                             "HKEY_CURRENT_USER\\%s\\%s", puttystr, sb->s);
         strbuf_free(sb);
@@ -70,20 +58,18 @@ settings_w *open_settings_w(const char *sessionname, char **errmsg)
 void write_setting_s(settings_w *handle, const char *key, const char *value)
 {
     if (handle)
-        RegSetValueEx(handle->sesskey, key, 0, REG_SZ, (CONST BYTE *)value,
+        put_reg_sz(handle->sesskey, key, value);
-                      1 + strlen(value));
 }
 
 void write_setting_i(settings_w *handle, const char *key, int value)
 {
     if (handle)
-        RegSetValueEx(handle->sesskey, key, 0, REG_DWORD,
+        put_reg_dword(handle->sesskey, key, value);
-                      (CONST BYTE *) &value, sizeof(value));
 }
 
 void close_settings_w(settings_w *handle)
 {
-    RegCloseKey(handle->sesskey);
+    close_regkey(handle->sesskey);
     sfree(handle);
 }
 
@@ -93,24 +79,12 @@ struct settings_r {
 
 settings_r *open_settings_r(const char *sessionname)
 {
-    HKEY subkey1, sesskey;
-    strbuf *sb;
-
     if (!sessionname || !*sessionname)
         sessionname = "Default Settings";
 
-    sb = strbuf_new();
+    strbuf *sb = strbuf_new();
     escape_registry_key(sessionname, sb);
-
+    HKEY sesskey = open_regkey(false, HKEY_CURRENT_USER, puttystr, sb->s);
-    if (RegOpenKey(HKEY_CURRENT_USER, puttystr, &subkey1) != ERROR_SUCCESS) {
-        sesskey = NULL;
-    } else {
-        if (RegOpenKey(subkey1, sb->s, &sesskey) != ERROR_SUCCESS) {
-            sesskey = NULL;
-        }
-        RegCloseKey(subkey1);
-    }
-
     strbuf_free(sb);
 
     if (!sesskey)
@@ -123,42 +97,15 @@ settings_r *open_settings_r(const char *sessionname)
 
 char *read_setting_s(settings_r *handle, const char *key)
 {
-    DWORD type, allocsize, size;
-    char *ret;
-
     if (!handle)
         return NULL;
-
+    return get_reg_sz(handle->sesskey, key);
-    /* Find out the type and size of the data. */
-    if (RegQueryValueEx(handle->sesskey, key, 0,
-                        &type, NULL, &size) != ERROR_SUCCESS ||
-        type != REG_SZ)
-        return NULL;
-
-    allocsize = size+1;         /* allow for an extra NUL if needed */
-    ret = snewn(allocsize, char);
-    if (RegQueryValueEx(handle->sesskey, key, 0,
-                        &type, (BYTE *)ret, &size) != ERROR_SUCCESS ||
-        type != REG_SZ) {
-        sfree(ret);
-        return NULL;
-    }
-    assert(size < allocsize);
-    ret[size] = '\0'; /* add an extra NUL in case RegQueryValueEx
-                       * didn't supply one */
-
-    return ret;
 }
 
 int read_setting_i(settings_r *handle, const char *key, int defvalue)
 {
-    DWORD type, val, size;
+    DWORD val;
-    size = sizeof(val);
+    if (!handle || !get_reg_dword(handle->sesskey, key, &val))
-
-    if (!handle ||
-        RegQueryValueEx(handle->sesskey, key, 0, &type,
-                        (BYTE *) &val, &size) != ERROR_SUCCESS ||
-        size != sizeof(val) || type != REG_DWORD)
         return defvalue;
     else
         return val;
@@ -241,25 +188,23 @@ void write_setting_filename(settings_w *handle,
 void close_settings_r(settings_r *handle)
 {
     if (handle) {
-        RegCloseKey(handle->sesskey);
+        close_regkey(handle->sesskey);
         sfree(handle);
     }
 }
 
 void del_settings(const char *sessionname)
 {
-    HKEY subkey1;
+    HKEY rkey = open_regkey(false, HKEY_CURRENT_USER, puttystr);
-    strbuf *sb;
+    if (!rkey)
-
-    if (RegOpenKey(HKEY_CURRENT_USER, puttystr, &subkey1) != ERROR_SUCCESS)
         return;
 
-    sb = strbuf_new();
+    strbuf *sb = strbuf_new();
     escape_registry_key(sessionname, sb);
-    RegDeleteKey(subkey1, sb->s);
+    del_regkey(rkey, sb->s);
     strbuf_free(sb);
 
-    RegCloseKey(subkey1);
+    close_regkey(rkey);
 
     remove_session_from_jumplist(sessionname);
 }
@@ -271,13 +216,11 @@ struct settings_e {
 
 settings_e *enum_settings_start(void)
 {
-    settings_e *ret;
+    HKEY key = open_regkey(false, HKEY_CURRENT_USER, puttystr);
-    HKEY key;
+    if (!key)
-
-    if (RegOpenKey(HKEY_CURRENT_USER, puttystr, &key) != ERROR_SUCCESS)
         return NULL;
 
-    ret = snew(settings_e);
+    settings_e *ret = snew(settings_e);
     if (ret) {
         ret->key = key;
         ret->i = 0;
@@ -288,30 +231,19 @@ settings_e *enum_settings_start(void)
 
 bool enum_settings_next(settings_e *e, strbuf *sb)
 {
-    size_t regbuf_size = MAX_PATH + 1;
+    char *name = enum_regkey(e->key, e->i);
-    char *regbuf = snewn(regbuf_size, char);
+    if (!name)
-    bool success;
+        return false;
-
-    while (1) {
-        DWORD retd = RegEnumKey(e->key, e->i, regbuf, regbuf_size);
-        if (retd != ERROR_MORE_DATA) {
-            success = (retd == ERROR_SUCCESS);
-            break;
-        }
-        sgrowarray(regbuf, regbuf_size, regbuf_size);
-    }
-
-    if (success)
-        unescape_registry_key(regbuf, sb);
 
+    unescape_registry_key(name, sb);
+    sfree(name);
     e->i++;
-    sfree(regbuf);
+    return true;
-    return success;
 }
 
 void enum_settings_finish(settings_e *e)
 {
-    RegCloseKey(e->key);
+    close_regkey(e->key);
     sfree(e);
 }
 
@@ -325,48 +257,30 @@ static void hostkey_regname(strbuf *sb, const char *hostname,
 int check_stored_host_key(const char *hostname, int port,
                           const char *keytype, const char *key)
 {
-    char *otherstr;
-    strbuf *regname;
-    int len;
-    HKEY rkey;
-    DWORD readlen;
-    DWORD type;
-    int ret, compare;
-
-    len = 1 + strlen(key);
-
     /*
-     * Now read a saved key in from the registry and see what it
+     * Read a saved key in from the registry and see what it says.
-     * says.
      */
-    regname = strbuf_new();
+    strbuf *regname = strbuf_new();
     hostkey_regname(regname, hostname, port, keytype);
 
-    if (RegOpenKey(HKEY_CURRENT_USER, PUTTY_REG_POS "\\SshHostKeys",
+    HKEY rkey = open_regkey(false, HKEY_CURRENT_USER,
-                   &rkey) != ERROR_SUCCESS) {
+                            PUTTY_REG_POS "\\SshHostKeys");
+    if (!rkey) {
         strbuf_free(regname);
         return 1;                      /* key does not exist in registry */
     }
 
-    readlen = len;
+    char *otherstr = get_reg_sz(rkey, regname->s);
-    otherstr = snewn(len, char);
+    if (!otherstr && !strcmp(keytype, "rsa")) {
-    ret = RegQueryValueEx(rkey, regname->s, NULL,
-                          &type, (BYTE *)otherstr, &readlen);
-
-    if (ret != ERROR_SUCCESS && ret != ERROR_MORE_DATA &&
-        !strcmp(keytype, "rsa")) {
         /*
          * Key didn't exist. If the key type is RSA, we'll try
          * another trick, which is to look up the _old_ key format
          * under just the hostname and translate that.
          */
         char *justhost = regname->s + 1 + strcspn(regname->s, ":");
-        char *oldstyle = snewn(len + 10, char); /* safety margin */
+        char *oldstyle = get_reg_sz(rkey, justhost);
-        readlen = len;
-        ret = RegQueryValueEx(rkey, justhost, NULL, &type,
-                              (BYTE *)oldstyle, &readlen);
 
-        if (ret == ERROR_SUCCESS && type == REG_SZ) {
+        if (oldstyle) {
             /*
              * The old format is two old-style bignums separated by
              * a slash. An old-style bignum is made of groups of
@@ -410,25 +324,23 @@ int check_stored_host_key(const char *hostname, int port,
              * wrong, and hyper-cautiously do nothing.
              */
             if (!strcmp(otherstr, key))
-                RegSetValueEx(rkey, regname->s, 0, REG_SZ, (BYTE *)otherstr,
+                put_reg_sz(rkey, regname->s, otherstr);
-                              strlen(otherstr) + 1);
         }
 
         sfree(oldstyle);
     }
 
-    RegCloseKey(rkey);
+    close_regkey(rkey);
 
-    compare = strcmp(otherstr, key);
+    int compare = otherstr ? strcmp(otherstr, key) : -1;
 
     sfree(otherstr);
     strbuf_free(regname);
 
-    if (ret == ERROR_MORE_DATA ||
+    if (!otherstr)
-        (ret == ERROR_SUCCESS && type == REG_SZ && compare))
-        return 2;                      /* key is different in registry */
-    else if (ret != ERROR_SUCCESS || type != REG_SZ)
         return 1;                      /* key does not exist in registry */
+    else if (compare)
+        return 2;                      /* key is different in registry */
     else
         return 0;                      /* key matched OK in registry */
 }
@@ -446,17 +358,14 @@ bool have_ssh_host_key(const char *hostname, int port,
 void store_host_key(const char *hostname, int port,
                     const char *keytype, const char *key)
 {
-    strbuf *regname;
+    strbuf *regname = strbuf_new();
-    HKEY rkey;
-
-    regname = strbuf_new();
     hostkey_regname(regname, hostname, port, keytype);
 
-    if (RegCreateKey(HKEY_CURRENT_USER, PUTTY_REG_POS "\\SshHostKeys",
+    HKEY rkey = open_regkey(true, HKEY_CURRENT_USER,
-                     &rkey) == ERROR_SUCCESS) {
+                            PUTTY_REG_POS "\\SshHostKeys");
-        RegSetValueEx(rkey, regname->s, 0, REG_SZ,
+    if (rkey) {
-                      (BYTE *)key, strlen(key) + 1);
+        put_reg_sz(rkey, regname->s, key);
-        RegCloseKey(rkey);
+        close_regkey(rkey);
     } /* else key does not exist in registry */
 
     strbuf_free(regname);
@@ -498,7 +407,6 @@ static bool try_random_seed_and_free(char *path, int action, HANDLE *hout)
 
 static HANDLE access_random_seed(int action)
 {
-    HKEY rkey;
     HANDLE rethandle;
 
     /*
@@ -517,16 +425,16 @@ static HANDLE access_random_seed(int action)
      * Registry, if any.
      */
     {
-        char regpath[MAX_PATH + 1];
+        HKEY rkey = open_regkey(false, HKEY_CURRENT_USER, PUTTY_REG_POS);
-        DWORD type, size = sizeof(regpath);
+        if (rkey) {
-        if (RegOpenKey(HKEY_CURRENT_USER, PUTTY_REG_POS, &rkey) ==
+            char *regpath = get_reg_sz(rkey, "RandSeedFile");
-            ERROR_SUCCESS) {
+            close_regkey(rkey);
-            int ret = RegQueryValueEx(rkey, "RandSeedFile",
+            if (regpath) {
-                                      0, &type, (BYTE *)regpath, &size);
+                bool success = try_random_seed(regpath, action, &rethandle);
-            RegCloseKey(rkey);
+                sfree(regpath);
-            if (ret == ERROR_SUCCESS && type == REG_SZ &&
+                if (success)
-                try_random_seed(regpath, action, &rethandle))
+                    return rethandle;
-                return rethandle;
+            }
         }
     }
 
@@ -644,129 +552,66 @@ void write_random_seed(void *data, int len)
 static int transform_jumplist_registry
     (const char *add, const char *rem, char **out)
 {
-    int ret;
+    HKEY rkey = open_regkey(true, HKEY_CURRENT_USER, reg_jumplist_key);
-    HKEY pjumplist_key;
+    if (!rkey)
-    DWORD type;
-    DWORD value_length;
-    char *old_value, *new_value;
-    char *piterator_old, *piterator_new, *piterator_tmp;
-
-    ret = RegCreateKeyEx(HKEY_CURRENT_USER, reg_jumplist_key, 0, NULL,
-                         REG_OPTION_NON_VOLATILE, (KEY_READ | KEY_WRITE), NULL,
-                         &pjumplist_key, NULL);
-    if (ret != ERROR_SUCCESS) {
         return JUMPLISTREG_ERROR_KEYOPENCREATE_FAILURE;
-    }
 
     /* Get current list of saved sessions in the registry. */
-    value_length = 200;
+    strbuf *oldlist = get_reg_multi_sz(rkey, reg_jumplist_value);
-    old_value = snewn(value_length, char);
+    if (!oldlist) {
-    ret = RegQueryValueEx(pjumplist_key, reg_jumplist_value, NULL, &type,
+        /* Start again with the empty list. */
-                          (BYTE *)old_value, &value_length);
+        oldlist = strbuf_new();
-    /* When the passed buffer is too small, ERROR_MORE_DATA is
+        put_data(oldlist, "\0\0", 2);
-     * returned and the required size is returned in the length
-     * argument. */
-    if (ret == ERROR_MORE_DATA) {
-        sfree(old_value);
-        old_value = snewn(value_length, char);
-        ret = RegQueryValueEx(pjumplist_key, reg_jumplist_value, NULL, &type,
-                              (BYTE *)old_value, &value_length);
-    }
-
-    if (ret == ERROR_FILE_NOT_FOUND) {
-        /* Value doesn't exist yet. Start from an empty value. */
-        *old_value = '\0';
-        *(old_value + 1) = '\0';
-    } else if (ret != ERROR_SUCCESS) {
-        /* Some non-recoverable error occurred. */
-        sfree(old_value);
-        RegCloseKey(pjumplist_key);
-        return JUMPLISTREG_ERROR_VALUEREAD_FAILURE;
-    } else if (type != REG_MULTI_SZ) {
-        /* The value present in the registry has the wrong type: we
-         * try to delete it and start from an empty value. */
-        ret = RegDeleteValue(pjumplist_key, reg_jumplist_value);
-        if (ret != ERROR_SUCCESS) {
-            sfree(old_value);
-            RegCloseKey(pjumplist_key);
-            return JUMPLISTREG_ERROR_VALUEREAD_FAILURE;
-        }
-
-        *old_value = '\0';
-        *(old_value + 1) = '\0';
-    }
-
-    /* Check validity of registry data: REG_MULTI_SZ value must end
-     * with \0\0. */
-    piterator_tmp = old_value;
-    while (((piterator_tmp - old_value) < (value_length - 1)) &&
-           !(*piterator_tmp == '\0' && *(piterator_tmp+1) == '\0')) {
-        ++piterator_tmp;
-    }
-
-    if ((piterator_tmp - old_value) >= (value_length-1)) {
-        /* Invalid value. Start from an empty value. */
-        *old_value = '\0';
-        *(old_value + 1) = '\0';
     }
 
     /*
      * Modify the list, if we're modifying.
      */
+    bool write_failure = false;
     if (add || rem) {
-        /* Walk through the existing list and construct the new list of
+        BinarySource src[1];
-         * saved sessions. */
+        BinarySource_BARE_INIT_PL(src, ptrlen_from_strbuf(oldlist));
-        new_value = snewn(value_length + (add ? strlen(add) + 1 : 0), char);
+        strbuf *newlist = strbuf_new();
-        piterator_new = new_value;
-        piterator_old = old_value;
 
         /* First add the new item to the beginning of the list. */
-        if (add) {
+        if (add)
-            strcpy(piterator_new, add);
+            put_asciz(newlist, add);
-            piterator_new += strlen(piterator_new) + 1;
+
-        }
         /* Now add the existing list, taking care to leave out the removed
          * item, if it was already in the existing list. */
-        while (*piterator_old != '\0') {
+        while (true) {
-            if (!rem || strcmp(piterator_old, rem) != 0) {
+            const char *olditem = get_asciz(src);
+            if (get_err(src))
+                break;
+
+            if (!rem || strcmp(olditem, rem) != 0) {
                 /* Check if this is a valid session, otherwise don't add. */
-                settings_r *psettings_tmp = open_settings_r(piterator_old);
+                settings_r *psettings_tmp = open_settings_r(olditem);
                 if (psettings_tmp != NULL) {
                     close_settings_r(psettings_tmp);
-                    strcpy(piterator_new, piterator_old);
+                    put_asciz(newlist, olditem);
-                    piterator_new += strlen(piterator_new) + 1;
                 }
             }
-            piterator_old += strlen(piterator_old) + 1;
         }
-        *piterator_new = '\0';
-        ++piterator_new;
 
         /* Save the new list to the registry. */
-        ret = RegSetValueEx(pjumplist_key, reg_jumplist_value, 0, REG_MULTI_SZ,
+        write_failure = !put_reg_multi_sz(rkey, reg_jumplist_value, newlist);
-                            (BYTE *)new_value, piterator_new - new_value);
 
-        sfree(old_value);
+        strbuf_free(oldlist);
-        old_value = new_value;
+        oldlist = newlist;
-    } else
-        ret = ERROR_SUCCESS;
-
-    /*
-     * Either return or free the result.
-     */
-    if (out && ret == ERROR_SUCCESS)
-        *out = old_value;
-    else
-        sfree(old_value);
-
-    /* Clean up and return. */
-    RegCloseKey(pjumplist_key);
-
-    if (ret != ERROR_SUCCESS) {
-        return JUMPLISTREG_ERROR_VALUEWRITE_FAILURE;
-    } else {
-        return JUMPLISTREG_OK;
     }
+
+    close_regkey(rkey);
+
+    if (out && !write_failure)
+        *out = strbuf_to_str(oldlist);
+    else
+        strbuf_free(oldlist);
+
+    if (write_failure)
+        return JUMPLISTREG_ERROR_VALUEWRITE_FAILURE;
+    else
+        return JUMPLISTREG_OK;
 }
 
 /* Adds a new entry to the jumplist entries in the registry. */
@@ -800,26 +645,22 @@ char *get_jumplist_registry_entries (void)
  */
 static void registry_recursive_remove(HKEY key)
 {
-    DWORD i;
+    char *name;
-    char name[MAX_PATH + 1];
-    HKEY subkey;
 
-    i = 0;
+    DWORD i = 0;
-    while (RegEnumKey(key, i, name, sizeof(name)) == ERROR_SUCCESS) {
+    while ((name = enum_regkey(key, i)) != NULL) {
-        if (RegOpenKey(key, name, &subkey) == ERROR_SUCCESS) {
+        HKEY subkey = open_regkey(false, key, name);
+        if (subkey) {
             registry_recursive_remove(subkey);
-            RegCloseKey(subkey);
+            close_regkey(subkey);
         }
-        RegDeleteKey(key, name);
+        del_regkey(key, name);
+        sfree(name);
     }
 }
 
 void cleanup_all(void)
 {
-    HKEY key;
-    int ret;
-    char name[MAX_PATH + 1];
-
     /* ------------------------------------------------------------
      * Wipe out the random seed file, in all of its possible
      * locations.
@@ -839,31 +680,34 @@ void cleanup_all(void)
     /*
      * Open the main PuTTY registry key and remove everything in it.
      */
-    if (RegOpenKey(HKEY_CURRENT_USER, PUTTY_REG_POS, &key) ==
+    HKEY key = open_regkey(false, HKEY_CURRENT_USER, PUTTY_REG_POS);
-        ERROR_SUCCESS) {
+    if (key) {
         registry_recursive_remove(key);
-        RegCloseKey(key);
+        close_regkey(key);
     }
     /*
      * Now open the parent key and remove the PuTTY main key. Once
      * we've done that, see if the parent key has any other
      * children.
      */
-    if (RegOpenKey(HKEY_CURRENT_USER, PUTTY_REG_PARENT,
+    if ((key = open_regkey(false, HKEY_CURRENT_USER,
-                   &key) == ERROR_SUCCESS) {
+                           PUTTY_REG_PARENT)) != NULL) {
-        RegDeleteKey(key, PUTTY_REG_PARENT_CHILD);
+        del_regkey(key, PUTTY_REG_PARENT_CHILD);
-        ret = RegEnumKey(key, 0, name, sizeof(name));
+        char *name = enum_regkey(key, 0);
-        RegCloseKey(key);
+        close_regkey(key);
+
         /*
          * If the parent key had no other children, we must delete
          * it in its turn. That means opening the _grandparent_
          * key.
          */
-        if (ret != ERROR_SUCCESS) {
+        if (name) {
-            if (RegOpenKey(HKEY_CURRENT_USER, PUTTY_REG_GPARENT,
+            sfree(name);
-                           &key) == ERROR_SUCCESS) {
+        } else {
-                RegDeleteKey(key, PUTTY_REG_GPARENT_CHILD);
+            if ((key = open_regkey(false, HKEY_CURRENT_USER,
-                RegCloseKey(key);
+                                   PUTTY_REG_GPARENT)) != NULL) {
+                del_regkey(key, PUTTY_REG_GPARENT_CHILD);
+                close_regkey(key);
             }
         }
     }

windows/utils/registry.c

@@ -0,0 +1,184 @@
+/*
+ * Implement convenience wrappers on the awkward low-level functions
+ * for accessing the Windows registry.
+ */
+
+#include "putty.h"
+
+HKEY open_regkey_fn(bool create, HKEY hk, const char *path, ...)
+{
+    HKEY toret = NULL;
+    bool hk_needs_close = false;
+    va_list ap;
+    va_start(ap, path);
+
+    for (; path; path = va_arg(ap, const char *)) {
+        HKEY hk_sub = NULL;
+
+        LONG status;
+        if (create)
+            status = RegCreateKeyEx(
+                hk, path, 0, NULL, REG_OPTION_NON_VOLATILE,
+                KEY_READ | KEY_WRITE, NULL, &hk_sub, NULL);
+        else
+            status = RegOpenKeyEx(
+                hk, path, 0, KEY_READ | KEY_WRITE, &hk_sub);
+
+        if (status != ERROR_SUCCESS)
+            goto out;
+
+        if (hk_needs_close)
+            RegCloseKey(hk);
+        hk = hk_sub;
+        hk_needs_close = true;
+    }
+
+    toret = hk;
+    hk = NULL;
+    hk_needs_close = false;
+
+  out:
+    va_end(ap);
+    if (hk_needs_close)
+        RegCloseKey(hk);
+    return toret;
+}
+
+void close_regkey(HKEY key)
+{
+    RegCloseKey(key);
+}
+
+void del_regkey(HKEY key, const char *name)
+{
+    RegDeleteKey(key, name);
+}
+
+char *enum_regkey(HKEY key, int index)
+{
+    size_t regbuf_size = MAX_PATH + 1;
+    char *regbuf = snewn(regbuf_size, char);
+
+    while (1) {
+        LONG status = RegEnumKey(key, index, regbuf, regbuf_size);
+        if (status == ERROR_SUCCESS)
+            return regbuf;
+        if (status != ERROR_MORE_DATA) {
+            sfree(regbuf);
+            return NULL;
+        }
+        sgrowarray(regbuf, regbuf_size, regbuf_size);
+    }
+}
+
+bool get_reg_dword(HKEY key, const char *name, DWORD *out)
+{
+    DWORD type, size;
+    size = sizeof(*out);
+
+    if (RegQueryValueEx(key, name, 0, &type,
+                        (BYTE *)out, &size) != ERROR_SUCCESS ||
+        size != sizeof(*out) || type != REG_DWORD)
+        return false;
+    else
+        return true;
+}
+
+bool put_reg_dword(HKEY key, const char *name, DWORD value)
+{
+    return RegSetValueEx(key, name, 0, REG_DWORD, (CONST BYTE *) &value,
+                         sizeof(value)) == ERROR_SUCCESS;
+}
+
+char *get_reg_sz(HKEY key, const char *name)
+{
+    DWORD type, size;
+
+    if (RegQueryValueEx(key, name, 0, &type, NULL,
+                        &size) != ERROR_SUCCESS || type != REG_SZ)
+        return NULL;                   /* not a string */
+
+    size_t allocsize = size+1;         /* allow for an extra NUL if needed */
+    char *toret = snewn(allocsize, char);
+    if (RegQueryValueEx(key, name, 0, &type, (BYTE *)toret,
+                        &size) != ERROR_SUCCESS || type != REG_SZ) {
+        sfree(toret);
+        return NULL;
+    }
+    assert(size < allocsize);
+    toret[size] = '\0'; /* add an extra NUL in case RegQueryValueEx
+                         * didn't supply one */
+
+    return toret;
+}
+
+bool put_reg_sz(HKEY key, const char *name, const char *str)
+{
+    /* You have to store the trailing NUL as well */
+    return RegSetValueEx(key, name, 0, REG_SZ, (CONST BYTE *)str,
+                         1 + strlen(str)) == ERROR_SUCCESS;
+}
+
+/*
+ * REG_MULTI_SZ items are stored as a concatenation of NUL-terminated
+ * strings, terminated in turn with an empty string, i.e. a second
+ * consecutive NUL.
+ *
+ * We represent these in their storage format, as a strbuf - but
+ * *without* the second consecutive NUL.
+ *
+ * So you can build up a new MULTI_SZ value in a strbuf by calling
+ * put_asciz once per output string and then put_reg_multi_sz; and you
+ * can consume one by initialising a BinarySource to the result of
+ * get_reg_multi_sz, and then calling get_asciz on it and assuming
+ * that !get_err(src) means you have a real output string.
+ *
+ * Also, calling strbuf_to_str on one of these will give you back a
+ * bare 'char *' with the same double-NUL termination, to pass back to
+ * a caller.
+ */
+strbuf *get_reg_multi_sz(HKEY key, const char *name)
+{
+    DWORD type, size;
+
+    if (RegQueryValueEx(key, name, 0, &type, NULL,
+                        &size) != ERROR_SUCCESS || type != REG_MULTI_SZ)
+        return NULL;                   /* not a string */
+
+    strbuf *toret = strbuf_new();
+    void *ptr = strbuf_append(toret, (size_t)size + 2);
+    if (RegQueryValueEx(key, name, 0, &type, (BYTE *)ptr,
+                        &size) != ERROR_SUCCESS || type != REG_MULTI_SZ) {
+        strbuf_free(toret);
+        return NULL;
+    }
+    strbuf_shrink_to(toret, size);
+    /* Ensure we end with exactly one \0 */
+    while (strbuf_chomp(toret, '\0'));
+    put_byte(toret, '\0');
+    return toret;
+}
+
+bool put_reg_multi_sz(HKEY key, const char *name, strbuf *str)
+{
+    /*
+     * Of course, to write our string list into the registry, we _do_
+     * have to include both trailing NULs. But this is easy, because a
+     * strbuf is also designed to hold a single string and make it
+     * conveniently accessible in NUL-terminated form, so it stores a
+     * NUL in its buffer just beyond its formal length. So we just
+     * include that extra byte in the data we write.
+     */
+    return RegSetValueEx(key, name, 0, REG_MULTI_SZ, (CONST BYTE *)str->s,
+                         str->len + 1) == ERROR_SUCCESS;
+}
+
+char *get_reg_sz_simple(HKEY key, const char *name, const char *leaf)
+{
+    HKEY subkey = open_regkey(false, key, name);
+    if (!subkey)
+        return NULL;
+    char *toret = get_reg_sz(subkey, leaf);
+    RegCloseKey(subkey);
+    return toret;
+}

windows/utils/registry_get_string.c

@@ -1,43 +0,0 @@
-/*
- * Self-contained function to try to fetch a single string value from
- * the Registry, and return it as a dynamically allocated C string.
- */
-
-#include "putty.h"
-
-char *registry_get_string(HKEY root, const char *path, const char *leaf)
-{
-    HKEY key = root;
-    bool need_close_key = false;
-    char *toret = NULL, *str = NULL;
-
-    if (path) {
-        if (RegCreateKey(key, path, &key) != ERROR_SUCCESS)
-            goto out;
-        need_close_key = true;
-    }
-
-    DWORD type, size;
-    if (RegQueryValueEx(key, leaf, 0, &type, NULL, &size) != ERROR_SUCCESS)
-        goto out;
-    if (type != REG_SZ)
-        goto out;
-
-    str = snewn(size + 1, char);
-    DWORD size_got = size;
-    if (RegQueryValueEx(key, leaf, 0, &type, (LPBYTE)str,
-                        &size_got) != ERROR_SUCCESS)
-        goto out;
-    if (type != REG_SZ || size_got > size)
-        goto out;
-    str[size_got] = '\0';
-
-    toret = str;
-    str = NULL;
-
-  out:
-    if (need_close_key)
-        RegCloseKey(key);
-    sfree(str);
-    return toret;
-}