Hi Peter,
diff --git a/programs/services/services.c b/programs/services/services.c index 36ed117..cd71a1f 100644 --- a/programs/services/services.c +++ b/programs/services/services.c @@ -106,9 +106,9 @@ static DWORD load_service_config(HKEY hKey, struct service_entry *entry) return err; if ((err = load_reg_string(hKey, SZ_DESCRIPTION, 0, &entry->description)) != 0) return err; - if ((err = load_reg_multisz(hKey, SZ_DEPEND_ON_SERVICE, &entry->dependOnServices)) != 0) + if ((err = load_reg_multisz(hKey, SZ_DEPEND_ON_SERVICE, TRUE, &entry->dependOnServices)) != 0) return err; - if ((err = load_reg_multisz(hKey, SZ_DEPEND_ON_GROUP, &entry->dependOnGroups)) != 0) + if ((err = load_reg_multisz(hKey, SZ_DEPEND_ON_GROUP, FALSE, &entry->dependOnGroups)) != 0) return err;
if ((err = load_reg_dword(hKey, SZ_TYPE, &entry->config.dwServiceType)) != 0) diff --git a/programs/services/services.h b/programs/services/services.h index fd99bf9..cd7de02 100644 --- a/programs/services/services.h +++ b/programs/services/services.h @@ -87,7 +87,7 @@ LPWSTR strdupW(LPCWSTR str); BOOL check_multisz(LPCWSTR lpMultiSz, DWORD cbSize);
DWORD load_reg_string(HKEY hKey, LPCWSTR szValue, BOOL bExpand, LPWSTR *output); -DWORD load_reg_multisz(HKEY hKey, LPCWSTR szValue, LPWSTR *output); +DWORD load_reg_multisz(HKEY hKey, LPCWSTR szValue, BOOL bAllowSingle, LPWSTR *output); DWORD load_reg_dword(HKEY hKey, LPCWSTR szValue, DWORD *output);
static inline LPCWSTR get_display_name(struct service_entry *service) diff --git a/programs/services/utils.c b/programs/services/utils.c index 89eb500..191cc5f 100644 --- a/programs/services/utils.c +++ b/programs/services/utils.c @@ -102,7 +102,7 @@ failed: return err; }
-DWORD load_reg_multisz(HKEY hKey, LPCWSTR szValue, LPWSTR *output) +DWORD load_reg_multisz(HKEY hKey, LPCWSTR szValue, BOOL bAllowSingle, LPWSTR *output) { DWORD size, type; LPWSTR buf = NULL; @@ -118,7 +118,7 @@ DWORD load_reg_multisz(HKEY hKey, LPCWSTR szValue, LPWSTR *output) } goto failed; } - if (type != REG_MULTI_SZ) + if (!((type == REG_MULTI_SZ) || ((type == REG_SZ) && bAllowSingle))) { err = ERROR_INVALID_DATATYPE; goto failed;
It seems as though the bAllowSingle variable is useless, as the only caller of it tries first with TRUE and again with FALSE. If the registry value is of type REG_SZ, the first call (when bAllowSingle is TRUE) will succeed. Similarly, if it's of type REG_MULTI_SZ, the first call will succeed. Thus, the second call, with bAllowSingle is FALSE, is always unnecessary.
Furthermore, since there's only one place that calls load_reg_multisz, and it accepts both REG_SZ and REG_MULTI_SZ, why not just change load_reg_multisz always to accept either REG_SZ or REG_MULTI_SZ? Complicating the interface unnecessarily isn't a good change.
I have a minor quibble with your second patch: LPWSTR path = NULL, str; @@ -174,17 +175,31 @@ static BOOL load_driver(void)
/* read the executable path from memory */ size = 0; - if (RegQueryValueExW( driver_hkey, ImagePathW, NULL, &type, NULL, &size )) return FALSE; - - str = HeapAlloc( GetProcessHeap(), 0, size ); - if (!RegQueryValueExW( driver_hkey, ImagePathW, NULL, &type, (LPBYTE)str, &size )) + if (!RegQueryValueExW( driver_hkey, ImagePathW, NULL, &type, NULL, &size )) + { + str = HeapAlloc( GetProcessHeap(), 0, size ); You only use str in the then branch of this if statement, so its scope should be changed: it should be declared where, where it's allocated and later freed, instead. --Juan