Re: [PATCH v5 5/9] monitor: Have MonitorDef::get_value() return an unsigned type

[Pierrick Bouvier]

On 1/29/26 8:40 AM, Philippe Mathieu-Daudé wrote:
diff --git a/include/monitor/hmp-target.h b/include/monitor/hmp-target.h
index 5167d17d41d..6d6653aee6e 100644
--- a/include/monitor/hmp-target.h
+++ b/include/monitor/hmp-target.h
@@ -32,14 +32,13 @@ typedef struct MonitorDef MonitorDef;
  struct MonitorDef {
      const char *name;
      int offset;
-    target_long (*get_value)(Monitor *mon, const struct MonitorDef *md,
-                             int val);
+    uint64_t (*get_value)(Monitor *mon, const struct MonitorDef *md, int val);
      int type;
  };
  #endif
  
-#define MD_TLONG 0
-#define MD_I32   1
+#define MD_TULONG 0
+#define MD_U32    1
  
  const MonitorDef *target_monitor_defs(void);
  int target_get_monitor_def(CPUState *cs, const char *name, uint64_t *pval);
diff --git a/monitor/hmp-target.c b/monitor/hmp-target.c
index 420969bd6eb..1600666ee92 100644
--- a/monitor/hmp-target.c
+++ b/monitor/hmp-target.c
@@ -67,7 +67,6 @@ int get_monitor_def(Monitor *mon, int64_t *pval, const char 
*name)
  {
      const MonitorDef *md = target_monitor_defs();
      CPUState *cs = mon_get_cpu(mon);
-    void *ptr;
      uint64_t tmp = 0;
      int ret;
  
@@ -81,13 +80,14 @@ int get_monitor_def(Monitor *mon, int64_t *pval, const char *name)
                  *pval = md->get_value(mon, md, md->offset);
              } else {
                  CPUArchState *env = mon_get_cpu_env(mon);
-                ptr = (uint8_t *)env + md->offset;
+                void *ptr = (uint8_t *)env + md->offset;
+
                  switch(md->type) {
-                case MD_I32:
-                    *pval = *(int32_t *)ptr;
+                case MD_U32:
+                    *pval = *(uint32_t *)ptr;
                      break;
-                case MD_TLONG:
-                    *pval = *(target_long *)ptr;
+                case MD_TULONG:
+                    *pval = *(target_ulong *)ptr;
                      break;
                  default:
                      *pval = 0;

This one is not obvious to me.
All the caller chain is using int64_t (maybe wrongly), so would be safer 
to keep a signed value, and adapt the read size.
So best would to merge this diff with next patch in series.