sim: ppc: enable -Wpointer-sign warnings
authorTom de Vries <tdevries@suse.de>
Wed, 19 May 2021 10:46:47 +0000 (12:46 +0200)
committerMike Frysinger <vapier@gentoo.org>
Thu, 9 Sep 2021 03:33:25 +0000 (23:33 -0400)
When compiling with --enable-werror and CFLAGS="-O0 -g -Wall", we run into:
...
src/sim/ppc/hw_memory.c: In function 'hw_memory_init_address':
src/sim/ppc/hw_memory.c:204:7: error: pointer targets in passing argument 4 \
  of 'device_find_integer_array_property' differ in signedness \
  [-Werror=pointer-sign]
       &new_chunk->size);
       ^
...

Fix these by adding an explicit pointer cast.  It's a bit ugly to use APIs
based on signed integers to read out unsigned values, but in practice, this
is par for the course in the ppc code.

We already use signed APIs and assign the result to unsigned values a lot:
see how device_find_integer_property returns a signed integer (cell), but
then assign it to unsigned types.  The array APIs are not used that often
which is why we don't see many warnings, and we disable warnings when we
assign signed integers to unsigned integers in general.

The dtc/libfdt project (which is the standard in other projects) processes
the fdt blob as a series of bytes without any type information.  Typing is
left to the caller.  They have core APIs that read/write bytes, and a few
helper functions to cast/convert those bytes to the right value (e.g. u32).
In this ppc sim code, the core APIs use signed integers, and the callers
convert to unsigned, usually implicitly.

We could add some core APIs to the ppc sim that deal with raw bytes and then
add some helpers to convert to the right type, but that seems like a lot of
lifting for what boils down to a cast, and is effectively equivalent to all
the implicit assignments we use elsewhere.  Long term, a lot of the ppc code
should either get converted to existing sim common code, or we should stand
up proper APIs in the common code first, or use standard libraries to do all
the processing (e.g. libfdt).  Either way, this device.c code would all get
deleted, and callers (like these hw_*.c files) would get converted.  Which
is also why we go with a cast rather new (but largely unused) APIs.

sim/ppc/configure
sim/ppc/configure.ac
sim/ppc/hw_memory.c
sim/ppc/hw_opic.c

index 1e42a8adac4292a9fb0960e0a59b06cf29e590f7..a53a30f7fd41b6a901e8edb224b72321597f6055 100755 (executable)
@@ -3486,7 +3486,7 @@ sim_warnings="-Wall -Wdeclaration-after-statement -Wpointer-arith
 -Wmissing-declarations
 -Wmissing-prototypes
 -Wdeclaration-after-statement -Wmissing-parameter-type
--Wno-pointer-sign
+-Wpointer-sign
 -Wold-style-declaration -Wold-style-definition
 "
 # Enable -Wno-format by default when using gcc on mingw since many
index 82250818eab7ac85ce0df01a5883bd5cab365616..d1e9c09e9cd9350e8df9737b15dbbd90822da240 100644 (file)
@@ -427,7 +427,7 @@ sim_warnings="-Wall -Wdeclaration-after-statement -Wpointer-arith
 -Wmissing-declarations
 -Wmissing-prototypes
 -Wdeclaration-after-statement -Wmissing-parameter-type
--Wno-pointer-sign
+-Wpointer-sign
 -Wold-style-declaration -Wold-style-definition
 "
 # Enable -Wno-format by default when using gcc on mingw since many
index 46b22f7b6e3370469b5337e5162351415614779e..c0826b711395806cfab81e86d1ad7376b7df19b8 100644 (file)
@@ -199,9 +199,9 @@ hw_memory_init_address(device *me)
         cell_nr += 2) {
       hw_memory_chunk *new_chunk = ZALLOC(hw_memory_chunk);
       device_find_integer_array_property(me, "available", cell_nr,
-                                        &new_chunk->address);
+                                        (signed_cell *)&new_chunk->address);
       device_find_integer_array_property(me, "available", cell_nr + 1,
-                                        &new_chunk->size);
+                                        (signed_cell *)&new_chunk->size);
       new_chunk->available = 1;
       *curr_chunk = new_chunk;
       curr_chunk = &new_chunk->next;
index 8afe312a7efe97492c6a09eaf6e9a031a27b7fab..9404204aa2f37928e80244933e3842990779b8a7 100644 (file)
@@ -417,10 +417,12 @@ hw_opic_init_data(device *me)
       }
       if (!device_find_integer_array_property(me, "interrupt-ranges",
                                              reg_nr * 2,
-                                             &opic->isu_block[isb].int_number)
+                                             (signed_cell *)
+                                               &opic->isu_block[isb].int_number)
          || !device_find_integer_array_property(me, "interrupt-ranges",
                                                 reg_nr * 2 + 1,
-                                                &opic->isu_block[isb].range))
+                                                (signed_cell *)
+                                                  &opic->isu_block[isb].range))
        device_error(me, "missing or invalid interrupt-ranges property entry %d", reg_nr);
       /* first reg entry specifies the address of both the IDU and the
          first set of ISU registers, adjust things accordingly */