Re: [PATCH-for-11.1 v6 3/6] monitor: Have MonitorDef::get_value() always return int64_t type

[Markus Armbruster <armbru@redhat.com>]

Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:

> On 3/24/26 7:42 AM, Markus Armbruster wrote:
>> Peter Maydell <peter.maydell@linaro.org> writes:
>> 
>>> On Tue, 24 Mar 2026 at 12:57, Markus Armbruster <armbru@redhat.com> wrote:
>>>>
>>>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>>>>
>>>>> Simplify MonitorDef::get_value() handler by having it always
>>>>> return a int64_t type. Truncate to 32-bit in the single caller.
>>>>>
>>>>> Note, this handler is only implemented once for the x86 targets.
>>>
>>>>> @@ -78,7 +80,8 @@ int get_monitor_def(Monitor *mon, int64_t *pval, const char *name)
>>>>>       for(; md->name != NULL; md++) {
>>>>>           if (hmp_compare_cmd(name, md->name)) {
>>>>>               if (md->get_value) {
>>>>> -                *pval = md->get_value(mon, md, md->offset);
>>>>> +                int64_t val = md->get_value(mon, md, md->offset);
>>>>> +                *pval = target_long_bits() == 32 ? (int32_t)val : val;
>>>>
>>>> This assumes target_long_bits() returns either 32 or 64, doesn't it?
>>>>
>>>> Is this true today?
>>>
>>> It's certainly true today, and we insist on that: exec/target_long.h
>>> handles TARGET_LONG_SIZE being 4 or 8 and will #error on anything else.
>> 
>> Good.
>> 
>>> What other values do you expect it could have ?
>> 
>> There might be a need for 128 in the future.  Not an easy change to
>> make.
>>
>
> The day this will happen, there will be more places to fix than the 
> current patch, as it's assumed everywhere else that target_long_bits or 
> TARGET_LONG_BITS is 32 or 64 only. So I would not worry too much about 
> it at the moment.

Yes, not an easy change to make.

I guess my comment came out of a feeling of unease about

    target_long_bits() == 32 ? [32 bit code] : [64 bit code]

I feel there's a bit of friction between abstract requirements and the
concrete software interface.  In the abstract, we need to ask a yes/no
question here.  The concrete interface provides a function that returns
32/64, which works, but isn't obvious from the type.  I guess it's
obvious enough for anybody working in this area of the code.

If truncating to the target's actual width is common, consider providing
function for that.

Feel free to ignore me :)

[...]