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

Reply via email to