Rejected patches needing review.
Juan Lang
juan.lang at gmail.com
Tue Sep 16 09:58:43 CDT 2008
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
More information about the wine-devel
mailing list