From 81f839676d98024f6c0a6609866bfa81b5f35325 Mon Sep 17 00:00:00 2001 From: Tom de Vries Date: Wed, 19 May 2021 12:46:47 +0200 Subject: [PATCH] sim: ppc: enable -Wpointer-sign warnings 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 | 2 +- sim/ppc/configure.ac | 2 +- sim/ppc/hw_memory.c | 4 ++-- sim/ppc/hw_opic.c | 6 ++++-- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/sim/ppc/configure b/sim/ppc/configure index 1e42a8adac4..a53a30f7fd4 100755 --- a/sim/ppc/configure +++ b/sim/ppc/configure @@ -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 diff --git a/sim/ppc/configure.ac b/sim/ppc/configure.ac index 82250818eab..d1e9c09e9cd 100644 --- a/sim/ppc/configure.ac +++ b/sim/ppc/configure.ac @@ -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 diff --git a/sim/ppc/hw_memory.c b/sim/ppc/hw_memory.c index 46b22f7b6e3..c0826b71139 100644 --- a/sim/ppc/hw_memory.c +++ b/sim/ppc/hw_memory.c @@ -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; diff --git a/sim/ppc/hw_opic.c b/sim/ppc/hw_opic.c index 8afe312a7ef..9404204aa2f 100644 --- a/sim/ppc/hw_opic.c +++ b/sim/ppc/hw_opic.c @@ -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 */ -- 2.30.2