On 2017-05-22 03:09, hbelusca@svn.reactos.org wrote:
--- branches/setup_improvements/base/setup/lib/arcname.c (added) +++ branches/setup_improvements/base/setup/lib/arcname.c [iso-8859-1] Mon May 22 01:09:35 2017
+PCSTR +ArcGetNextTokenA(
- IN PCSTR ArcPath,
- OUT PANSI_STRING TokenSpecifier,
- OUT PULONG Key)
+{
- HRESULT hr;
- PCSTR p = ArcPath;
- ULONG SpecifierLength;
- ULONG KeyValue;
- /*
* We must have a valid "specifier(key)" string, where 'specifier'* cannot be the empty string, and is followed by '('.*/- p = strchr(p, '(');
- if (p == NULL)
return NULL; /* No '(' found */- if (p == ArcPath)
return NULL; /* Path starts with '(' and is thus invalid */- SpecifierLength = (p - ArcPath) * sizeof(CHAR);
- /*
* The strtoul function skips any leading whitespace.** Note that if the token is "specifier()" then strtoul won't perform* any conversion and return 0, therefore effectively making the token* equivalent to "specifier(0)", as it should be.*/- // KeyValue = atoi(p);
- KeyValue = strtoul(p, (PSTR*)&p, 10);
- /* Skip any trailing whitespace */
- while (*p && isspace(*p)) ++p;
isspace(0) is false so you don't need the *p check.
+PCWSTR +ArcGetNextTokenU(
- IN PCWSTR ArcPath,
- OUT PUNICODE_STRING TokenSpecifier,
- OUT PULONG Key)
+{
- HRESULT hr;
- PCWSTR p = ArcPath;
- ULONG SpecifierLength;
- ULONG KeyValue;
- /*
* We must have a valid "specifier(key)" string, where 'specifier'* cannot be the empty string, and is followed by '('.*/- p = wcschr(p, L'(');
- if (p == NULL)
return NULL; /* No '(' found */- if (p == ArcPath)
return NULL; /* Path starts with '(' and is thus invalid */- SpecifierLength = (p - ArcPath) * sizeof(WCHAR);
- ++p;
- /*
* The strtoul function skips any leading whitespace.** Note that if the token is "specifier()" then strtoul won't perform* any conversion and return 0, therefore effectively making the token* equivalent to "specifier(0)", as it should be.*/- // KeyValue = _wtoi(p);
- KeyValue = wcstoul(p, (PWSTR*)&p, 10);
- ASSERT(p);
This assert seems superfluous, you just dereferenced the pointer. Also, you incremented it just above and also (rightly) assumed it was larger than ArcPath.
+static NTSTATUS +ResolveArcNameManually(
- OUT PUNICODE_STRING NtName,
- IN OUT PCWSTR* ArcNamePath,
- IN PPARTLIST PartList OPTIONAL)
+{ [...] +Quit:
- if (FAILED(hr))
- {
/** We can directly cast the HRESULTs into NTSTATUS since the error codes* returned by StringCbPrintfW:* STRSAFE_E_INVALID_PARAMETER == 0x80070057,* STRSAFE_E_INSUFFICIENT_BUFFER == 0x8007007a,* do not have assigned values in the NTSTATUS space.*/return (NTSTATUS)hr;
To me that sounds like an argument of why you _can't_ do that. (You know the Rtl versions of these functions give you an NTSTATUS btw, right?)
--- branches/setup_improvements/base/setup/lib/arcname_tests.c (added) +++ branches/setup_improvements/base/setup/lib/arcname_tests.c [iso-8859-1] Mon May 22 01:09:35 2017 @@ -0,0 +1,142 @@ +/*
- Tests for the arcname.c functions:
- ArcPathNormalize(),
- ArcPathToNtPath().
- You should certainly fix the included headers before being able
- to compile this file (as I didn't bother to have it compilable
- under our (P/N)DK, but just under VS).
- */
Wine's test framework works okay for unit tests. You can make this an apitest by just #include'ing the C source file, then it can run on Testbot.