On 2017-05-22 03:09, hbelu...@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. _______________________________________________ Ros-dev mailing list Ros-dev@reactos.org http://www.reactos.org/mailman/listinfo/ros-dev