Am 01.11.22 um 10:27 schrieb Philippe Mathieu-Daudé:

On 1/11/22 09:28, Stefan Weil via wrote:
Am 12.09.22 um 14:26 schrieb Milica Lazarevic:
Both versions of IMMEDIATE functions have been removed.

Before this patch, we'd been calling img_format twice, the first time
through the IMMEDIATE to get an appropriate string and the second time
to print that string. There's no more need for that. Therefore, calls to
IMMEDIATE are removed, and now we're directly printing the integer
values instead.

Signed-off-by: Milica Lazarevic <milica.lazare...@syrmia.com>
---
  disas/nanomips.cpp | 756 ++++++++++++++++-----------------------------
  1 file changed, 265 insertions(+), 491 deletions(-)

diff --git a/disas/nanomips.cpp b/disas/nanomips.cpp
index 816155527d..441204bb84 100644
--- a/disas/nanomips.cpp
+++ b/disas/nanomips.cpp
[...]
@@ -3305,11 +3271,9 @@ static char *CACHE(uint64 instruction, Dis_info *info)
      uint64 rs_value = extract_rs_20_19_18_17_16(instruction);
      int64 s_value = extract_s__se8_15_7_6_5_4_3_2_1_0(instruction);
-    char *op = IMMEDIATE(op_value);
-    char *s = IMMEDIATE(s_value);
      const char *rs = GPR(rs_value);
-    return img_format("CACHE %s, %s(%s)", op, s, rs);
+    return img_format("CACHE 0x%" PRIx64 ", %s(%s)", op_value, s_value, rs);
  }
@@ -3329,11 +3293,9 @@ static char *CACHEE(uint64 instruction, Dis_info *info)
      uint64 rs_value = extract_rs_20_19_18_17_16(instruction);
      int64 s_value = extract_s__se8_15_7_6_5_4_3_2_1_0(instruction);
-    char *op = IMMEDIATE(op_value);
-    char *s = IMMEDIATE(s_value);
      const char *rs = GPR(rs_value);
-    return img_format("CACHEE %s, %s(%s)", op, s, rs);
+    return img_format("CACHEE 0x%" PRIx64 ", %s(%s)", op_value, s_value, rs);
  }

Do we really want to format "int64 s_value" as a string? The code now has lots of wrong format strings. Add the patch below to get the compiler report.

We once had a discussion about using G_GNUC_PRINTF for local functions or not. I think that this example clearly shows that it should be mandatory.

Yes. The problem here is nobody wants to maintain this code, but we
inherited it. IIUC this series doesn't make it worst, it just remove
the C++ dependency on UNIX-based hosts.


I expect that "%s" with an int64 s_value will cause a crash while the old code worked, so things are worse now and should be fixed for the release. If nobody maintains that code, I can try to prepare a patch.

Stefan



Reply via email to