[PATCH] msvcp120: Avoid type conversion warnings in test_tr2_sys__dir_operation.

[Gerald Pfeifer]

GCC 10 warns

  msvcp120.c:1777:11: warning: overflow in conversion from 'enum file_type'
  to 'int' changes value from 'type = 3735928559' to '-559038737' 
  msvcp120.c:1807:11: warning: overflow in conversion from 'enum file_type'
  to 'int' changes value from 'type = 3735928559' to '-559038737' 
  msvcp120.c:1817:11: warning: overflow in conversion from 'enum file_type'
  to 'int' changes value from 'type = 3735928559' to '-559038737' 

where 3735928559 is 0xdeadbeef.

This happens since first this variable is assigned to an enum and
that then to an int, and GCC diagnoses the latter as an overflow
(from signed to unsigned).

Assigning a hex constant does not trigger, so simply break out those
two assignments.

Gerald

Signed-off-by: Gerald Pfeifer <gerald(a)pfeifer.com>
---
 dlls/msvcp120/tests/msvcp120.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/dlls/msvcp120/tests/msvcp120.c b/dlls/msvcp120/tests/msvcp120.c
index 7a382ac6fa..3cfabaa7ca 100644
--- a/dlls/msvcp120/tests/msvcp120.c
+++ b/dlls/msvcp120/tests/msvcp120.c
@@ -1774,7 +1774,8 @@ static void test_tr2_sys__dir_operation(void)
 
     memset(first_file_name, 0xff, MAX_PATH);
     memset(dest, 0, MAX_PATH);
-    err = type = 0xdeadbeef;
+    err = 0xdeadbeef;
+    type = 0xdeadbeef;
     result_handle = NULL;
     result_handle = p_tr2_sys__Open_dir(first_file_name, "tr2_test_dir", &err, &type);
     ok(result_handle != NULL, "tr2_sys__Open_dir(): expect: not NULL, got %p\n", result_handle);
@@ -1804,7 +1805,8 @@ static void test_tr2_sys__dir_operation(void)
     ok(num_of_other_files == 0, "found %d other files\n", num_of_other_files);
 
     memset(first_file_name, 0xff, MAX_PATH);
-    err = type = 0xdeadbeef;
+    err = 0xdeadbeef;
+    type = 0xdeadbeef;
     result_handle = file;
     result_handle = p_tr2_sys__Open_dir(first_file_name, "not_exist", &err, &type);
     ok(result_handle == NULL, "tr2_sys__Open_dir(): expect: NULL, got %p\n", result_handle);
@@ -1814,7 +1816,8 @@ static void test_tr2_sys__dir_operation(void)
 
     CreateDirectoryA("empty_dir", NULL);
     memset(first_file_name, 0xff, MAX_PATH);
-    err = type = 0xdeadbeef;
+    err = 0xdeadbeef;
+    type = 0xdeadbeef;
     result_handle = file;
     result_handle = p_tr2_sys__Open_dir(first_file_name, "empty_dir", &err, &type);
     ok(result_handle == NULL, "tr2_sys__Open_dir(): expect: NULL, got %p\n", result_handle);
-- 
2.25.0

[Piotr Caban]

Hi Gerald,
> +    type = 0xdeadbeef;
> I guess this assignment will work on all compilers. Isn't assigning the 
> value outside of enum range unspecified in C standard? I don't know if 
> it's a valid concern but I'm worried it may still cause a warning on 
> some compilers (I don't know any that actually warns in this case). How 
> about adding a new value to enum and using it in tests for uninitialized 
> value (or just using 0)?

Thanks,
Piotr

[Gerald Pfeifer]

On Tue, 5 May 2020, Piotr Caban wrote:
> > +    type = 0xdeadbeef;
> > I guess this assignment will work on all compilers. Isn't assigning 
> > the value outside of enum range unspecified in C standard?

Based on your question I dug into the C11 standard and found

  "An identifier declared as an enumeration constant has type int."

So that should not be a concern.  And if it turns out it ever triggers
something, we can...
> I don't know if it's a valid concern but I'm worried it may still cause 
> a warning on some compilers (I don't know any that actually warns in 
> this case). How about adding a new value to enum and using it in tests 
> for uninitialized value (or just using 0)?

...always follow the approach you outline (which will be easier than 
since there is a single assignment to adjust, not the current double 
assignment).

If after the above you'd still like to see us use 0x0 or an distinct 
member of the enum (how would you call that?) let me know and I'll cook
a new patch.

Gerald

[Piotr Caban]

Hi Gerald,

I think I would prefer to go with additional enum value approach. Even 
the C11 standard gives such assignment as an example of when compiler 
may generate a warning (Annex I, Common warnings in final draft). It 
also states that type = 0xdeadbeef is an example of "undefined behavior" 
(signed integer overflow).

Something like this should work:
enum file_type {
     file_type_test_init = -1,
...
err = 0xdeadbeef;
type = file_type_test_init;

Thanks,
Piotr

On 5/6/20 10:40 PM, Gerald Pfeifer wrote:
> On Tue, 5 May 2020, Piotr Caban wrote:
> > > +    type = 0xdeadbeef;
> > > I guess this assignment will work on all compilers. Isn't assigning
> > > the value outside of enum range unspecified in C standard?
>
> Based on your question I dug into the C11 standard and found
>
> "An identifier declared as an enumeration constant has type int."
>
> So that should not be a concern.  And if it turns out it ever triggers
> something, we can...
> > I don't know if it's a valid concern but I'm worried it may still cause
> > a warning on some compilers (I don't know any that actually warns in
> > this case). How about adding a new value to enum and using it in tests
> > for uninitialized value (or just using 0)?
>
> ...always follow the approach you outline (which will be easier than
> since there is a single assignment to adjust, not the current double
> assignment).
>
> If after the above you'd still like to see us use 0x0 or an distinct
> member of the enum (how would you call that?) let me know and I'll cook
> a new patch.
>
> Gerald