[PATCH v4 0/1] MR129: tests: Read integer uniforms with strtol() and strtoul().

[Giovanni Mascellani (＠giomasce)]

Because %i sscanf() converters are deprecated, and in practice
clamp to [-2^31, 2^31) on 32 bit.

--
  v4: tests: Read integer uniforms with strtol() and strtoul().
 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/129

[PATCH v4 1/1] tests: Read integer uniforms with strtol() and strtoul().

[Giovanni Mascellani]

From: Giovanni Mascellani <gmascellani(a)codeweavers.com>

Because %i sscanf() converters are deprecated, and in practice
clamp to [-2^31, 2^31) on 32 bit.
---
 tests/d3d12.c         |  5 ---
 tests/shader_runner.c | 74 +++++++++++++++++++++++++++++++++++++------
 tests/utils.h         |  5 +++
 3 files changed, 70 insertions(+), 14 deletions(-)

diff --git a/tests/d3d12.c b/tests/d3d12.c
index 405c9419..f8d2ec60 100644
--- a/tests/d3d12.c
+++ b/tests/d3d12.c
@@ -28,11 +28,6 @@ struct test_options test_options = {0};
 static PFN_D3D12_CREATE_VERSIONED_ROOT_SIGNATURE_DESERIALIZER pfn_D3D12CreateVersionedRootSignatureDeserializer;
 static PFN_D3D12_SERIALIZE_VERSIONED_ROOT_SIGNATURE pfn_D3D12SerializeVersionedRootSignature;
 
-struct ivec4
-{
-    int x, y, z, w;
-};
-
 struct dvec2
 {
     double x, y;
diff --git a/tests/shader_runner.c b/tests/shader_runner.c
index 10c6dd21..56d252b7 100644
--- a/tests/shader_runner.c
+++ b/tests/shader_runner.c
@@ -371,6 +371,58 @@ static void set_uniforms(struct shader_runner *runner, size_t offset, size_t cou
     memcpy(runner->uniforms + offset, uniforms, count * sizeof(*runner->uniforms));
 }
 
+static void read_int(const char **line, int *i)
+{
+    char *rest;
+    long val;
+
+    errno = 0;
+    val = strtol(*line, &rest, 0);
+
+    if (errno != 0 || (*rest != '\0' && !isspace((unsigned char)*rest)))
+        fatal_error("Malformed int constant '%s'.\n", *line);
+
+    *i = val;
+    if (*i != val)
+        fatal_error("Out of range int constant '%s'.\n", *line);
+
+    *line = rest;
+}
+
+static void read_uint(const char **line, unsigned int *u)
+{
+    char *rest;
+    unsigned long val;
+
+    errno = 0;
+    val = strtoul(*line, &rest, 0);
+
+    if (errno != 0 || (*rest != '\0' && !isspace((unsigned char)*rest)))
+        fatal_error("Malformed uint constant '%s'.\n", *line);
+
+    *u = val;
+    if (*u != val)
+        fatal_error("Out of range uint constant '%s'.\n", *line);
+
+    *line = rest;
+}
+
+static void read_int4(const char **line, struct ivec4 *v)
+{
+    read_int(line, &v->x);
+    read_int(line, &v->y);
+    read_int(line, &v->z);
+    read_int(line, &v->w);
+}
+
+static void read_uint4(const char **line, struct uvec4 *v)
+{
+    read_uint(line, &v->x);
+    read_uint(line, &v->y);
+    read_uint(line, &v->z);
+    read_uint(line, &v->w);
+}
+
 static void parse_test_directive(struct shader_runner *runner, const char *line)
 {
     char *rest;
@@ -590,28 +642,32 @@ static void parse_test_directive(struct shader_runner *runner, const char *line)
                 fatal_error("Malformed float constant '%s'.\n", line);
             set_uniforms(runner, offset, 1, &f);
         }
-        else if (match_string(line, "int4", &line) || match_string(line, "uint4", &line))
+        else if (match_string(line, "int4", &line))
+        {
+            struct ivec4 v;
+
+            read_int4(&line, &v);
+            set_uniforms(runner, offset, 4, &v);
+        }
+        else if (match_string(line, "uint4", &line))
         {
-            int v[4];
+            struct uvec4 v;
 
-            if (sscanf(line, "%i %i %i %i", &v[0], &v[1], &v[2], &v[3]) < 4)
-                fatal_error("Malformed (u)int4 constant '%s'.\n", line);
-            set_uniforms(runner, offset, 4, v);
+            read_uint4(&line, &v);
+            set_uniforms(runner, offset, 4, &v);
         }
         else if (match_string(line, "int", &line))
         {
             int i;
 
-            if (sscanf(line, "%i", &i) < 1)
-                fatal_error("Malformed int constant '%s'.\n", line);
+            read_int(&line, &i);
             set_uniforms(runner, offset, 1, &i);
         }
         else if (match_string(line, "uint", &line))
         {
             unsigned int u;
 
-            if (sscanf(line, "%u", &u) < 1)
-                fatal_error("Malformed uint constant '%s'.\n", line);
+            read_uint(&line, &u);
             set_uniforms(runner, offset, 1, &u);
         }
         else
diff --git a/tests/utils.h b/tests/utils.h
index 99f1d3a3..5fd7b086 100644
--- a/tests/utils.h
+++ b/tests/utils.h
@@ -35,6 +35,11 @@ struct vec4
     float x, y, z, w;
 };
 
+struct ivec4
+{
+    int x, y, z, w;
+};
+
 struct uvec4
 {
     unsigned int x, y, z, w;
-- 
GitLab

https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/129

[Henri Verbeet (＠hverbeet)]

> > Here and to some extent above, I don't think we can just print "\*line"; there may be additional data beyond "rest" that we'd also output.
>
> This is already happening in various other places. See for example how the `probe` command is parsed. I guess the point of view is that the shader runner is a developer-facing tool, so we prefer a simpler code even if it results in somewhat less polished error messages.

I suppose, but I don't think it's that much more complicated to instead do something like
```c
fatal_error("Out of range integer constant '%.*s'.\n", (int)(rest - *line), *line);
```

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/129#note_27410

[Zebediah Figura (＠zfigura)]

> This is already happening in various other places. See for example how the `probe` command is parsed. I guess the point of view is that the shader runner is a developer-facing tool, so we prefer a simpler code even if it results in somewhat less polished error messages.

In a sense, although it was less an intentional decision towards simplicity as a decision not to put in the effort to make it well-written.

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/129#note_27426